Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mapping #4

Merged
merged 6 commits into from
Jul 24, 2024
Merged

mapping #4

merged 6 commits into from
Jul 24, 2024

Conversation

fbrv
Copy link

@fbrv fbrv commented Jul 22, 2024

No description provided.

@fbrv fbrv changed the title wip: mapping mapping Jul 22, 2024
@fbrv fbrv force-pushed the fb/pubkey-url-mapping branch 3 times, most recently from 8b8a692 to 367a554 Compare July 23, 2024 09:41
README.md Outdated Show resolved Hide resolved
src/lookahead/manager.rs Outdated Show resolved Hide resolved
src/lookahead/manager.rs Outdated Show resolved Hide resolved
src/forward_service.rs Show resolved Hide resolved
src/config.rs Outdated
#[serde(rename = "beacon-urls")]
pub beacon_urls: Vec<String>,
#[serde(rename = "lookahead")]
pub lookahead_providers_relays: Vec<Lookahead>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be something like lookaheads ?

src/config.rs Outdated
#[serde(rename = "chain-id")]
pub chain_id: u16,
#[serde(rename = "relay-urls")]
pub relay_urls: Vec<String>,
#[serde(rename = "relays")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this?

None => bail!("no lookahead provider found"),
Some(entry) => match &self.url_provider {
UrlProvider::LookaheadEntry => {
Ok(Url::from_str(&entry.url).expect("not a valid url"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i dont think we should panic here, eg. if there is a malformed string coming from the provider. Let's return an error and log the wrong string

.route("/", post(forward_request))
.layer(TraceLayer::new_for_http())
.with_state(Arc::new(shared_state))
}

#[tracing::instrument]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would log the full function parameters every time , which is a bit verbose. We can customize what is added to the span https://docs.rs/tracing/latest/tracing/attr.instrument.html

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should log all errors but otherwise we can keep debug/trace for others , otherwise logging for each request may be create too much overhead

#[serde(rename = "registry")]
pub registry: Option<HashMap<BlsPublicKey, Url>>,
#[serde(rename = "url-provider")]
pub url_provider: UrlProvider,
}

impl Config {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add some validation here? eg if there' s no relays

@ltitanb ltitanb merged commit 225b002 into main Jul 24, 2024
3 checks passed
@ltitanb ltitanb deleted the fb/pubkey-url-mapping branch July 24, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants