core, trie: import remaining verkle state processor tests

[gballet]

Ahead of the rebase, I am importing these tests that are crucial to test that the verkle testnet functions properly.

[jwasinger]

Needs a fix: TerminalTotalDifficultyPassed was removed.

[jwasinger]

Nitpick for readability: ahould also reference opcodes via their mnemonics and not raw values.

[gballet]

Included all the changes requested, except:

Nitpick for readability: ahould also reference opcodes via their mnemonics and not raw values.

No thank you, I would have to byte() cast every opcode, and the way it is currently done, we can highlight what in the code is important to understand what the test is trying to achieve.

[holiman]

LGTM, but let's drop the changes that touch stuff for no reason (whitespace).

[holiman]

Isn't it better to add all these to a file verkle_processing_test.go, or verkle_state_processor_test.go?

[holiman]

"SSTORE at slot 41 and reverts" ???
No. It starts with f8, which is the eof EXTCALL opcode, for starters. The one below, "SSTORE at slot 133 and reverts" starts with 02, a.k.a MUL, so obviously croaks on shallow stack immediately.

This kind of highlights the unmaintainability of binary blobs like this. IF you need particular code, either

1. Use slices of vm.Opcode (for simple stuff).
2. Use assembly programs, and feed them through some asm -> opcodes engine.

Tangential: What is the point of the payloads in all of these transactions?

[gballet]

these payloads aren't EOF, they are the RLP of raw transactions extracted from the testnet. They are blobs because they are extracted from a real life testnet block.

[holiman]

Sorry, undoing that approval: those payload blobs make zero sense

[holiman]

This test doesn't achieve that goal. The runtime bytecode is correct, it does a selfdestruct-to-self. However, the test passes even if one uses e.g. selfDestructContract := []byte{byte(vm.NUMBER), byte(vm.SELFDESTRUCT)}

But, if I add some money to it beforehand,

	// Also add the contract-to-destroy
	gspec.Alloc[contract] = types.Account{
		Balance: big.NewInt(100),
	}

then we get a crash:

=== RUN   TestProcessVerkleSelfDestructInSameTxWithSelfBeneficiary
    verkle_witness_test.go:958: Contract: 0x3A220f351252089D385b29beca14e27F204c296A
--- FAIL: TestProcessVerkleSelfDestructInSameTxWithSelfBeneficiary (1.51s)
panic: not implemented [recovered]
	panic: not implemented

goroutine 29 [running]:
testing.tRunner.func1.2({0xebcb80, 0x1290420})
	/usr/local/go/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1635 +0x35e
panic({0xebcb80?, 0x1290420?})
	/usr/local/go/src/runtime/panic.go:785 +0x132
github.com/ethereum/go-ethereum/trie.(*VerkleTrie).NodeIterator(0xa4bd96820128063b?, {0x6539b885bc236522?, 0x1f81a32f5e61a48c?, 0xaceb1d6fa1cb7083?})
	/home/user/go/src/github.com/ethereum/go-ethereum/trie/verkle.go:255 +0x25

I'll commit the change which makes it break, so we don't forget to fix it :)

[gballet]

This test doesn't achieve that goal. The runtime bytecode is correct, it does a selfdestruct-to-self. However, the test passes even if one uses e.g. selfDestructContract := []byte{byte(vm.NUMBER), byte(vm.SELFDESTRUCT)}

It does, the bug I'm interested in, which was fixed, is that if you self-destruct with yourself as a beneficiary, the funds don't end up in the witness - it should be nothing. If you selfdestruct to another address, the balance isn't transferred to the self address, so the "new value" will still be nil, which is normal, and the test passes.

But, if I add some money to it beforehand,

ETH is not money 😁

	// Also add the contract-to-destroy
	gspec.Alloc[contract] = types.Account{
		Balance: big.NewInt(100),
	}

This means you are no longer testing the use case, as the account already exists - which is still a valid test case, but you're changing what is being tested.

then we get a crash:

=== RUN   TestProcessVerkleSelfDestructInSameTxWithSelfBeneficiary
    verkle_witness_test.go:958: Contract: 0x3A220f351252089D385b29beca14e27F204c296A
--- FAIL: TestProcessVerkleSelfDestructInSameTxWithSelfBeneficiary (1.51s)
panic: not implemented [recovered]
	panic: not implemented

goroutine 29 [running]:
testing.tRunner.func1.2({0xebcb80, 0x1290420})
	/usr/local/go/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1635 +0x35e
panic({0xebcb80?, 0x1290420?})
	/usr/local/go/src/runtime/panic.go:785 +0x132
github.com/ethereum/go-ethereum/trie.(*VerkleTrie).NodeIterator(0xa4bd96820128063b?, {0x6539b885bc236522?, 0x1f81a32f5e61a48c?, 0xaceb1d6fa1cb7083?})
	/home/user/go/src/github.com/ethereum/go-ethereum/trie/verkle.go:255 +0x25

I'll commit the change which makes it break, so we don't forget to fix it :)

Ok yeah, interestingly that triggered a piece of code that shouldn't be called. I have to look into it, thanks.

[holiman]

you use NewContractCreation a lot. It's deprecated

[gballet]

no surprise here, I'm trying to merge changes to a fork from 4 years ago.

What should this be replaced with though? I still see it used all over the codebase. nvm found it.

[holiman]

You have a lot of tests surrounding selfdestruct, however, the scenario around "selfdestruct in the same tx" seems a bit under-explored. You do

• Send create-tx. During evaluation of initcode, instead of deploying code, invoke SELFDESTRUCT.

A more complicated scenario that might be worth including is:

• Have a contract (factory) A, already existing in state.
• Send tx to A. This triggers
  • A does CREATE2 and spawns B.
  • A calls B
    • B performs SELFDESTRUCT.
  • A returns ok (as opposed error-exit)

[namiloh]

Tip, types.SignTx(types.NewTx( can be replaced by types.SignNewTx

[holiman]

LGTM