Block, Tx: always copy Common on instantiation / Fix HF consistency bugs

[holgerd77]

I think we can safe us from a load of problems when the Common instance in block and tx is just copied over. After months of thinking about this I am pretty much sure this is the right thing to do. Both txs and blocks just DO have a certain HF state - and if this is attached to an outer Common which - e.g. in the case of our client with chainCommon - might move on or change, this will lead to consistency problems if the respective Tx or Block objects are still around.

PR was triggered when running the client on Rinkeby which broke at bloke 55 with:

This is now fixed with this PR - as always replaced by another error though (Invalid receipt trie). Original root cause was the block being executed on the wrong HF though.

I think these Common-copy-fix can go in as a bug fix, this will very much rather help prevent unexpected behavior within the community using the library, maybe there might be some rather unintended use cases (which should rather be discouraged) of the library where this might break, but I am pretty sure the benefits weight significantly more here.

In the client as well as in the blockchain library I then switched to hardforkByBlockNumber block and header instantiations. This should once and for all - together with the change from above - free us from all consistency problems in this regard.

I also fixed bugs along the way in Block where the tx objects have been instantiated with the wrong HF in the static constructors when the hardforkByBlockNumber option has been used for block instantiation.

Note that this PR is targeted towards the client-block-poa-fixes branch.

[codecov]

Codecov Report

Merging #1089 (ca69130) into client-block-poa-fixes (a738424) will increase coverage by 0.72%.
The diff coverage is 76.08%.

Flag	Coverage Δ
blockchain	83.51% <100.00%> (?)
common	86.97% <ø> (?)
tx	90.15% <100.00%> (?)
vm	82.32% <74.41%> (-1.41%)	⬇️

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

[holgerd77]

This is the next failure now (independent from this PR):

Block number 55 on Rinkeby.

[holgerd77]

Here is the debug output from the VM:

[holgerd77]

Have found another state changing error in 749b93f where we were still updating the miner account in runTx() also on PoA chains. Block from above is still failing though, now with another (very likely) still invalid state root 😜 (if it is not something else from the receipt):

{
  stateRoot: <Buffer 9e 15 ba 99 5a a6 a3 4f 60 f4 4e 16 4d a3 de aa 75 ab e7 bb eb af cc b4 fd 44 47 b9 99 25 d1 6d>,
  gasUsed: <Buffer 52 08>,
  bitvector: <Buffer 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... 206 more bytes>,
  logs: []
}

[holgerd77]

Ok, I've now added two additional loggers vm:state and vm:block here, this should - on a first round - very much complete the picture. 😄 //cc @jochem-brouwer

Output as following:

Still open for review.

[holgerd77]

But back to the failing tx on Rinkeby (the above was just some other testing log output).

So here is the log:

Actually this is really a dead-simple transaction, which really puzzles me. Just a simple transfer of value. The only thing I can think of right now is something going wrong due to the very large initial account balance of 0x31b98d14007bdee637298086988a0bbd31184523.

[holgerd77]

Got it. 😄

The fee reward didn't need to be removed completely (have removed the commit doing this) but instead the fees have to be provided to the signer account. VM execution is now going forward. I have also added additional backwards-compatibility checks for adding the block clique functionality in the VM as well as some reasonable default for returning a signer on default blocks (zero address), otherwise e.g. the VM would break when be run on a default block.

[holgerd77]

Argh, guys, can someone please review this? Tomorrow I would add yet another PR on top of these two! 😀😅

[ryanio]

lgtm! nice fixes, and wow this new debug output looks really amazing