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

feat: add block propagation handlers #205

Merged
merged 1 commit into from Nov 15, 2022
Merged

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Nov 15, 2022

Adds missing block propagation handlers for NewBlock and NewBlogHashes.

While they're part of devp2 v67 they should be switched off for POS network: https://eips.ethereum.org/EIPS/eip-3675#devp2p

Since this is determined by the forkId we should add this as part of the is_valid_message,

@mattsse mattsse requested a review from Rjected November 15, 2022 12:09
@mattsse mattsse added the A-devp2p Related to the Ethereum P2P protocol label Nov 15, 2022
@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #205 (00bf63c) into main (b60ced1) will decrease coverage by 0.32%.
The diff coverage is 21.95%.

@@            Coverage Diff             @@
##             main     #205      +/-   ##
==========================================
- Coverage   68.69%   68.36%   -0.33%     
==========================================
  Files         213      213              
  Lines       17896    17974      +78     
==========================================
- Hits        12293    12288       -5     
- Misses       5603     5686      +83     
Impacted Files Coverage Δ
crates/net/network/src/fetch.rs 0.00% <0.00%> (ø)
crates/net/network/src/import.rs 0.00% <0.00%> (ø)
crates/net/network/src/manager.rs 0.00% <0.00%> (ø)
crates/net/network/src/message.rs 0.00% <0.00%> (ø)
crates/net/network/src/state.rs 0.00% <0.00%> (ø)
crates/net/network/src/swarm.rs 0.00% <0.00%> (ø)
crates/net/eth-wire/src/types/broadcast.rs 66.66% <100.00%> (+28.57%) ⬆️
crates/transaction-pool/src/test_util/mock.rs 63.04% <0.00%> (-7.83%) ⬇️
crates/transaction-pool/src/pool/txpool.rs 59.14% <0.00%> (-0.54%) ⬇️
crates/net/discv4/src/lib.rs 68.00% <0.00%> (-0.15%) ⬇️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@mattsse mattsse changed the title feat: add blog propagation handlers feat: add block propagation handlers Nov 15, 2022
Copy link
Member

@Rjected Rjected 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, although there is still no forkid for the merge yet afaik. for example geth drops block broadcasts when its merger.PoSFinalized() returns true. this is set by the handler for engine_forkchoiceUpdatedV1

@mattsse
Copy link
Collaborator Author

mattsse commented Nov 15, 2022

nice find, I will create an issue for this and think about a solution, where and how to configure this.

@mattsse mattsse merged commit 6b336c6 into main Nov 15, 2022
@mattsse mattsse deleted the matt/add-blog-propagation branch November 15, 2022 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devp2p Related to the Ethereum P2P protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants