core: panic if txs and receipts don't match in types.NewBlock(..)
#29501
Conversation
lightclient
requested review from
karalabe,
holiman and
rjl493456442
as code owners
lightclient
force-pushed
the
tx-and-receipt-counts
branch
from
b947281
to
876973d
Compare
lightclient
force-pushed
the
tx-and-receipt-counts
branch
from
876973d
to
a911e7d
Compare
lightclient
changed the title
types.NewBlock(..)types.NewBlock(..)
| @@ -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") | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Makes sense to me! Fine either way.
There was a problem hiding this comment.
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"
Fixes a
TODOintypes.NewBlock.