core/rawdb: fix transaction indexing/unindexing hashing error (ethereum#22457)

* core/rawdb: more verbose error logs + better hashing

* core/rawdb: add failing testcase

* core/rawdb: properly hash transactions while indexing/unindexing

* core/rawdb: exit on error + better log msg

Author: holiman

core/rawdb/chain_iterator.go

@@ -23,10 +23,10 @@ import (
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/prque"
+	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/rlp"
-	"golang.org/x/crypto/sha3"
 )
 
 // InitDatabaseFromFreezer reinitializes an empty database from a previous batch
@@ -135,32 +135,15 @@ func iterateTransactions(db ethdb.Database, from uint64, to uint64, reverse bool
 				close(hashesCh)
 			}
 		}()
-
-		var hasher = sha3.NewLegacyKeccak256()
 		for data := range rlpCh {
-			it, err := rlp.NewListIterator(data.rlp)
-			if err != nil {
-				log.Warn("tx iteration error", "error", err)
-				return
-			}
-			it.Next()
-			txs := it.Value()
-			txIt, err := rlp.NewListIterator(txs)
-			if err != nil {
-				log.Warn("tx iteration error", "error", err)
+			var body types.Body
+			if err := rlp.DecodeBytes(data.rlp, &body); err != nil {
+				log.Warn("Failed to decode block body", "block", data.number, "error", err)
 				return
 			}
 			var hashes []common.Hash
-			for txIt.Next() {
-				if err := txIt.Err(); err != nil {
-					log.Warn("tx iteration error", "error", err)
-					return
-				}
-				var txHash common.Hash
-				hasher.Reset()
-				hasher.Write(txIt.Value())
-				hasher.Sum(txHash[:0])
-				hashes = append(hashes, txHash)
+			for _, tx := range body.Transactions {
+				hashes = append(hashes, tx.Hash())
 			}
 			result := &blockTxHashes{
 				hashes: hashes,

core/rawdb/chain_iterator_test.go

@@ -33,14 +33,34 @@ func TestChainIterator(t *testing.T) {
 
 	var block *types.Block
 	var txs []*types.Transaction
-	for i := uint64(0); i <= 10; i++ {
-		if i == 0 {
-			block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, nil, nil, nil, newHasher()) // Empty genesis block
+	to := common.BytesToAddress([]byte{0x11})
+	block = types.NewBlock(&types.Header{Number: big.NewInt(int64(0))}, nil, nil, nil, newHasher()) // Empty genesis block
+	WriteBlock(chainDb, block)
+	WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
+	for i := uint64(1); i <= 10; i++ {
+		var tx *types.Transaction
+		if i%2 == 0 {
+			tx = types.NewTx(&types.LegacyTx{
+				Nonce:    i,
+				GasPrice: big.NewInt(11111),
+				Gas:      1111,
+				To:       &to,
+				Value:    big.NewInt(111),
+				Data:     []byte{0x11, 0x11, 0x11},
+			})
 		} else {
-			tx := types.NewTransaction(i, common.BytesToAddress([]byte{0x11}), big.NewInt(111), 1111, big.NewInt(11111), []byte{0x11, 0x11, 0x11})
-			txs = append(txs, tx)
-			block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher())
+			tx = types.NewTx(&types.AccessListTx{
+				ChainID:  big.NewInt(1337),
+				Nonce:    i,
+				GasPrice: big.NewInt(11111),
+				Gas:      1111,
+				To:       &to,
+				Value:    big.NewInt(111),
+				Data:     []byte{0x11, 0x11, 0x11},
+			})
 		}
+		txs = append(txs, tx)
+		block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher())
 		WriteBlock(chainDb, block)
 		WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
 	}
@@ -66,7 +86,7 @@ func TestChainIterator(t *testing.T) {
 				numbers = append(numbers, int(h.number))
 				if len(h.hashes) > 0 {
 					if got, exp := h.hashes[0], txs[h.number-1].Hash(); got != exp {
-						t.Fatalf("hash wrong, got %x exp %x", got, exp)
+						t.Fatalf("block %d: hash wrong, got %x exp %x", h.number, got, exp)
 					}
 				}
 			}
@@ -88,14 +108,37 @@ func TestIndexTransactions(t *testing.T) {
 
 	var block *types.Block
 	var txs []*types.Transaction
-	for i := uint64(0); i <= 10; i++ {
-		if i == 0 {
-			block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, nil, nil, nil, newHasher()) // Empty genesis block
+	to := common.BytesToAddress([]byte{0x11})
+
+	// Write empty genesis block
+	block = types.NewBlock(&types.Header{Number: big.NewInt(int64(0))}, nil, nil, nil, newHasher())
+	WriteBlock(chainDb, block)
+	WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
+
+	for i := uint64(1); i <= 10; i++ {
+		var tx *types.Transaction
+		if i%2 == 0 {
+			tx = types.NewTx(&types.LegacyTx{
+				Nonce:    i,
+				GasPrice: big.NewInt(11111),
+				Gas:      1111,
+				To:       &to,
+				Value:    big.NewInt(111),
+				Data:     []byte{0x11, 0x11, 0x11},
+			})
 		} else {
-			tx := types.NewTransaction(i, common.BytesToAddress([]byte{0x11}), big.NewInt(111), 1111, big.NewInt(11111), []byte{0x11, 0x11, 0x11})
-			txs = append(txs, tx)
-			block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher())
+			tx = types.NewTx(&types.AccessListTx{
+				ChainID:  big.NewInt(1337),
+				Nonce:    i,
+				GasPrice: big.NewInt(11111),
+				Gas:      1111,
+				To:       &to,
+				Value:    big.NewInt(111),
+				Data:     []byte{0x11, 0x11, 0x11},
+			})
 		}
+		txs = append(txs, tx)
+		block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newHasher())
 		WriteBlock(chainDb, block)
 		WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
 	}
@@ -108,10 +151,10 @@ func TestIndexTransactions(t *testing.T) {
 			}
 			number := ReadTxLookupEntry(chainDb, txs[i-1].Hash())
 			if exist && number == nil {
-				t.Fatalf("Transaction indice missing")
+				t.Fatalf("Transaction index %d missing", i)
 			}
 			if !exist && number != nil {
-				t.Fatalf("Transaction indice is not deleted")
+				t.Fatalf("Transaction index %d is not deleted", i)
 			}
 		}
 		number := ReadTxIndexTail(chainDb)

rlp/iterator.go

@@ -23,6 +23,7 @@ type listIterator struct {
 }
 
 // NewListIterator creates an iterator for the (list) represented by data
+// TODO: Consider removing this implementation, as it is no longer used.
 func NewListIterator(data RawValue) (*listIterator, error) {
 	k, t, c, err := readKind(data)
 	if err != nil {