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

Block import crate #232

Merged
merged 7 commits into from
Aug 28, 2024
Merged

Block import crate #232

merged 7 commits into from
Aug 28, 2024

Conversation

cchudant
Copy link
Member

@cchudant cchudant commented Aug 21, 2024

Closes #228 for now, as this takes most of the code from that pr except the pipeline.

  • chain config is now in its own primitive crate, and the StarknetVersion struct was moved there
  • class block ids are not stored in the class info anymore
  • all of the commitment computation and block conversion is done in a new crate block-import, and mempool uses it to store blocks too
  • dc_sync is now only the pipeline and the fetching

I have a few commits on top of this one, which add a genesis provider that also uses the block import crate - i'm opening the pr without these for now so that @jbcaron can work on top of this without having to deal with broken code


/// This needs to be called sequentially, it will apply the state diff to the db, verify the state root and save the block.
/// This runs on the [`rayon`] threadpool however as it uses parallelism inside.
pub fn verify_apply_inner(
Copy link
Member

Choose a reason for hiding this comment

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

Can the block storage in the database and the trie not be done in parallel? Is it too much load for the database?

Copy link
Member Author

Choose a reason for hiding this comment

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

thats true we should do that

Copy link
Member Author

@cchudant cchudant Aug 23, 2024

Choose a reason for hiding this comment

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

we can't, because the db needs the block hash and global_state_root, and we don't have the global_state_root until we have computed the trie
we could overlap some of the db writes inside dc-db, like, everything except the actual block header which we could store last once we have the global state root, but this would make the interface between dc-db and block-import a mess i think
Each of these steps are parallelized internally, so I don't think we would see much of a perf change at all

I am adding a comment pointing to this potential improvement so that we can potentially investigate it deeper if we ever want to but I'm pretty sure this is not worth it

@EvolveArt
Copy link
Contributor

pls rebase @cchudant

@antiyro antiyro marked this pull request as ready for review August 22, 2024 17:52
crates/client/block_import/src/pre_validate.rs Outdated Show resolved Hide resolved
crates/client/block_import/src/pre_validate.rs Outdated Show resolved Hide resolved
crates/client/block_import/src/pre_validate.rs Outdated Show resolved Hide resolved
crates/client/block_import/src/verify_apply.rs Outdated Show resolved Hide resolved
crates/client/mempool/src/block_production.rs Outdated Show resolved Hide resolved
crates/client/mempool/src/block_production.rs Show resolved Hide resolved
Copy link

Coverage report

The coverage rate is 67.42014742014743%

69% of new lines are covered.

Diff Coverage details (click to unfold)

crates/node/src/service/l1.rs

0.0% of new lines are covered

crates/node/src/service/rpc/server.rs

100.0% of new lines are covered

crates/client/block_import/src/pre_validate.rs

87.4493927125506% of new lines are covered

crates/client/mempool/src/block_production.rs

0.0% of new lines are covered

crates/client/mempool/src/inner.rs

0.0% of new lines are covered

crates/client/block_import/src/verify_apply.rs

68.89952153110048% of new lines are covered

crates/node/src/util.rs

100.0% of new lines are covered

crates/primitives/transactions/src/compute_hash.rs

76.92307692307692% of new lines are covered

crates/client/rpc/src/methods/read/estimate_fee.rs

0.0% of new lines are covered

crates/primitives/transactions/src/from_starknet_core.rs

12.056737588652481% of new lines are covered

crates/client/rpc/src/methods/trace/simulate_transactions.rs

0.0% of new lines are covered

crates/client/sync/src/utils.rs

100.0% of new lines are covered

crates/node/src/service/sync.rs

100.0% of new lines are covered

crates/client/block_import/src/lib.rs

88.88888888888889% of new lines are covered

crates/client/mempool/src/close_block.rs

0.0% of new lines are covered

crates/client/sync/src/l2.rs

96.29629629629629% of new lines are covered

crates/primitives/class/src/compile.rs

100.0% of new lines are covered

crates/client/sync/src/lib.rs

95.1219512195122% of new lines are covered

crates/client/sync/src/fetch/mod.rs

50.0% of new lines are covered

crates/client/sync/src/fetch/fetchers.rs

96.19047619047619% of new lines are covered

crates/client/block_import/src/rayon.rs

87.5% of new lines are covered

crates/client/db/src/db_block_id.rs

100.0% of new lines are covered

crates/client/db/src/lib.rs

0.0% of new lines are covered

crates/client/db/src/class_db.rs

86.36363636363636% of new lines are covered

crates/client/rpc/src/mempool_provider.rs

0.0% of new lines are covered

crates/primitives/transactions/src/from_broadcasted_transaction.rs

0.0% of new lines are covered

crates/node/src/cli/mod.rs

100.0% of new lines are covered

crates/primitives/block/src/header.rs

80.0% of new lines are covered

crates/client/mempool/src/lib.rs

50.0% of new lines are covered

crates/client/block_import/src/verify_apply/contracts.rs

0.0% of new lines are covered

@antiyro antiyro merged commit 77f957c into main Aug 28, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants