core/forkid: change forkhash rlp encoding from int to [4]byte

core/forkid/forkid.go

@@ -46,7 +46,7 @@ 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
+	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
 }
 
@@ -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

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
+				{0, ID{Hash: checksumToBytes(0xfc64ec04), Next: 1150000}},       // Unsynced
-				{1149999, ID{Hash: 0xfc64ec04, Next: 1150000}}, // Last Frontier block
+				{1149999, ID{Hash: checksumToBytes(0xfc64ec04), Next: 1150000}}, // Last Frontier block
-				{1150000, ID{Hash: 0x97c2c34c, Next: 1920000}}, // First Homestead block
+				{1150000, ID{Hash: checksumToBytes(0x97c2c34c), Next: 1920000}}, // First Homestead block
-				{1919999, ID{Hash: 0x97c2c34c, Next: 1920000}}, // Last Homestead block
+				{1919999, ID{Hash: checksumToBytes(0x97c2c34c), Next: 1920000}}, // Last Homestead block
-				{1920000, ID{Hash: 0x91d1f948, Next: 2463000}}, // First DAO block
+				{1920000, ID{Hash: checksumToBytes(0x91d1f948), Next: 2463000}}, // First DAO block
-				{2462999, ID{Hash: 0x91d1f948, Next: 2463000}}, // Last DAO block
+				{2462999, ID{Hash: checksumToBytes(0x91d1f948), Next: 2463000}}, // Last DAO block
-				{2463000, ID{Hash: 0x7a64da13, Next: 2675000}}, // First Tangerine block
+				{2463000, ID{Hash: checksumToBytes(0x7a64da13), Next: 2675000}}, // First Tangerine block
-				{2674999, ID{Hash: 0x7a64da13, Next: 2675000}}, // Last Tangerine block
+				{2674999, ID{Hash: checksumToBytes(0x7a64da13), Next: 2675000}}, // Last Tangerine block
-				{2675000, ID{Hash: 0x3edd5b10, Next: 4370000}}, // First Spurious block
+				{2675000, ID{Hash: checksumToBytes(0x3edd5b10), Next: 4370000}}, // First Spurious block
-				{4369999, ID{Hash: 0x3edd5b10, Next: 4370000}}, // Last Spurious block
+				{4369999, ID{Hash: checksumToBytes(0x3edd5b10), Next: 4370000}}, // Last Spurious block
-				{4370000, ID{Hash: 0xa00bc324, Next: 7280000}}, // First Byzantium block
+				{4370000, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}}, // First Byzantium block
-				{7279999, ID{Hash: 0xa00bc324, Next: 7280000}}, // Last Byzantium block
+				{7279999, ID{Hash: checksumToBytes(0xa00bc324), Next: 7280000}}, // Last Byzantium block
-				{7280000, ID{Hash: 0x668db0af, Next: 0}},       // First and last Constantinople, first Petersburg block
+				{7280000, ID{Hash: checksumToBytes(0x668db0af), Next: 0}},       // First and last Constantinople, first Petersburg block
-				{7987396, ID{Hash: 0x668db0af, Next: 0}},       // Today 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
+				{0, ID{Hash: checksumToBytes(0x30c7ddbc), Next: 10}},            // Unsynced, last Frontier, Homestead and first Tangerine block
-				{9, ID{Hash: 0x30c7ddbc, Next: 10}},            // Last Tangerine block
+				{9, ID{Hash: checksumToBytes(0x30c7ddbc), Next: 10}},            // Last Tangerine block
-				{10, ID{Hash: 0x63760190, Next: 1700000}},      // First Spurious block
+				{10, ID{Hash: checksumToBytes(0x63760190), Next: 1700000}},      // First Spurious block
-				{1699999, ID{Hash: 0x63760190, Next: 1700000}}, // Last Spurious block
+				{1699999, ID{Hash: checksumToBytes(0x63760190), Next: 1700000}}, // Last Spurious block
-				{1700000, ID{Hash: 0x3ea159c7, Next: 4230000}}, // First Byzantium block
+				{1700000, ID{Hash: checksumToBytes(0x3ea159c7), Next: 4230000}}, // First Byzantium block
-				{4229999, ID{Hash: 0x3ea159c7, Next: 4230000}}, // Last Byzantium block
+				{4229999, ID{Hash: checksumToBytes(0x3ea159c7), Next: 4230000}}, // Last Byzantium block
-				{4230000, ID{Hash: 0x97b544f3, Next: 4939394}}, // First Constantinople block
+				{4230000, ID{Hash: checksumToBytes(0x97b544f3), Next: 4939394}}, // First Constantinople block
-				{4939393, ID{Hash: 0x97b544f3, Next: 4939394}}, // Last Constantinople block
+				{4939393, ID{Hash: checksumToBytes(0x97b544f3), Next: 4939394}}, // Last Constantinople block
-				{4939394, ID{Hash: 0xd6e2149b, Next: 0}},       // First Petersburg block
+				{4939394, ID{Hash: checksumToBytes(0xd6e2149b), Next: 0}},       // First Petersburg block
-				{5822692, ID{Hash: 0xd6e2149b, Next: 0}},       // Today 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
+				{0, ID{Hash: checksumToBytes(0x3b8e0691), Next: 1}},             // Unsynced, last Frontier block
-				{1, ID{Hash: 0x60949295, Next: 2}},             // First and last Homestead block
+				{1, ID{Hash: checksumToBytes(0x60949295), Next: 2}},             // First and last Homestead block
-				{2, ID{Hash: 0x8bde40dd, Next: 3}},             // First and last Tangerine block
+				{2, ID{Hash: checksumToBytes(0x8bde40dd), Next: 3}},             // First and last Tangerine block
-				{3, ID{Hash: 0xcb3a64bb, Next: 1035301}},       // First Spurious block
+				{3, ID{Hash: checksumToBytes(0xcb3a64bb), Next: 1035301}},       // First Spurious block
-				{1035300, ID{Hash: 0xcb3a64bb, Next: 1035301}}, // Last Spurious block
+				{1035300, ID{Hash: checksumToBytes(0xcb3a64bb), Next: 1035301}}, // Last Spurious block
-				{1035301, ID{Hash: 0x8d748b57, Next: 3660663}}, // First Byzantium block
+				{1035301, ID{Hash: checksumToBytes(0x8d748b57), Next: 3660663}}, // First Byzantium block
-				{3660662, ID{Hash: 0x8d748b57, Next: 3660663}}, // Last Byzantium block
+				{3660662, ID{Hash: checksumToBytes(0x8d748b57), Next: 3660663}}, // Last Byzantium block
-				{3660663, ID{Hash: 0xe49cab14, Next: 4321234}}, // First Constantinople block
+				{3660663, ID{Hash: checksumToBytes(0xe49cab14), Next: 4321234}}, // First Constantinople block
-				{4321233, ID{Hash: 0xe49cab14, Next: 4321234}}, // Last Constantinople block
+				{4321233, ID{Hash: checksumToBytes(0xe49cab14), Next: 4321234}}, // Last Constantinople block
-				{4321234, ID{Hash: 0xafec6b27, Next: 0}},       // First Petersburg block
+				{4321234, ID{Hash: checksumToBytes(0xafec6b27), Next: 0}},       // First Petersburg block
-				{4586649, ID{Hash: 0xafec6b27, Next: 0}},       // Today 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
+				{0, ID{Hash: checksumToBytes(0xa3f5ab08), Next: 0}},      // Unsynced, last Frontier, Homestead, Tangerine, Spurious, Byzantium, Constantinople and first Petersburg block
-				{795329, ID{Hash: 0xa3f5ab08, Next: 0}}, // Today 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)
+		}
+	}
+}