Extra receipt for internal EVM call when finalizing blocks #584
Conversation
|
Looks like this may have broken fast sync? |
|
Sorry, thought it would be working because transfer tests also fast sync. But validators aren't elected there... |
core/blockchain.go
Outdated
| @@ -816,6 +816,16 @@ func SetReceiptsData(config *params.ChainConfig, block *types.Block, receipts ty | |||
| logIndex++ | |||
| } | |||
| } | |||
| // Handle extra receipt | |||
There was a problem hiding this comment.
nit: wdyt of Handle block finalization receipt?
| @@ -84,8 +84,16 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.StateDB, cfg | |||
| allLogs = append(allLogs, receipt.Logs...) | |||
| } | |||
| // Finalize the block, applying any consensus engine specific extras (e.g. block rewards) | |||
| statedb.Prepare(common.Hash{}, block.Hash(), len(block.Transactions())) | |||
There was a problem hiding this comment.
When logs are added, the associated transaction will be read from the instance variables that are set by this method.
| p.engine.Finalize(p.bc, header, statedb, block.Transactions(), block.Uncles(), receipts, block.Randomness()) | ||
|
|
||
| if len(statedb.GetLogs(common.Hash{})) > 0 { |
There was a problem hiding this comment.
Not 100% clear on what's going on here. Would there be a single receipt for all blocks? If not, won't we have the null hash being mapped to many receipts?
There was a problem hiding this comment.
Yeah, many receipts will be associated into null hash, but they can be separated because they are contained in different blocks.
Comparison between using null and block hash
- if block hash is used, there will be more hashes without corresponding transactions
- if null hash is used, (tx hash, log index) pairs will no longer be unique
I'll next check that tx hash in receipt is not consensus data.
|
Thanks for working on this! |
| @@ -786,11 +786,11 @@ func SetReceiptsData(config *params.ChainConfig, block *types.Block, receipts ty | |||
| signer := types.MakeSigner(config, block.Number()) | |||
|
|
|||
| transactions, logIndex := block.Transactions(), uint(0) | |||
| if len(transactions) != len(receipts) { | |||
| if len(transactions) != len(receipts) && len(transactions)+1 != len(receipts) { | |||
There was a problem hiding this comment.
Please add a comment above this:
The receipts may include an additional "block finalization" receipt
core/blockchain.go
Outdated
| @@ -816,6 +816,16 @@ func SetReceiptsData(config *params.ChainConfig, block *types.Block, receipts ty | |||
| logIndex++ | |||
| } | |||
| } | |||
| // Handle extra receipt | |||
| @@ -614,6 +614,9 @@ func (w *worker) resultLoop() { | |||
| // receipt/log of individual transactions were created. | |||
| for _, log := range receipt.Logs { | |||
| log.BlockHash = hash | |||
| if (log.TxHash == common.Hash{}) { | |||
There was a problem hiding this comment.
Please add a comment Handle block finalization receipt
Description
When a block is finalized, the events emitted from internal EVM calls were not stored into the block, because there was no associated transaction and receipt. This PR adds an extra receipt in the case the block finalization emits events.
Tested
Let's see if CI works.
Other changes
Related issues
Backwards compatibility
Might cause some issues, for example the events are associated to tx with hash
0x00000000.