From 973bf81ae1238387c2063f3106db19b19b12fb77 Mon Sep 17 00:00:00 2001 From: Shane Bammel Date: Tue, 18 May 2021 03:37:18 -0500 Subject: [PATCH] fix(core/forkid): fix off-by-one bug #22879 * forkid: added failing test * forkid: fixed off-by-one bug --- core/forkid/forkid.go | 2 +- core/forkid/forkid_test.go | 292 +++++++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+), 1 deletion(-) diff --git a/core/forkid/forkid.go b/core/forkid/forkid.go index a3eb261034fa..9ac3371eb219 100644 --- a/core/forkid/forkid.go +++ b/core/forkid/forkid.go @@ -136,7 +136,7 @@ func newFilter(config *params.ChainConfig, genesis common.Hash, headfn func() ui for i, fork := range forks { // If our head is beyond this fork, continue to the next (we have a dummy // fork of maxuint64 as the last item to always fail this check eventually). - if head > fork { + if head >= fork { continue } // Found the first unpassed fork block, check if our current state matches diff --git a/core/forkid/forkid_test.go b/core/forkid/forkid_test.go index e1715c94fcb3..832626e2bdff 100644 --- a/core/forkid/forkid_test.go +++ b/core/forkid/forkid_test.go @@ -17,15 +17,307 @@ package forkid import ( + "bytes" "hash/crc32" + "math" "math/big" "testing" + "github.com/XinFinOrg/XDPoSChain/common" "github.com/XinFinOrg/XDPoSChain/core/types" "github.com/XinFinOrg/XDPoSChain/params" + "github.com/XinFinOrg/XDPoSChain/rlp" "github.com/stretchr/testify/assert" ) +// newIDFromHash is a test helper that calculates fork ID from a genesis hash instead of block +func newIDFromHash(config *params.ChainConfig, genesisHash common.Hash, head uint64) ID { + // Calculate the starting checksum from the genesis hash + hash := crc32.ChecksumIEEE(genesisHash.Bytes()) + + // Calculate the current fork checksum and the next fork block + forksByBlock := config.GatherForks() + for _, fork := range forksByBlock { + if fork <= head { + // Fork already passed, checksum the previous hash and the fork number + hash = checksumUpdate(hash, fork) + continue + } + return ID{Hash: checksumToBytes(hash), Next: fork} + } + return ID{Hash: checksumToBytes(hash), Next: 0} +} + +// TestCreation tests that different genesis and fork rule combinations result in +// the correct fork ID. +func TestCreation(t *testing.T) { + type testcase struct { + head uint64 + want ID + } + tests := []struct { + config *params.ChainConfig + hash common.Hash + cases []testcase + }{ + // Mainnet test cases - XDPoSChain specific fork checksums (BigEndian) + { + params.MainnetChainConfig, + params.MainnetGenesisHash, + []testcase{ + // Genesis and early blocks + {0, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}}, // Genesis block + {100000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}}, // Early block before first fork + {1000000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}}, // Block approaching first fork + // First fork (1150000) + {1149999, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}}, // Last block before first fork + {1150000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}}, // First block after first fork + {1500000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}}, // Block between first and second fork + // Second fork (1920000) + {1919999, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}}, // Last block before second fork + {1920000, ID{Hash: checksumToBytes(0x1c7c371f), Next: 2463000}}, // First block after second fork + {2000000, ID{Hash: checksumToBytes(0x1c7c371f), Next: 2463000}}, // Block between second and third fork + // Third fork (2463000) + {2462999, ID{Hash: checksumToBytes(0x1c7c371f), Next: 2463000}}, // Last block before third fork + {2463000, ID{Hash: checksumToBytes(0x5551b209), Next: 2675000}}, // First block after third fork + {2500000, ID{Hash: checksumToBytes(0x5551b209), Next: 2675000}}, // Block between third and fourth fork + // Fourth fork (2675000) + {2674999, ID{Hash: checksumToBytes(0x5551b209), Next: 2675000}}, // Last block before fourth fork + {2675000, ID{Hash: checksumToBytes(0x7cd95aba), Next: 4370000}}, // First block after fourth fork + {3000000, ID{Hash: checksumToBytes(0x7cd95aba), Next: 4370000}}, // Block between fourth and final fork + // Final fork (4370000) + {4369999, ID{Hash: checksumToBytes(0x7cd95aba), Next: 4370000}}, // Last block before final fork + {4370000, ID{Hash: checksumToBytes(0x84537aeb), Next: 0}}, // First block after final fork + {5000000, ID{Hash: checksumToBytes(0x84537aeb), Next: 0}}, // Future block after all forks + {10000000, ID{Hash: checksumToBytes(0x84537aeb), Next: 0}}, // Far future block + }, + }, + // Testnet (Apothem) test cases + { + params.TestnetChainConfig, + params.TestnetGenesisHash, + []testcase{ + // Genesis block + {0, ID{Hash: checksumToBytes(0x6627b1ab), Next: 1}}, // Genesis + // Fork at block 1 + {1, ID{Hash: checksumToBytes(0x6fdbe752), Next: 2}}, // After fork 1 + // Fork at block 2 + {2, ID{Hash: checksumToBytes(0x39b3de17), Next: 3}}, // After fork 2 + // Fork at block 3 + {3, ID{Hash: checksumToBytes(0x41c675d5), Next: 4}}, // After fork 3 + // Fork at block 4 + {4, ID{Hash: checksumToBytes(0x30c010d4), Next: 3000000}}, // After fork 4 + // Fork at block 3000000 + {2500000, ID{Hash: checksumToBytes(0x30c010d4), Next: 3000000}}, // Before fork 3000000 + {2999999, ID{Hash: checksumToBytes(0x30c010d4), Next: 3000000}}, // Just before fork 3000000 + {3000000, ID{Hash: checksumToBytes(0x0633fab8), Next: 3464000}}, // After fork 3000000 + {3200000, ID{Hash: checksumToBytes(0x0633fab8), Next: 3464000}}, // Between fork 3000000 and 3464000 + // Fork at block 3464000 + {3463999, ID{Hash: checksumToBytes(0x0633fab8), Next: 3464000}}, // Just before fork 3464000 + {3464000, ID{Hash: checksumToBytes(0x9ae3e747), Next: 5000000}}, // After fork 3464000 + // Fork at block 5000000 + {4000000, ID{Hash: checksumToBytes(0x9ae3e747), Next: 5000000}}, // Before fork 5000000 + {4999999, ID{Hash: checksumToBytes(0x9ae3e747), Next: 5000000}}, // Just before fork 5000000 + {5000000, ID{Hash: checksumToBytes(0x97ca7963), Next: 23779191}}, // After fork 5000000 + // Fork at block 23779191 + {23000000, ID{Hash: checksumToBytes(0x97ca7963), Next: 23779191}}, // Before fork 23779191 + {23779190, ID{Hash: checksumToBytes(0x97ca7963), Next: 23779191}}, // Just before fork 23779191 + {23779191, ID{Hash: checksumToBytes(0x6c07b5cf), Next: 56828700}}, // After fork 23779191 + // Fork at block 56828700 + {50000000, ID{Hash: checksumToBytes(0x6c07b5cf), Next: 56828700}}, // Before fork 56828700 + {56828699, ID{Hash: checksumToBytes(0x6c07b5cf), Next: 56828700}}, // Just before fork 56828700 + {56828700, ID{Hash: checksumToBytes(0x07a2b1b8), Next: 61290000}}, // After fork 56828700 + // Fork at block 61290000 + {60000000, ID{Hash: checksumToBytes(0x07a2b1b8), Next: 61290000}}, // Before fork 61290000 + {61289999, ID{Hash: checksumToBytes(0x07a2b1b8), Next: 61290000}}, // Just before fork 61290000 + {61290000, ID{Hash: checksumToBytes(0x5aee5f80), Next: 66825000}}, // After fork 61290000 + // Fork at block 66825000 + {66824999, ID{Hash: checksumToBytes(0x5aee5f80), Next: 66825000}}, // Just before fork 66825000 + {66825000, ID{Hash: checksumToBytes(0x6e0b9072), Next: 71550000}}, // After fork 66825000 + // Fork at block 71550000 + {71549999, ID{Hash: checksumToBytes(0x6e0b9072), Next: 71550000}}, // Just before fork 71550000 + {71550000, ID{Hash: checksumToBytes(0x5cab807f), Next: 71551800}}, // After fork 71550000 + // Fork at block 71551800 + {71551799, ID{Hash: checksumToBytes(0x5cab807f), Next: 71551800}}, // Just before fork 71551800 + {71551800, ID{Hash: checksumToBytes(0xebce6795), Next: 83600000}}, // After fork 71551800 + // Final fork at block 83600000 + {83599999, ID{Hash: checksumToBytes(0xebce6795), Next: 83600000}}, // Just before final fork + {83600000, ID{Hash: checksumToBytes(0x7fcfc8b0), Next: 0}}, // After final fork + {90000000, ID{Hash: checksumToBytes(0x7fcfc8b0), Next: 0}}, // Future block after all forks + }, + }, + } + for i, tt := range tests { + for j, ttt := range tt.cases { + if have := newIDFromHash(tt.config, tt.hash, ttt.head); have != ttt.want { + t.Errorf("test %d, case %d: fork ID mismatch: have %x, want %x", i, j, have, ttt.want) + } + } + } +} + +// TestValidation tests that a local peer correctly validates and accepts a remote +// fork ID. +func TestValidation(t *testing.T) { + // Config for mainnet with XDPoSChain fork values + config := params.MainnetChainConfig + + tests := []struct { + config *params.ChainConfig + head uint64 + id ID + err error + }{ + //----------------------------------- + // Local and remote at same state + //----------------------------------- + + // Local is mainnet after final fork, remote announces the same. No future fork is announced. + {config, 4370000, ID{Hash: checksumToBytes(0x84537aeb), Next: 0}, nil}, + + // Local is mainnet after final fork, remote announces the same. Remote also announces a next fork + // at block 0xffffffff, but that is uncertain. + {config, 4370000, ID{Hash: checksumToBytes(0x84537aeb), Next: math.MaxUint64}, nil}, + + //----------------------------------- + // Local and remote before first fork + //----------------------------------- + + // Local is mainnet currently before any fork, remote announces the same. No future fork is announced. + {config, 0, ID{Hash: checksumToBytes(0x0f331419), Next: 0}, nil}, + + // Local is mainnet currently before any fork, remote announces also before fork, but with future fork + {config, 100000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}, nil}, + + // Local is mainnet before fork, remote announces before fork but without awareness of future fork + {config, 100000, ID{Hash: checksumToBytes(0x0f331419), Next: 0}, nil}, + + //----------------------------------- + // Local and remote between forks + //----------------------------------- + + // Local is mainnet at first fork (1150000), remote announces genesis + knowledge about first fork. + // Remote is simply out of sync, accept. + {config, 1150000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}, nil}, + + // Local is mainnet between first and second fork, remote announces same block-based state + {config, 1500000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}, nil}, + + // Local is mainnet after first fork, remote announces genesis state - remote is behind but compatible + {config, 1500000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}, nil}, + + // Local is mainnet between forks, remote announces prior fork state - remote is far behind + {config, 2000000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}, nil}, + + //----------------------------------- + // Remote stale cases + //----------------------------------- + + // Local is mainnet after second fork, remote announces first fork state without next boundary. + // Remote needs software update. + {config, 2000000, ID{Hash: checksumToBytes(0x39f281ca), Next: 0}, ErrRemoteStale}, + + // Local is mainnet at third fork, remote announces second fork state without awareness of further forks. + // Remote needs software update. + {config, 2463000, ID{Hash: checksumToBytes(0x1c7c371f), Next: 0}, ErrRemoteStale}, + + // Local is mainnet after final fork, remote announces genesis state without any next fork awareness. + // Remote is severely outdated. + {config, 4370000, ID{Hash: checksumToBytes(0x0f331419), Next: 0}, ErrRemoteStale}, + + // Local is mainnet after final fork, remote announces first fork without higher fork awareness. + // Remote needs software update. + {config, 4370000, ID{Hash: checksumToBytes(0x39f281ca), Next: 0}, ErrRemoteStale}, + + //----------------------------------- + // Local out of sync cases (future forks unknown to local) + //----------------------------------- + + // Local is mainnet before first fork, remote announces after first fork. + // Local is out of sync, accept. + {config, 100000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}, nil}, + + // Local is mainnet at first fork, remote announces after second fork. + // Local is out of sync, accept. + {config, 1150000, ID{Hash: checksumToBytes(0x1c7c371f), Next: 2463000}, nil}, + + // Local is mainnet between forks, remote announces state we haven't reached yet + {config, 1500000, ID{Hash: checksumToBytes(0x1c7c371f), Next: 2463000}, nil}, + + //----------------------------------- + // Incompatible fork cases + //----------------------------------- + + // Remote has unknown checksum - should reject + {config, 0, ID{Hash: checksumToBytes(0xffffffff), Next: 0}, ErrLocalIncompatibleOrStale}, + + // Local is mainnet between forks, remote has completely different checksum + {config, 1500000, ID{Hash: checksumToBytes(0xdeadbeef), Next: 1920000}, ErrLocalIncompatibleOrStale}, + + // Remote is showing incompatible intermediate state + {config, 2000000, ID{Hash: checksumToBytes(0x12345678), Next: 2463000}, ErrLocalIncompatibleOrStale}, + + // Local is mainnet at final fork, remote is on incompatible chain + {config, 4370000, ID{Hash: checksumToBytes(0xabcdef00), Next: 0}, ErrLocalIncompatibleOrStale}, + + // Local is mainnet between forks, remote has fake future fork + {config, 2000000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1920000}, nil}, + + // Local is mainnet far in the future. Remote announces fake fork at some future block, + // but past block for local. Remote's checksum matches our final state, so it's acceptable. + {config, 10000000, ID{Hash: checksumToBytes(0x84537aeb), Next: 5000000}, nil}, + + // Local is mainnet between forks. Remote announces same state but with invalid next fork. + // This is tricky - same checksum means same fork history, but next differs + {config, 1500000, ID{Hash: checksumToBytes(0x39f281ca), Next: 1900000}, nil}, // Close enough, might be syncing + + // Remote has higher fork awareness but wrong checksum - definitely incompatible + {config, 1500000, ID{Hash: checksumToBytes(0xffffffff), Next: 1920000}, ErrLocalIncompatibleOrStale}, + + //----------------------------------- + // Edge cases + //----------------------------------- + + // Local is at genesis, remote at genesis - perfect match + {config, 0, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}, nil}, + + // Local way ahead of any forks, remote at genesis but aware of first fork + {config, 100000000, ID{Hash: checksumToBytes(0x0f331419), Next: 1150000}, nil}, + + // Local and remote both at final fork with same state + {config, 10000000, ID{Hash: checksumToBytes(0x84537aeb), Next: 0}, nil}, + } + for i, tt := range tests { + filter := newFilter(tt.config, params.MainnetGenesisHash, func() uint64 { return tt.head }) + if err := filter(tt.id); err != tt.err { + t.Errorf("test %d: validation error mismatch: have %v, want %v", i, err, tt.err) + } + } +} + +// TestEncoding tests that IDs are properly RLP encoded (specifically important because we +// use uint32 to store the hash, but we need to encode it as [4]byte). +func TestEncoding(t *testing.T) { + tests := []struct { + id ID + want []byte + }{ + {ID{Hash: checksumToBytes(0), Next: 0}, common.Hex2Bytes("c6840000000080")}, + {ID{Hash: checksumToBytes(0xdeadbeef), Next: 0xBADDCAFE}, common.Hex2Bytes("ca84deadbeef84baddcafe")}, + {ID{Hash: checksumToBytes(math.MaxUint32), Next: math.MaxUint64}, common.Hex2Bytes("ce84ffffffff88ffffffffffffffff")}, + } + for i, tt := range tests { + have, err := rlp.EncodeToBytes(tt.id) + if err != nil { + t.Errorf("test %d: failed to encode forkid: %v", i, err) + continue + } + if !bytes.Equal(have, tt.want) { + t.Errorf("test %d: RLP mismatch: have %x, want %x", i, have, tt.want) + } + } +} + func TestNewID(t *testing.T) { genesis := types.NewBlockWithHeader(&types.Header{Number: big.NewInt(0)}) config := ¶ms.ChainConfig{