Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core: panic if txs and receipts don't match in types.NewBlock(..) #29501

Closed
wants to merge 1 commit into from
+75 −46
Closed

core: panic if txs and receipts don't match in types.NewBlock(..) #29501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion core/rawdb/accessors_indexes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ func TestLookupStorage(t *testing.T) {
tx2 := types.NewTransaction(2, common.BytesToAddress([]byte{0x22}), big.NewInt(222), 2222, big.NewInt(22222), []byte{0x22, 0x22, 0x22})
tx3 := types.NewTransaction(3, common.BytesToAddress([]byte{0x33}), big.NewInt(333), 3333, big.NewInt(33333), []byte{0x33, 0x33, 0x33})
txs := []*types.Transaction{tx1, tx2, tx3}
receipts := []*types.Receipt{{}, {}, {}}

block := types.NewBlock(&types.Header{Number: big.NewInt(314)}, txs, nil, nil, newTestHasher())
block := types.NewBlock(&types.Header{Number: big.NewInt(314)}, txs, nil, receipts, newTestHasher())

// Check that no transactions entries are in a pristine database
for i, tx := range txs {
Expand Down
18 changes: 14 additions & 4 deletions core/rawdb/chain_iterator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ func TestChainIterator(t *testing.T) {
WriteBlock(chainDb, block)
WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
for i := uint64(1); i <= 10; i++ {
var tx *types.Transaction
var (
tx *types.Transaction
receipt *types.Receipt
)
if i%2 == 0 {
tx = types.NewTx(&types.LegacyTx{
Nonce: i,
Expand All @@ -48,6 +51,7 @@ func TestChainIterator(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{}
} else {
tx = types.NewTx(&types.AccessListTx{
ChainID: big.NewInt(1337),
Expand All @@ -58,9 +62,10 @@ func TestChainIterator(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{Type: types.AccessListTxType}
}
txs = append(txs, tx)
block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newTestHasher())
block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, types.Receipts{receipt}, newTestHasher())
WriteBlock(chainDb, block)
WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
}
Expand Down Expand Up @@ -116,7 +121,10 @@ func TestIndexTransactions(t *testing.T) {
WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())

for i := uint64(1); i <= 10; i++ {
var tx *types.Transaction
var (
tx *types.Transaction
receipt *types.Receipt
)
if i%2 == 0 {
tx = types.NewTx(&types.LegacyTx{
Nonce: i,
Expand All @@ -126,6 +134,7 @@ func TestIndexTransactions(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{}
} else {
tx = types.NewTx(&types.AccessListTx{
ChainID: big.NewInt(1337),
Expand All @@ -136,9 +145,10 @@ func TestIndexTransactions(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{Type: types.AccessListTxType}
}
txs = append(txs, tx)
block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, nil, newTestHasher())
block = types.NewBlock(&types.Header{Number: big.NewInt(int64(i))}, []*types.Transaction{tx}, nil, types.Receipts{receipt}, newTestHasher())
WriteBlock(chainDb, block)
WriteCanonicalHash(chainDb, block.Hash(), block.NumberU64())
}
Expand Down
5 changes: 4 additions & 1 deletion core/types/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,12 @@ type extblock struct {
// are ignored and set to values derived from the given txs, uncles
// and receipts.
func NewBlock(header *Header, txs []*Transaction, uncles []*Header, receipts []*Receipt, hasher TrieHasher) *Block {
if len(txs) != len(receipts) {
panic("cannot make new block with mismatched number of txs and receipts")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been discussing that this feels a bit drastic, given that we use the method in multiple places. Whilst this should not really happen - and a panic might be warranted in the miner or such packages - in other places it's less drastic (e.g. network piecing together bits and bobs of a block). Whilst true that it should not call with junk params, it's less clear that a hard panic would be the best course of action...

CC @fjl?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could soften it with a log.Error, but yeah, we should decide if we really want to go nuclear here and if we feel confident about not blowing up anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! Fine either way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not panic here. It might be useful, e.g. for creating blocks in various test-scenarios, to have mismatched txs/receipts (but still correctly derived receiptsroot etc). So 👍 on "stern warning" but from me 👎 on "hard crash"

}

b := &Block{header: CopyHeader(header)}

// TODO: panic if len(txs) != len(receipts)
if len(txs) == 0 {
b.header.TxHash = EmptyTxsHash
} else {
Expand Down
37 changes: 17 additions & 20 deletions eth/catalyst/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1538,28 +1538,25 @@ func equalBody(a *types.Body, b *engine.ExecutionPayloadBodyV1) bool {
}

func TestBlockToPayloadWithBlobs(t *testing.T) {
header := types.Header{}
var txs []*types.Transaction

inner := types.BlobTx{
BlobHashes: make([]common.Hash, 1),
}
var (
header = types.Header{}
tx = types.NewTx(&types.BlobTx{
BlobHashes: make([]common.Hash, 1),
})
receipt = &types.Receipt{Type: types.BlobTxType}
sidecars = []*types.BlobTxSidecar{
{
Blobs: make([]kzg4844.Blob, 1),
Commitments: make([]kzg4844.Commitment, 1),
Proofs: make([]kzg4844.Proof, 1),
},
}
block = types.NewBlock(&header, types.Transactions{tx}, nil, types.Receipts{receipt}, trie.NewStackTrie(nil))

txs = append(txs, types.NewTx(&inner))
sidecars := []*types.BlobTxSidecar{
{
Blobs: make([]kzg4844.Blob, 1),
Commitments: make([]kzg4844.Commitment, 1),
Proofs: make([]kzg4844.Proof, 1),
},
}
want = len(tx.BlobHashes())
envelope = engine.BlockToExecutableData(block, nil, sidecars)
)

block := types.NewBlock(&header, txs, nil, nil, trie.NewStackTrie(nil))
envelope := engine.BlockToExecutableData(block, nil, sidecars)
var want int
for _, tx := range txs {
want += len(tx.BlobHashes())
}
if got := len(envelope.BlobsBundle.Commitments); got != want {
t.Fatalf("invalid number of commitments: got %v, want %v", got, want)
}
Expand Down
6 changes: 4 additions & 2 deletions eth/filters/filter_system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,8 @@ func TestPendingLogsSubscription(t *testing.T) {
}
}

// TestLightFilterLogs ensures that filters against FilterAPI in light mode work
// correctly.
func TestLightFilterLogs(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -849,10 +851,10 @@ func TestLightFilterLogs(t *testing.T) {
if i == 0 {
return
}
tx, _ := types.SignTx(types.NewTx(&types.LegacyTx{Nonce: uint64(i - 1), To: &common.Address{}, Value: big.NewInt(1000), Gas: params.TxGas, GasPrice: b.BaseFee(), Data: nil}), signer, key)
b.AddUncheckedTx(tx)
receipts[i-1].Bloom = types.CreateBloom(types.Receipts{receipts[i-1]})
b.AddUncheckedReceipt(receipts[i-1])
tx, _ := types.SignTx(types.NewTx(&types.LegacyTx{Nonce: uint64(i - 1), To: &common.Address{}, Value: big.NewInt(1000), Gas: params.TxGas, GasPrice: b.BaseFee(), Data: nil}), signer, key)
b.AddTx(tx)
})
for i, block := range blocks {
rawdb.WriteBlock(db, block)
Expand Down
44 changes: 30 additions & 14 deletions internal/ethapi/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1319,11 +1319,14 @@ func hex2Bytes(str string) *hexutil.Bytes {
func TestRPCMarshalBlock(t *testing.T) {
t.Parallel()
var (
txs []*types.Transaction
to = common.BytesToAddress([]byte{0x11})
txs []*types.Transaction
receipts []*types.Receipt
to = common.BytesToAddress([]byte{0x11})
)
for i := uint64(1); i <= 4; i++ {
var tx *types.Transaction
var receipt *types.Receipt

if i%2 == 0 {
tx = types.NewTx(&types.LegacyTx{
Nonce: i,
Expand All @@ -1333,6 +1336,7 @@ func TestRPCMarshalBlock(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{}
} else {
tx = types.NewTx(&types.AccessListTx{
ChainID: big.NewInt(1337),
Expand All @@ -1343,10 +1347,12 @@ func TestRPCMarshalBlock(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{Type: types.AccessListTxType}
}
txs = append(txs, tx)
receipts = append(receipts, receipt)
}
block := types.NewBlock(&types.Header{Number: big.NewInt(100)}, txs, nil, nil, blocktest.NewHasher())
block := types.NewBlock(&types.Header{Number: big.NewInt(100)}, txs, nil, receipts, blocktest.NewHasher())

var testSuite = []struct {
inclTx bool
Expand All @@ -1362,14 +1368,14 @@ func TestRPCMarshalBlock(t *testing.T) {
"extraData": "0x",
"gasLimit": "0x0",
"gasUsed": "0x0",
"hash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"hash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"miner": "0x0000000000000000000000000000000000000000",
"mixHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"nonce": "0x0000000000000000",
"number": "0x64",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0xf64e8de40892653b3efce13a5c98babbc9e8f5fe773bd7a4236ce8527e8a7448",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"size": "0x296",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
Expand All @@ -1387,14 +1393,14 @@ func TestRPCMarshalBlock(t *testing.T) {
"extraData": "0x",
"gasLimit": "0x0",
"gasUsed": "0x0",
"hash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"hash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"miner": "0x0000000000000000000000000000000000000000",
"mixHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"nonce": "0x0000000000000000",
"number": "0x64",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0xf64e8de40892653b3efce13a5c98babbc9e8f5fe773bd7a4236ce8527e8a7448",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"size": "0x296",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
Expand All @@ -1418,21 +1424,21 @@ func TestRPCMarshalBlock(t *testing.T) {
"extraData": "0x",
"gasLimit": "0x0",
"gasUsed": "0x0",
"hash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"hash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
"miner": "0x0000000000000000000000000000000000000000",
"mixHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"nonce": "0x0000000000000000",
"number": "0x64",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0xf64e8de40892653b3efce13a5c98babbc9e8f5fe773bd7a4236ce8527e8a7448",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"size": "0x296",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
"timestamp": "0x0",
"transactions": [
{
"blockHash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"blockHash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"blockNumber": "0x64",
"from": "0x0000000000000000000000000000000000000000",
"gas": "0x457",
Expand All @@ -1452,7 +1458,7 @@ func TestRPCMarshalBlock(t *testing.T) {
"yParity": "0x0"
},
{
"blockHash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"blockHash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"blockNumber": "0x64",
"from": "0x0000000000000000000000000000000000000000",
"gas": "0x457",
Expand All @@ -1470,7 +1476,7 @@ func TestRPCMarshalBlock(t *testing.T) {
"s": "0x0"
},
{
"blockHash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"blockHash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"blockNumber": "0x64",
"from": "0x0000000000000000000000000000000000000000",
"gas": "0x457",
Expand All @@ -1490,7 +1496,7 @@ func TestRPCMarshalBlock(t *testing.T) {
"yParity": "0x0"
},
{
"blockHash": "0x9b73c83b25d0faf7eab854e3684c7e394336d6e135625aafa5c183f27baa8fee",
"blockHash": "0xa183006a06975781325079fe27341461ea0afc81a45cff88265666b418ec6555",
"blockNumber": "0x64",
"from": "0x0000000000000000000000000000000000000000",
"gas": "0x457",
Expand Down Expand Up @@ -1525,6 +1531,9 @@ func TestRPCMarshalBlock(t *testing.T) {
}
}

// TestRPCGetBlockOrHeader tests many different scenarios for GetBlockBy* and GetHeaderBy*.
//
// note: to regenerate the tests, set the env var WRITE_TEST_FILES.
func TestRPCGetBlockOrHeader(t *testing.T) {
t.Parallel()

Expand All @@ -1551,13 +1560,14 @@ func TestRPCGetBlockOrHeader(t *testing.T) {
Value: big.NewInt(111),
Data: []byte{0x11, 0x11, 0x11},
})
receipt = &types.Receipt{}
withdrawal = &types.Withdrawal{
Index: 0,
Validator: 1,
Address: common.Address{0x12, 0x34},
Amount: 10,
}
pending = types.NewBlockWithWithdrawals(&types.Header{Number: big.NewInt(11), Time: 42}, []*types.Transaction{tx}, nil, nil, []*types.Withdrawal{withdrawal}, blocktest.NewHasher())
pending = types.NewBlockWithWithdrawals(&types.Header{Number: big.NewInt(11), Time: 42}, types.Transactions{tx}, nil, types.Receipts{receipt}, types.Withdrawals{withdrawal}, blocktest.NewHasher())
)
backend := newTestBackend(t, genBlocks, genesis, ethash.NewFaker(), func(i int, b *core.BlockGen) {
// Transfer from account[0] to account[1]
Expand Down Expand Up @@ -1869,6 +1879,9 @@ func setupReceiptBackend(t *testing.T, genBlocks int) (*testBackend, []common.Ha
return backend, txHashes
}

// TestRPCGetTransactionReceipt tests retrieval of transaction receipts.
//
// note: to regenerate the tests, set the env var WRITE_TEST_FILES.
func TestRPCGetTransactionReceipt(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -1937,6 +1950,9 @@ func TestRPCGetTransactionReceipt(t *testing.T) {
}
}

// TestRPCGetBlockReceipts tests retrieval of all tx receipts in a block.
//
// note: to regenerate the tests, set the env var WRITE_TEST_FILES.
func TestRPCGetBlockReceipts(t *testing.T) {
t.Parallel()

Expand Down
4 changes: 2 additions & 2 deletions internal/ethapi/testdata/eth_getBlockByNumber-tag-pending-fullTx.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
"nonce": null,
"number": "0xb",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0x83c8b28568964036301f75daabe6bed98cb58802cc082489dca64099f8a22419",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"size": "0x256",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
"timestamp": "0x2a",
"totalDifficulty": null,
"transactions": [
{
"blockHash": "0x6cebd9f966ea686f44b981685e3f0eacea28591a7a86d7fbbe521a86e9f81165",
"blockHash": "0x1fc9b3c2ac08acc7d73715f013129f76b3b4ee513d70d9b0bb7dcce498677d4d",
"blockNumber": "0xb",
"from": "0x0000000000000000000000000000000000000000",
"gas": "0x457",
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/testdata/eth_getBlockByNumber-tag-pending.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"nonce": null,
"number": "0xb",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0x83c8b28568964036301f75daabe6bed98cb58802cc082489dca64099f8a22419",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"size": "0x256",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/testdata/eth_getHeaderByNumber-tag-pending.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"nonce": null,
"number": "0xb",
"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
"receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
"receiptsRoot": "0x83c8b28568964036301f75daabe6bed98cb58802cc082489dca64099f8a22419",
"sha3Uncles": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
"stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
"timestamp": "0x2a",
Expand Down