fix(net): set status, forkfilter from chainspec #939
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* previously, if unset, the fork_filter would be set using the ChainSpec and head. if the status was set, but the fork_filter was not set, the fork filter and generated status's forkid would not match. this would lead to unintentional disconnects by us on custom networks. * remove the status and fork_filter fields from the network config builder * changes the head field to BlockHashNumber * if unset, set the head hash to the ChainSpec genesis, and the head number to zero * derive the Status and ForkFilter based on the current head and ChainSpec * add a test checking that the forkid hash and next are set to the correct values, based on the chainspec * set serde as a default feature for reth-network
Rjected
added
A-networking
Rjected
added a commit
that referenced
this pull request
Jan 20, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 20, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
18 tasks
mattsse
approved these changes
Jan 20, 2023
| @@ -94,5 +94,6 @@ tempfile = "3.3" | |||
| serial_test = "0.10" | |||
|
|
|||
| [features] | |||
| default = ["serde"] | |||
There was a problem hiding this comment.
I think this is reasonable
Rjected
added a commit
that referenced
this pull request
Jan 20, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 22, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 22, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 22, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 27, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 30, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 30, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
Rjected
added a commit
that referenced
this pull request
Jan 30, 2023
* this change and #939 should fix an issue where the fork filter and status were set incorrectly, causing a premature disconnect
literallymarvellous
pushed a commit
to literallymarvellous/reth
that referenced
this pull request
Feb 5, 2023
literallymarvellous
pushed a commit
to literallymarvellous/reth
that referenced
this pull request
Feb 5, 2023
literallymarvellous
pushed a commit
to literallymarvellous/reth
that referenced
this pull request
Feb 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if unset, the
fork_filterfor theNetworkConfigwould be set using thechain_specandheadfields. Thestatuswould be its default value, but thefork_filterbeing generated from the chainspec. As a result, the fork filter would not match the status. This would lead to unintentional disconnects by us on custom networks.This removes the
statusandfork_filterfields from the network config builder, and changes theheadfield type toBlockHashNumber. If unset, the head hash is set to the genesis hash, and the head number set to zero. TheStatusandForkFilterare then set based on this head and existing chainspec.This adds a test checking that the forkid
hashandnextare set to the correct values, based on the chainspec.Sets
serdeas a default feature forreth-network.