Generate receipt roots in runBlock

[alcuadrado]

Hey guys,

We found a bug in Hardhat network yesterday, that IMO is a bug in ethereumjs-vm, so I fixed it :)

I think that if a user calls vm.runBlock with the generate: true, is because they don't want to validate the block but to modify the state and update (generate) the fields of the block header that can only be computed after executing the txs. If this is the actual intention, then block.header.receiptTrie was missing.

This PR just updates that header field, and adds a test for it.

Also, as you can see in the test, using runBlock with generate: true is really really inconvenient. I don't get why the new Block wasn't added to RunBlockResult. You can only access it with an event listener. The good news is that adding it is a backwards compatible change. Should I create a PR doing it?

[codecov]

Codecov Report

Merging #1037 (8916a0c) into master (b1bcb03) will increase coverage by 0.01%.
The diff coverage is n/a.

Flag	Coverage Δ
block	77.65% <ø> (ø)
blockchain	77.92% <ø> (ø)
client	88.27% <ø> (-0.10%)	⬇️
common	91.87% <ø> (ø)
devp2p	82.77% <ø> (+0.16%)	⬆️
ethash	82.08% <ø> (ø)
tx	86.25% <ø> (ø)
vm	83.05% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[alcuadrado]

btw, I loved these lines updating the block @ryanio https://github.com/ethereumjs/ethereumjs-vm/pull/1037/files#diff-31eaac2b514be99694029f78d9049fe0c99a28d5bb01060644fee69fbe203926R172-R175

[holgerd77]

@alcuadrado thanks for the PR, did you guys update to VM v5 and so will you get the update on a normal release? If so, how were your update experiences?

[alcuadrado]

Not yet, I also fixed it in a v4 fork. We are currently on holiday seasons, so won't be able to migrate this month. I'll report our experience doing it.

[jochem-brouwer]

Great find - you are indeed right that the receiptsRoot should also be generated, but also the field gasUsed and logsBloom should be generated - right?

You are also right that runBlock is very annoying for this use case. However, we have an issue (#1018) which proposes a new static method which actually handles this generating stuff. I think I'm in favor of that - it makes more sense to call some method like generateBlock (I don't think this is the best name for the method though, maybe something like generateBlockFields?) which handles setting these 4 vars (stateRoot, gasUsed, logsBloom and receiptsRoot) and returns the actual "verified" block.

Also this PR #974 is relevant.

[oozan]

btw, I loved these lines updating the block @ryanio https://github.com/ethereumjs/ethereumjs-vm/pull/1037/files#diff-31eaac2b514be99694029f78d9049fe0c99a28d5bb01060644fee69fbe203926R172-R175

thanks for this reply!