Skip to content

Conversation

@mpaulucci
Copy link
Collaborator

@mpaulucci mpaulucci commented Mar 19, 2025

Motivation
Don't have to rely on the global mutex of the Syncer to fetch invalid blocks

Description
Not sure if this is the ultimate solution, Im still unsure if it is better to store invalid ancestors in db or in memoy

@github-actions
Copy link

github-actions bot commented Mar 19, 2025

Lines of code report

Total lines added: 75
Total lines removed: 15
Total lines changed: 90

Detailed view
+----------------------------------------------------+-------+------+
| File                                               | Lines | Diff |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs               | 576   | -1   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync_manager.rs       | 112   | -12  |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/fork_choice.rs | 376   | +1   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/payload.rs     | 681   | -2   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs                | 713   | +1   |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/api.rs                       | 228   | +6   |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store.rs                     | 1216  | +15  |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/in_memory.rs        | 559   | +14  |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/libmdbx.rs          | 1249  | +16  |
+----------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/redb.rs             | 1077  | +22  |
+----------------------------------------------------+-------+------+

@mpaulucci mpaulucci force-pushed the persist-bad-blocks branch from 5f07ffd to 747d7cd Compare March 20, 2025 13:32
@mpaulucci mpaulucci marked this pull request as ready for review April 1, 2025 12:40
@mpaulucci mpaulucci requested a review from a team as a code owner April 1, 2025 12:40
Copy link
Contributor

@ElFantasma ElFantasma left a comment

Choose a reason for hiding this comment

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

This LGTM, but don't have an opinion on whether we should store invalid ancestors on db or in memory. I guess it depends on if we want the data to survive app restart or not, right?

Comment on lines 233 to 234
.get_invalid_ancestor(head_block.parent_hash)
.unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to unwrap here?

Comment on lines 240 to 244
} else {
// Check if there is an ongoing sync before applying the forkchoice
match context.sync_status()? {
// Apply current fork choice
SyncStatus::Inactive => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably for another PR, but this change in addition to the refactor of the SyncManger might allow us to enable some additional Invalid NewPayload hive test.

Copy link
Contributor

@fmoletta fmoletta left a comment

Choose a reason for hiding this comment

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

LGTM!

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
@mpaulucci mpaulucci added this pull request to the merge queue Apr 8, 2025
Merged via the queue into main with commit 8be6e86 Apr 8, 2025
19 checks passed
@mpaulucci mpaulucci deleted the persist-bad-blocks branch April 8, 2025 14:25
pedrobergamini pushed a commit to pedrobergamini/ethrex that referenced this pull request Aug 24, 2025
**Motivation**
Don't have to rely on the global mutex of the Syncer to fetch invalid
blocks

**Description**
Not sure if this is the ultimate solution, Im still unsure if it is
better to store invalid ancestors in db or in memoy

---------

Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com>
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.

5 participants