core, miner: do not populate receipts field during mining #31813
Conversation
MariusVanDerWijden
requested review from
fjl,
rjl493456442,
s1na and
lightclient
as code owners
| receipt := &types.Receipt{Type: tx.Type(), PostState: root, CumulativeGasUsed: usedGas} | ||
| if result.Failed() { | ||
| receipt.Status = types.ReceiptStatusFailed | ||
| } else { | ||
| receipt.Status = types.ReceiptStatusSuccessful | ||
| } | ||
| receipt.Logs = statedb.GetLogs(tx.Hash()) | ||
| receipt.Bloom = types.CreateBloom(receipt) |
There was a problem hiding this comment.
The logs constructed during the block insertion (e.g., full sync) will be emitted to the log subscribers, which assumes the logs should be fully annotated.
Here, the logs are partially annotated, missing blockNumber and blockHash fields.
There was a problem hiding this comment.
Are you sure? I don't see where they are emitted to the log subscribers. I only see them written to the db and later on read from the db during collectLogs and the missing fields are derived
There was a problem hiding this comment.
Gary has a point. This is where the logs are being sent out to subscribers:
go-ethereum/core/blockchain.go
Line 1626 in 8781e93
I missed this. We don't have pending subscription anymore but the log subscription to newly mined logs is there.
I think it will be tricky to derive the fields in the consumer unless we change the structure of the logsFeed. At least would need the header and unflattened logs.
|
I think generally it's a good direction to get rid of the log/receipts deriving in the chain progressing/block building. These field extension should only be added in the filter where the logs are pushed to the subscribers. |
Address an issue brought up by Optimism here: ethereum-optimism/op-geth#462
We are currently populating all receipt fields whenever a transaction is added to the pending state (which included calculating the blockhash). This PR changes it, so we only compute the necessary consensus fields