From 94eb30eb60079cfdf3502586584b940d881cde47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 3 Jul 2019 19:13:26 +0300 Subject: [PATCH] core/forkid: change forkhash rlp encoding from int to [4]byte --- core/forkid/forkid.go | 21 +++++-- core/forkid/forkid_test.go | 125 ++++++++++++++++++++++--------------- 2 files changed, 90 insertions(+), 56 deletions(-) diff --git a/core/forkid/forkid.go b/core/forkid/forkid.go index 4b9663aa05..8c1700879a 100644 --- a/core/forkid/forkid.go +++ b/core/forkid/forkid.go @@ -46,8 +46,8 @@ var ( // ID is a fork identifier as defined by EIP-2124. type ID struct { - Hash uint32 // CRC32 checksum of the genesis block and passed fork block numbers - Next uint64 // Block number of the next upcoming fork, or 0 if no forks are known + Hash [4]byte // CRC32 checksum of the genesis block and passed fork block numbers + Next uint64 // Block number of the next upcoming fork, or 0 if no forks are known } // NewID calculates the Ethereum fork ID from the chain config and head. @@ -77,7 +77,7 @@ func newID(config *params.ChainConfig, genesis common.Hash, head uint64) ID { next = fork break } - return ID{Hash: hash, Next: next} + return ID{Hash: checksumToBytes(hash), Next: next} } // NewFilter creates an filter that returns if a fork ID should be rejected or not @@ -99,11 +99,13 @@ func newFilter(config *params.ChainConfig, genesis common.Hash, headfn func() ui // Calculate the all the valid fork hash and fork next combos var ( forks = gatherForks(config) - sums = make([]uint32, len(forks)+1) // 0th is the genesis + sums = make([][4]byte, len(forks)+1) // 0th is the genesis ) - sums[0] = crc32.ChecksumIEEE(genesis[:]) + hash := crc32.ChecksumIEEE(genesis[:]) + sums[0] = checksumToBytes(hash) for i, fork := range forks { - sums[i+1] = checksumUpdate(sums[i], fork) + hash = checksumUpdate(hash, fork) + sums[i+1] = checksumToBytes(hash) } // Add two sentries to simplify the fork checks and don't require special // casing the last one. @@ -182,6 +184,13 @@ func checksumUpdate(hash uint32, fork uint64) uint32 { return crc32.Update(hash, crc32.IEEETable, blob[:]) } +// checksumToBytes converts a uint32 checksum into a [4]byte array. +func checksumToBytes(hash uint32) [4]byte { + var blob [4]byte + binary.BigEndian.PutUint32(blob[:], hash) + return blob +} + // gatherForks gathers all the known forks and creates a sorted list out of them. func gatherForks(config *params.ChainConfig) []uint64 { // Gather all the fork block numbers via reflection diff --git a/core/forkid/forkid_test.go b/core/forkid/forkid_test.go index 48eaae8744..b33f85bec7 100644 --- a/core/forkid/forkid_test.go +++ b/core/forkid/forkid_test.go @@ -17,11 +17,13 @@ package forkid import ( + "bytes" "math" "testing" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/rlp" ) // TestCreation tests that different genesis and fork rule combinations result in @@ -41,20 +43,20 @@ func TestCreation(t *testing.T) { params.MainnetChainConfig, params.MainnetGenesisHash, []testcase{ - {0, ID{Hash: 0xfc64ec04, Next: 1150000}}, // Unsynced - {1149999, ID{Hash: 0xfc64ec04, Next: 1150000}}, // Last Frontier block - {1150000, ID{Hash: 0x97c2c34c, Next: 1920000}}, // First Homestead block - {1919999, ID{Hash: 0x97c2c34c, Next: 1920000}}, // Last Homestead block - {1920000, ID{Hash: 0x91d1f948, Next: 2463000}}, // First DAO block - {2462999, ID{Hash: 0x91d1f948, Next: 2463000}}, // Last DAO block - {2463000, ID{Hash: 0x7a64da13, Next: 2675000}}, // First Tangerine block - {2674999, ID{Hash: 0x7a64da13, Next: 2675000}}, // Last Tangerine block - {2675000, ID{Hash: 0x3edd5b10, Next: 4370000}}, // First Spurious block - {4369999, ID{Hash: 0x3edd5b10, Next: 4370000}}, // Last Spurious block - {4370000, ID{Hash: 0xa00bc324, Next: 7280000}}, // First Byzantium block - {7279999, ID{Hash: 0xa00bc324, Next: 7280000}}, // Last Byzantium block - {7280000, ID{Hash: 0x668db0af, Next: 0}}, // First and last Constantinople, first Petersburg block - {7987396, ID{Hash: 0x668db0af, Next: 0}}, // Today Petersburg block + {0, ID{Hash: checksumToBytes(0xfc64ec04), Next: 1150000}}, // Unsynced + {1149999, ID{Hash: checksumToBytes(0xfc64ec04), Next: 1150000}}, // Last Frontier block + {1150000, ID{Hash: checksumToBytes(0x97c2c34c), Next: 1920000}}, // First Homestead block + {1919999, ID{Hash: checksumToBytes(0x97c2c34c), Next: 1920000}}, // Last Homestead block + {1920000, ID{Hash: checksumToBytes(0x91d1f948), Next: 2463000}}, // First DAO block + {2462999, ID{Hash: checksumToBytes(0x91d1f948), Next: 2463000}}, // Last DAO block + {2463000, ID{Hash: checksumToBytes(0x7a64da13), Next: 2675000}}, // First Tangerine block + {2674999, ID{Hash: checksumToBytes(0x7a64da13), Next: 2675000}}, // Last Tangerine block + {2675000, ID{Hash: checksumToBytes(0x3edd5b10), Next: 4370000}}, // First Spurious block + {4369999, ID{Hash: checksumToBytes(0x3edd5b10), Next: 4370000}}, // Last Spurious block + {4370000, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}}, // First Byzantium block + {7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}}, // Last Byzantium block + {7280000, ID{Hash: checksumToBytes(0x668db0af), Next: 0}}, // First and last Constantinople, first Petersburg block + {7987396, ID{Hash: checksumToBytes(0x668db0af), Next: 0}}, // Today Petersburg block }, }, // Ropsten test cases @@ -62,16 +64,16 @@ func TestCreation(t *testing.T) { params.TestnetChainConfig, params.TestnetGenesisHash, []testcase{ - {0, ID{Hash: 0x30c7ddbc, Next: 10}}, // Unsynced, last Frontier, Homestead and first Tangerine block - {9, ID{Hash: 0x30c7ddbc, Next: 10}}, // Last Tangerine block - {10, ID{Hash: 0x63760190, Next: 1700000}}, // First Spurious block - {1699999, ID{Hash: 0x63760190, Next: 1700000}}, // Last Spurious block - {1700000, ID{Hash: 0x3ea159c7, Next: 4230000}}, // First Byzantium block - {4229999, ID{Hash: 0x3ea159c7, Next: 4230000}}, // Last Byzantium block - {4230000, ID{Hash: 0x97b544f3, Next: 4939394}}, // First Constantinople block - {4939393, ID{Hash: 0x97b544f3, Next: 4939394}}, // Last Constantinople block - {4939394, ID{Hash: 0xd6e2149b, Next: 0}}, // First Petersburg block - {5822692, ID{Hash: 0xd6e2149b, Next: 0}}, // Today Petersburg block + {0, ID{Hash: checksumToBytes(0x30c7ddbc), Next: 10}}, // Unsynced, last Frontier, Homestead and first Tangerine block + {9, ID{Hash: checksumToBytes(0x30c7ddbc), Next: 10}}, // Last Tangerine block + {10, ID{Hash: checksumToBytes(0x63760190), Next: 1700000}}, // First Spurious block + {1699999, ID{Hash: checksumToBytes(0x63760190), Next: 1700000}}, // Last Spurious block + {1700000, ID{Hash: checksumToBytes(0x3ea159c7), Next: 4230000}}, // First Byzantium block + {4229999, ID{Hash: checksumToBytes(0x3ea159c7), Next: 4230000}}, // Last Byzantium block + {4230000, ID{Hash: checksumToBytes(0x97b544f3), Next: 4939394}}, // First Constantinople block + {4939393, ID{Hash: checksumToBytes(0x97b544f3), Next: 4939394}}, // Last Constantinople block + {4939394, ID{Hash: checksumToBytes(0xd6e2149b), Next: 0}}, // First Petersburg block + {5822692, ID{Hash: checksumToBytes(0xd6e2149b), Next: 0}}, // Today Petersburg block }, }, // Rinkeby test cases @@ -79,17 +81,17 @@ func TestCreation(t *testing.T) { params.RinkebyChainConfig, params.RinkebyGenesisHash, []testcase{ - {0, ID{Hash: 0x3b8e0691, Next: 1}}, // Unsynced, last Frontier block - {1, ID{Hash: 0x60949295, Next: 2}}, // First and last Homestead block - {2, ID{Hash: 0x8bde40dd, Next: 3}}, // First and last Tangerine block - {3, ID{Hash: 0xcb3a64bb, Next: 1035301}}, // First Spurious block - {1035300, ID{Hash: 0xcb3a64bb, Next: 1035301}}, // Last Spurious block - {1035301, ID{Hash: 0x8d748b57, Next: 3660663}}, // First Byzantium block - {3660662, ID{Hash: 0x8d748b57, Next: 3660663}}, // Last Byzantium block - {3660663, ID{Hash: 0xe49cab14, Next: 4321234}}, // First Constantinople block - {4321233, ID{Hash: 0xe49cab14, Next: 4321234}}, // Last Constantinople block - {4321234, ID{Hash: 0xafec6b27, Next: 0}}, // First Petersburg block - {4586649, ID{Hash: 0xafec6b27, Next: 0}}, // Today Petersburg block + {0, ID{Hash: checksumToBytes(0x3b8e0691), Next: 1}}, // Unsynced, last Frontier block + {1, ID{Hash: checksumToBytes(0x60949295), Next: 2}}, // First and last Homestead block + {2, ID{Hash: checksumToBytes(0x8bde40dd), Next: 3}}, // First and last Tangerine block + {3, ID{Hash: checksumToBytes(0xcb3a64bb), Next: 1035301}}, // First Spurious block + {1035300, ID{Hash: checksumToBytes(0xcb3a64bb), Next: 1035301}}, // Last Spurious block + {1035301, ID{Hash: checksumToBytes(0x8d748b57), Next: 3660663}}, // First Byzantium block + {3660662, ID{Hash: checksumToBytes(0x8d748b57), Next: 3660663}}, // Last Byzantium block + {3660663, ID{Hash: checksumToBytes(0xe49cab14), Next: 4321234}}, // First Constantinople block + {4321233, ID{Hash: checksumToBytes(0xe49cab14), Next: 4321234}}, // Last Constantinople block + {4321234, ID{Hash: checksumToBytes(0xafec6b27), Next: 0}}, // First Petersburg block + {4586649, ID{Hash: checksumToBytes(0xafec6b27), Next: 0}}, // Today Petersburg block }, }, // Goerli test cases @@ -97,8 +99,8 @@ func TestCreation(t *testing.T) { params.GoerliChainConfig, params.GoerliGenesisHash, []testcase{ - {0, ID{Hash: 0xa3f5ab08, Next: 0}}, // Unsynced, last Frontier, Homestead, Tangerine, Spurious, Byzantium, Constantinople and first Petersburg block - {795329, ID{Hash: 0xa3f5ab08, Next: 0}}, // Today Petersburg block + {0, ID{Hash: checksumToBytes(0xa3f5ab08), Next: 0}}, // Unsynced, last Frontier, Homestead, Tangerine, Spurious, Byzantium, Constantinople and first Petersburg block + {795329, ID{Hash: checksumToBytes(0xa3f5ab08), Next: 0}}, // Today Petersburg block }, }, } @@ -120,56 +122,56 @@ func TestValidation(t *testing.T) { err error }{ // Local is mainnet Petersburg, remote announces the same. No future fork is announced. - {7987396, ID{Hash: 0x668db0af, Next: 0}, nil}, + {7987396, ID{Hash: checksumToBytes(0x668db0af), Next: 0}, nil}, // Local is mainnet Petersburg, remote announces the same. Remote also announces a next fork // at block 0xffffffff, but that is uncertain. - {7987396, ID{Hash: 0x668db0af, Next: math.MaxUint64}, nil}, + {7987396, ID{Hash: checksumToBytes(0x668db0af), Next: math.MaxUint64}, nil}, // Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces // also Byzantium, but it's not yet aware of Petersburg (e.g. non updated node before the fork). // In this case we don't know if Petersburg passed yet or not. - {7279999, ID{Hash: 0xa00bc324, Next: 0}, nil}, + {7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: 0}, nil}, // Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces // also Byzantium, and it's also aware of Petersburg (e.g. updated node before the fork). We // don't know if Petersburg passed yet (will pass) or not. - {7279999, ID{Hash: 0xa00bc324, Next: 7280000}, nil}, + {7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}, nil}, // Local is mainnet currently in Byzantium only (so it's aware of Petersburg), remote announces // also Byzantium, and it's also aware of some random fork (e.g. misconfigured Petersburg). As // neither forks passed at neither nodes, they may mismatch, but we still connect for now. - {7279999, ID{Hash: 0xa00bc324, Next: math.MaxUint64}, nil}, + {7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: math.MaxUint64}, nil}, // Local is mainnet Petersburg, remote announces Byzantium + knowledge about Petersburg. Remote // is simply out of sync, accept. - {7987396, ID{Hash: 0x668db0af, Next: 7280000}, nil}, + {7987396, ID{Hash: checksumToBytes(0x668db0af), 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. - {7987396, ID{Hash: 0x3edd5b10, Next: 4370000}, nil}, + {7987396, ID{Hash: checksumToBytes(0x3edd5b10), Next: 4370000}, nil}, // Local is mainnet Byzantium, remote announces Petersburg. Local is out of sync, accept. - {7279999, ID{Hash: 0x668db0af, Next: 0}, nil}, + {7279999, ID{Hash: checksumToBytes(0x668db0af), Next: 0}, nil}, // Local is mainnet Spurious, remote announces Byzantium, but is not aware of Petersburg. Local // out of sync. Local also knows about a future fork, but that is uncertain yet. - {4369999, ID{Hash: 0xa00bc324, Next: 0}, nil}, + {4369999, ID{Hash: checksumToBytes(0xa00bc324), Next: 0}, nil}, // Local is mainnet Petersburg. remote announces Byzantium but is not aware of further forks. // Remote needs software update. - {7987396, ID{Hash: 0xa00bc324, Next: 0}, ErrRemoteStale}, + {7987396, ID{Hash: checksumToBytes(0xa00bc324), Next: 0}, ErrRemoteStale}, // Local is mainnet Petersburg, and isn't aware of more forks. Remote announces Petersburg + // 0xffffffff. Local needs software update, reject. - {7987396, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, + {7987396, ID{Hash: checksumToBytes(0x5cddc0e1), Next: 0}, ErrLocalIncompatibleOrStale}, // Local is mainnet Byzantium, and is aware of Petersburg. Remote announces Petersburg + // 0xffffffff. Local needs software update, reject. - {7279999, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, + {7279999, ID{Hash: checksumToBytes(0x5cddc0e1), Next: 0}, ErrLocalIncompatibleOrStale}, // Local is mainnet Petersburg, remote is Rinkeby Petersburg. - {7987396, ID{Hash: 0xafec6b27, Next: 0}, ErrLocalIncompatibleOrStale}, + {7987396, ID{Hash: checksumToBytes(0xafec6b27), Next: 0}, ErrLocalIncompatibleOrStale}, } for i, tt := range tests { filter := newFilter(params.MainnetChainConfig, params.MainnetGenesisHash, func() uint64 { return tt.head }) @@ -178,3 +180,26 @@ func TestValidation(t *testing.T) { } } } + +// 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) + } + } +}