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.

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

Extra receipt for internal EVM call when finalizing blocks #584

Merged
merged 14 commits into from
Nov 20, 2019
Merged
7 changes: 7 additions & 0 deletions consensus/istanbul/backend/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,13 @@ func (sb *Backend) Finalize(chain consensus.ChainReader, header *types.Header, s
header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
header.UncleHash = nilUncleHash

if len(state.GetLogs(common.Hash{})) > 0 {
receipt := types.NewReceipt(nil, false, 0)
receipt.Logs = state.GetLogs(common.Hash{})
receipt.Bloom = types.CreateBloom(types.Receipts{receipt})
receipts = append(receipts, receipt)
}

// Assemble and return the final block for sealing
return types.NewBlock(header, txs, nil, receipts, randomness), nil
}
Expand Down
14 changes: 12 additions & 2 deletions core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a comment above this:
The receipts may include an additional "block finalization" receipt

return errors.New("transaction and receipt count mismatch")
}

for j := 0; j < len(receipts); j++ {
for j := 0; j < len(transactions); j++ {
// The transaction hash can be retrieved from the transaction itself
receipts[j].TxHash = transactions[j].Hash()

Expand All @@ -816,6 +816,16 @@ func SetReceiptsData(config *params.ChainConfig, block *types.Block, receipts ty
logIndex++
}
}
// Handle extra receipt
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: wdyt of Handle block finalization receipt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wdyt?

if len(transactions)+1 == len(receipts) {
j := len(transactions)
for k := 0; k < len(receipts[j].Logs); k++ {
receipts[j].Logs[k].BlockNumber = block.NumberU64()
receipts[j].Logs[k].BlockHash = block.Hash()
receipts[j].Logs[k].Index = logIndex
logIndex++
}
}
return nil
}

Expand Down
8 changes: 8 additions & 0 deletions core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

receipt := types.NewReceipt(nil, false, 0)
receipt.Logs = statedb.GetLogs(common.Hash{})
receipt.Bloom = types.CreateBloom(types.Receipts{receipt})
receipts = append(receipts, receipt)
}

return receipts, allLogs, *usedGas, nil
}

Expand Down
8 changes: 8 additions & 0 deletions miner/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,14 @@ func (w *worker) commit(uncles []*types.Header, interval func(), update bool, st
}

block, err := w.engine.Finalize(w.chain, w.current.header, s, w.current.txs, uncles, w.current.receipts, w.current.randomness)

if len(s.GetLogs(common.Hash{})) > 0 {
receipt := types.NewReceipt(nil, false, 0)
receipt.Logs = s.GetLogs(common.Hash{})
receipt.Bloom = types.CreateBloom(types.Receipts{receipt})
receipts = append(receipts, receipt)
}

if err != nil {
return err
}
Expand Down