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

fix: disconnect on PoW peer message #119

Merged
merged 3 commits into from
Jan 7, 2025
Merged

Conversation

greged93
Copy link
Collaborator

@greged93 greged93 commented Dec 27, 2024

Avoid disconnecting peer on NewBlock and NewBlockHashes peer messages. Feature flag the disconnect and simply log the block hash(es) when "scroll" feature is active.

Resolves #99

Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

I don't think the scroll feature flag is suitable here as it has implications for the state commitment types/operations. For example, the scroll feature flag is not compatible with the native mpt implementation.

I wonder if instead of introducing a feature flag we should see if we can modify the NetworkConfig to use PoW instead of PoS?

/// The default mode of the network.
pub network_mode: NetworkMode,

crates/net/network/src/manager.rs Outdated Show resolved Hide resolved
Signed-off-by: Gregory Edison <[email protected]>
…on `NewBlock` and `NewBlockHashes`

Signed-off-by: Gregory Edison <[email protected]>
@greged93 greged93 force-pushed the fix/dropped-connections branch from 5290b30 to 50cadec Compare January 6, 2025 08:43
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

Looks good! What are the implications here? Does this mean we can sync the node via the existing L2 P2P network after this change?

@greged93
Copy link
Collaborator Author

greged93 commented Jan 7, 2025

we could sync the node via the L2 P2P before this change, but we were getting a lot of disconnects due to the NewBlock and NewBlockHashes messages. With this fix, we don't get disconnected anymore.

@greged93 greged93 merged commit f364a2a into scroll Jan 7, 2025
43 checks passed
@greged93 greged93 deleted the fix/dropped-connections branch January 7, 2025 12:53
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.

Dropped network connections
2 participants