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

This commit is contained in:
Péter Szilágyi 2019-10-30 13:05:31 +02:00 committed by Felix Lange
parent e306304414
commit 9641cacea8
2 changed files with 21 additions and 3 deletions

View File

@ -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.
@ -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

View File

@ -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.
@ -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 })