VM: Add vm.buildBlock() and BlockBuilder API

[ryanio]

This PR adds vm.buildBlock() for building a block and adding transactions one at a time.

It can be used like so:

const blockBuilder = await vm.buildBlock({ parentBlock })
const txResult = await blockBuilder.addTransaction(tx)
const block = await blockBuilder.build()

It creates a checkpoint on the StateManager and modifies the state as transactions are run. The checkpoint is committed on build() or discarded with revert().

Closes #1018.

This PR also:

• includes fixes for runBlock with generate: true by setting the header fields for gasUsed, logs bloom, receiptTrie, and transactionsTrie. (closes Generate receipt roots in runBlock #1037)
• moves the tx receipt generation logic in runBlock to a function inside the same file called generateTxReceipt, which is then exported and used in BlockBuilder for generating the receiptTrie.

WIP, still finishing some last spec tests but sharing for initial review.

[codecov]

Codecov Report

Merging #1158 (088122c) into master (486b992) will increase coverage by 0.16%.
The diff coverage is 87.02%.

Flag	Coverage Δ
block	82.08% <80.00%> (+0.06%)	⬆️
blockchain	84.23% <ø> (ø)
client	84.00% <ø> (-0.04%)	⬇️
common	87.39% <ø> (ø)
devp2p	84.13% <ø> (-0.15%)	⬇️
ethash	82.08% <ø> (ø)
tx	84.11% <ø> (ø)
vm	84.02% <87.30%> (+2.33%)	⬆️

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

[holgerd77]

Hi @ryanio, thanks for opening this, this looks really great, the API on this is really beautiful! 😄

I've stumbled upon one bigger thing (the miner reward question on runTx()) which would have caused the post-txs state root to be wrong, this would need some addressing.

I think we can't integrate this PR into the berlin releases any more, this is just too much in the open so close to the releases, hope that's ok. But also considering the complexity of the introduced code here I generally think it makes sense to first test this code in the client (which just is a super-natural fit) before releasing, then we can really be pretty sure that everything is working properly.

We can then target another minor VM release with these changes together with an eventual first client release in 2-4 weeks or so. How does that sound?

[ryanio]

ok this should be ready for review now :)

I noticed that if a PoA block wasn't instantiated with the extraData field or wasn't at least length 97 (vanity + seal) it would return an invalid cliqueSigner() address since the signature would be derived incorrectly.
I fixed this in the block header constructor by padding this.extraData to the minimum required length. I previously had this inside the cliqueSealBlock function but it had to be set on this.extraData for this.cliqueSigHash() to use it correctly.

[jochem-brouwer]

If we use clique, is the miner rewarded?

[ryanio]

there are no block rewards in clique poa. the signer will get the txs amount spent on gas (applied in runTx)

[jochem-brouwer]

Some comments and questions

[alcuadrado]

This looks great, @ryanio! We'll use it as soon as it's released!

I mostly focused this review on the new API and the fix to runBlock. Didn't get into the tests, nor the Clique things.

[alcuadrado]

I hadn't realized that this was necessary here 🤯

[alcuadrado]

The transactions list is not really necessary here, as the EVM can't observe it. Not that it's bad having it either, but I'm just making this comment in case this can lead to some kind of optimization or something.

[ryanio]

got it, yeah, i wanted to pass in the most representative block in case runTx gets updated in the future and uses something that wasn't anticipated.

[alcuadrado]

Thanks for this fix! Looks good.

[holgerd77]

Note: please do not merge before #1170, we can discuss, but atm I am planning to release access list generation as a bugfix release very soon, since I think this is relatively time pressing and should be out before the mainnet HF and at the same time I think it would be good to give the PR here some additional days of practical application test and integration within the client.

[holgerd77]

Ok, had another look at the code changes, this looks good to me, also the test cases are really excellent foster trust in the implementation, since they really read very well on how to use the library and things behave. 😄 👍 Will merge and prepare for release.