Fix the way BlockchainTest and StateTest handle invalid transactions

[marioevz]

Right now with #2, intrinsically invalid transactions (not reverting transactions) are removed from the block before execution.

We need to change this behavior to leave the transactions in the block to properly test that blocks with a certain type of invalid transactions are correctly rejected.

This affects the way that StateTest operates, as it bundles all transactions into a single block and then compares the cumulative post-state that results from the execution of all transactions in sequence.

This means that if we leave invalid transactions in the block, the entire block will be rejected and nothing will be tested.

We could solve this issue of two ways.

Assuming that we have Tx1..TxN within a StateTest, and also each Tx is contained in a separate block, we can have:

a)

flowchart LR;
G --> B1[Tx1];
B1[Tx1] --> B2[Tx2];
B2[Tx2] --> ...;
... --> BN[TxN];

Loading

Where each transaction block is on top of the previous transaction.

b)

flowchart TD;
G --> B1[Tx1];
G --> B1'[Tx2];
G --> ...;
G --> B1'''[TxN];

Loading

Where all blocks are at height G+1

(a) makes writing state tests more complicated, because the tester has to account for the previous transactions to calculate the full post-state. With (b), every transaction is isolated and writing the verification post-state is easier (every transaction could have a different verification post-state).

[lightclient]

So you're proposing we either a) verify the aggregate of all the txs at the end of the block or b) verify the result of each tx? IUC, a) is the current status quo?

I think I prefer a) because b) is a strict subset of a) where each tx is just a block.

[marioevz]

Yes, a) is more similar to the status quo.

The only problem I see with a) is that if someone wants to design a test with a specific environment, for example, I want to test BASEFEE, and I set my environment with currentBaseFee=123, then want to check the opcode result on different transactions, if we have a separate block per transaction, the base fee would in reality be dropping each block, so this could potentially be very confusing.

I don't like b) that much either. Having to specify a different post per transaction might also get confusing.

Another option could be to keep all transactions in a single block as is right now but not allow invalid transactions in StateTest, and if you want to design a test with invalid transactions, do a BlockchainTest and explicitly check for the rejected block, and StateTest could still allow transactions that revert.