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.

Sign up for GitHub

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

Jump to bottom

core/forkid: add two clauses for more precise validation #20220

Merged
merged 1 commit into from
Oct 30, 2019
Merged

core/forkid: add two clauses for more precise validation #20220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions core/forkid/forkid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,13 @@ func newFilter(config *params.ChainConfig, genesis common.Hash, headfn func() ui
// Create a validator that will filter out incompatible chains
return func(id ID) error {
// Run the fork checksum validation ruleset:
// 1. If local and remote FORK_CSUM matches, connect.
// 1. If local and remote FORK_CSUM matches, compare local head to FORK_NEXT.
// The two nodes are in the same fork state currently. They might know
// of differing future forks, but that's not relevant until the fork
// triggers (might be postponed, nodes might be updated to match).
// 1a. A remotely announced but remotely not passed block is already passed
// locally, disconnect, since the chains are incompatible.
// 1b. No remotely announced fork; or not yet passed locally, connect.
// 2. If the remote FORK_CSUM is a subset of the local past forks and the
// remote FORK_NEXT matches with the locally following fork block number,
// connect.
Expand All @@ -145,7 +148,12 @@ func newFilter(config *params.ChainConfig, genesis common.Hash, headfn func() ui
// Found the first unpassed fork block, check if our current state matches
// the remote checksum (rule #1).
if sums[i] == id.Hash {
// Yay, fork checksum matched, ignore any upcoming fork
// Fork checksum matched, check if a remote future fork block already passed
// locally without the local node being aware of it (rule #1a).
if id.Next > 0 && head >= id.Next {
return ErrLocalIncompatibleOrStale
}
// Haven't passed locally a remote-only fork, accept the connection (rule #1b).
return nil
}
// The local and remote nodes are in different forks currently, check if the
Expand Down
12 changes: 11 additions & 1 deletion core/forkid/forkid_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func TestValidation(t *testing.T) {

// Local is mainnet Petersburg, remote announces Byzantium + knowledge about Petersburg. Remote
// is simply out of sync, accept.
{7987396, ID{Hash: checksumToBytes(0x668db0af), Next: 7280000}, nil},
{7987396, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}, nil},

// Local is mainnet Petersburg, remote announces Spurious + knowledge about Byzantium. Remote
// is definitely out of sync. It may or may not need the Petersburg update, we don't know yet.
Expand All @@ -178,6 +178,16 @@ func TestValidation(t *testing.T) {

// Local is mainnet Petersburg, remote is Rinkeby Petersburg.
{7987396, ID{Hash: checksumToBytes(0xafec6b27), Next: 0}, ErrLocalIncompatibleOrStale},

// Local is mainnet Petersburg, far in the future. Remote announces Gopherium (non existing fork)
// at some future block 88888888, for itself, but past block for local. Local is incompatible.
//
// This case detects non-upgraded nodes with majority hash power (typical Ropsten mess).
{88888888, ID{Hash: checksumToBytes(0x668db0af), Next: 88888888}, ErrLocalIncompatibleOrStale},

// Local is mainnet Byzantium. Remote is also in Byzantium, but announces Gopherium (non existing
// fork) at block 7279999, before Petersburg. Local is incompatible.
{7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: 7279999}, ErrLocalIncompatibleOrStale},
}
for i, tt := range tests {
filter := newFilter(params.MainnetChainConfig, params.MainnetGenesisHash, func() uint64 { return tt.head })
Expand Down