Skip to content

v1: Initial implementation #1

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

Merged
merged 18 commits into from
Mar 24, 2023
Merged

v1: Initial implementation #1

merged 18 commits into from
Mar 24, 2023

Conversation

PJColombo
Copy link
Member

@PJColombo PJColombo commented Mar 16, 2023

Resolves #2 , resolves #8
Notes:

  • Last slot tracking database updates are performed after an error occurs or iterating over a set of slots (default to processing all the slots from 0 to the current one) instead of updating it after each slot individually to reduce slot processing overhead.

- Abstract db handling logic
- Add mongodb implementation
- Keep track of last processed slot.
- Refactor slot processing logic.
- Handle `unwrap()` calls properly.
@PJColombo PJColombo marked this pull request as ready for review March 18, 2023 19:09
@PJColombo PJColombo linked an issue Mar 19, 2023 that may be closed by this pull request
@PJColombo PJColombo requested a review from 0xGabi March 19, 2023 15:59
- Modularize env and context initialization logic
- Include db manager creation logic into struct
Copy link
Contributor

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

Overall it's coming together! A few comments, mainly about being a tad more idiomatic about some things. Feel free to close/ignore what you feel like deferring for later.

pub async fn get_block(
&self,
slot: Option<u32>,
) -> Result<Option<Block>, Box<dyn error::Error>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we wanna have fancier error handling (totally doable later down the line) we could probably use anyhow to easily bubble up/downcast errors and define all the possible error types for this API client with thiserror (here's a good example)

Copy link
Member Author

@PJColombo PJColombo Mar 22, 2023

Choose a reason for hiding this comment

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

I was having a bit of trouble finding the best approach for error handling.

This definitely seems the way to go. Thank you for sharing this crate and example!.

src/slots/mod.rs Outdated
utils::web3::{calculate_versioned_hash, get_eip_4844_tx, get_tx_versioned_hashes},
};

type StdErr = Box<dyn error::Error>;
Copy link
Contributor

Choose a reason for hiding this comment

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

again, recommend using anyhow :D

src/slots/mod.rs Outdated
let beacon_block = match beacon_api.get_block(Some(slot)).await? {
Some(block) => block,
None => {
info!(
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! Have you looked into tracing? it's essentially logging on steroids—will help with instrumenting robust logging and we could eventually trace all API calls which will help debug stuff in the future.

Copy link
Member Author

@PJColombo PJColombo Mar 22, 2023

Choose a reason for hiding this comment

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

I found out about it yesterday when I was working on parallelizing the indexer.

The concept of structured spans, as opposed to conventional logs, makes it a more suitable choice for asynchronous and multi-threading environments.

Thank you for sharing!

- Move env variables logic to its own file.
- Generalize std error.
- Use `reqwest` client.
- Add try/catch instantiation
- Rewrite functions to make it more idiomatic
@PJColombo
Copy link
Member Author

I created some issues to be solved in future PRs following some of the remaining comments discussed above: #9 , #3, #11.

Merging this now!

@PJColombo PJColombo merged commit 9a6dab7 into master Mar 24, 2023
@PJColombo PJColombo deleted the v1 branch March 24, 2023 20:52
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.

Support parameter to start retrieving blobs from given block/slot number Indexer Rust implementation
3 participants