Add test for checkpoint/revert

[yann300]

This test the checkpoint/revert against the contract storage: contract storages don't seem to be reverted as it should.
Also this test might reveal an issue with the account storage: in order to send the third tx, the nonce has to be set to 2 although this should be 1 after the revert.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.21%. Comparing base (6766a5d) to head (20b9bbf).

Additional details and impacted files

Flag	Coverage Δ
block	88.43% <ø> (ø)
blockchain	91.61% <ø> (ø)
client	84.85% <ø> (ø)
common	98.43% <ø> (ø)
devp2p	82.12% <ø> (ø)
ethash	∅ <ø> (∅)
evm	74.16% <ø> (ø)
genesis	99.98% <ø> (ø)
rlp	?
statemanager	77.00% <ø> (ø)
trie	89.32% <ø> (-0.63%)	⬇️
tx	95.55% <ø> (ø)
util	89.19% <ø> (ø)
vm	?
wallet	88.35% <ø> (ø)

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

[holgerd77]

Could you state by adding a "should do x..." comment along assert() what you actually want to test here?

[holgerd77]

I would expect some assert() here (at least additionally, before doing another thing (runBlock() again) since it is the revert() what you seem to want to test?

[holgerd77]

Ok, just looked a bit deeper.

One can get additional library-specific logging with something like:

DEBUG=ethjs,statemanager:statemanager,statemanager:cache:storage,vm:* npx vitest test/api/runTx.spec.ts

This one already gives a somewhat good overview of what is happening when, so loggin all the state actions by the statemanager logger and the general flow with the vm logger (if you add e.g. evm:* it quickly gets too noisy).

This gives an output similar to:

2024-03-14T12:59:50.578Z vm:tx Sender's pre-tx balance is 998573135
2024-03-14T12:59:50.578Z statemanager:statemanager Save account address=0xbe862ad9abfe6f22bcb087716c7d89a26051f74c nonce=1 balance=997583135 contract=no empty=no
2024-03-14T12:59:50.578Z vm:tx Update fromAccount (caller) balance (-> 997583135))
2024-03-14T12:59:50.578Z vm:tx Running tx=0xc1b81dae58f220ddbe27ed0529c4af43db72afe882b48b13c181a3e96e6101c4 with caller=0xbe862ad9abfe6f22bcb087716c7d89a26051f74c gasLimit=68936 to=0x61de9dc6f6cff1df2809480882cfd3c2364b28f7 value=0 data=0x119fbbd4
2024-03-14T12:59:50.578Z statemanager:statemanager Save account address=0xbe862ad9abfe6f22bcb087716c7d89a26051f74c nonce=2 balance=997583135 contract=no empty=no
2024-03-14T12:59:50.578Z statemanager:cache:storage New checkpoint 4
2024-03-14T12:59:50.578Z statemanager:statemanager Save account address=0xbe862ad9abfe6f22bcb087716c7d89a26051f74c nonce=2 balance=997583135 contract=no empty=no
2024-03-14T12:59:50.578Z statemanager:statemanager Save account address=0x61de9dc6f6cff1df2809480882cfd3c2364b28f7 nonce=1 balance=0 contract=yes empty=no
2024-03-14T12:59:50.579Z statemanager:cache:storage Get storage for 61de9dc6f6cff1df2809480882cfd3c2364b28f7
2024-03-14T12:59:50.579Z statemanager:cache:storage Put storage for 61de9dc6f6cff1df2809480882cfd3c2364b28f7: 0000000000000000000000000000000000000000000000000000000000000000 -> 80
2024-03-14T12:59:50.580Z statemanager:cache:storage Get storage for 61de9dc6f6cff1df2809480882cfd3c2364b28f7
2024-03-14T12:59:50.580Z statemanager:cache:storage Get storage for 61de9dc6f6cff1df2809480882cfd3c2364b28f7
2024-03-14T12:59:50.580Z statemanager:cache:storage Put storage for 61de9dc6f6cff1df2809480882cfd3c2364b28f7: 0000000000000000000000000000000000000000000000000000000000000000 -> 01
2024-03-14T12:59:50.580Z statemanager:cache:storage Commit to checkpoint 3
2024-03-14T12:59:50.580Z statemanager:statemanager state checkpoint committed
2024-03-14T12:59:50.580Z vm:tx Update fromAccount (caller) nonce (-> 1)
2024-03-14T12:59:50.580Z vm:tx ----------------------------------------------------------------------------------------------------
2024-03-14T12:59:50.580Z vm:tx Received tx execResult: [ executionGasUsed=22382 exceptionError=none returnValue=0x gasRefund=0 ]
2024-03-14T12:59:50.580Z vm:tx Generated tx bloom with logs=0

As some basic comment: our usage of the checkpointing mechanism is basically - at least somewhat implicitly - based on the assumption that things start before block execution at checkpoint level 0 and then go back down there and then things are flushed or whatever. So we have code (in state manager like if (this._checkpointCount === 0) { await this.flush() } or similar). That does not mean that you can absolutely not use an additional outer checkpointing layer but it is already some kind of a hack with our system not fully build for the purpose. 😋

I have some suspicions where things might go wrong (we have e.g. an OriginalStorageCache which is re-initialized with new block storage values and where things might already be updated when storage being reverted on 2nd or 3rd block), just to get the broader picture: from what EVM/VM versions are you actually upgrading? So that one get a feeling what your previous code base was. Especially since we updated the state manager caches significantly along the way.

[holgerd77]

What you can definitely experiment with is setting the caches (storage, account, code) in StateManager to 0 respectively deactivate and see if this changes anything. So this needs a custom StateManager to passed to the VM (maybe you are doing this already anyhow). Eventually additionally deactivating the Trie caches might also be worth a test (this additionally needs a Trie manually passed to the StateManager 😋 ).

[jochem-brouwer]

I have looked into this but I think what is at least missing is proper documentation for EVMs journal (what it actually does).

Also, note that runTx calls into EVMs cleanJournal. This will clean the journal (reset it). So, your checkpoints will actually get deleted 🤔 The journal is thus not fit for this purpose if runTx is used (which logically gets used for your use case).

I will take a deeper look tomorrow (also see discord for some additional questions).