test: check witness when contract creation fails #333
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gballet
changed the title
gballet
commented
Jan 17, 2024
Comment on lines
+573
to
+590
| gspec = &Genesis{ | ||
| Config: config, | ||
| Alloc: GenesisAlloc{ | ||
| coinbase: GenesisAccount{ | ||
| Balance: big.NewInt(1000000000000000000), // 1 ether | ||
| Nonce: 0, | ||
| }, | ||
| account1: GenesisAccount{ | ||
| Balance: big.NewInt(1000000000000000000), // 1 ether | ||
| Nonce: 0, | ||
| }, | ||
| account2: GenesisAccount{ | ||
| Balance: big.NewInt(1000000000000000000), // 1 ether | ||
| Nonce: 1, | ||
| }, | ||
| }, | ||
| } | ||
| ) |
There was a problem hiding this comment.
these values are chosen to simulate the state of kaustinen on blocks 13 and 16, since that's what these problematic txs are taken from.
gballet
commented
Jan 17, 2024
gballet
commented
Jan 17, 2024
gballet
commented
Jan 17, 2024
| gen.AddTx(&tx3) | ||
| } else { | ||
| var tx types.Transaction | ||
| txpayload := common.Hex2Bytes("01f8d683010f2c028443ad7d0e830186a08080b880b00e7fa3c849dce891cce5fae8a4c46cbb313d6aec0c0ffe7863e05fb7b22d4807674c6055527ffbfcb0938f3e18f7937aa8fa95d880afebd5c4cec0d85186095832d03c85cf8a60755260ab60955360cf6096536066609753606e60985360fa609953609e609a53608e609b536024609c5360f6609d536072609e5360a4609fc080a08fc6f7101f292ff1fb0de8ac69c2d320fbb23bfe61cf327173786ea5daee6e37a044c42d91838ef06646294bf4f9835588aee66243b16a66a2da37641fae4c045f") |
gballet
commented
Jan 17, 2024
Comment on lines
-189
to
-210
| func GetTreeKeyStorageSlot(address []byte, storageKey *uint256.Int) []byte { | ||
| pos := storageKey.Clone() | ||
| if storageKey.Cmp(codeStorageDelta) < 0 { | ||
| pos.Add(HeaderStorageOffset, storageKey) | ||
| } else { | ||
| pos.Add(MainStorageOffset, storageKey) | ||
| } | ||
| treeIndex := new(uint256.Int).Div(pos, VerkleNodeWidth) | ||
|
|
||
| // calculate the sub_index, i.e. the index in the stem tree. | ||
| // Because the modulus is 256, it's the last byte of treeIndex | ||
| subIndexMod := new(uint256.Int).Mod(pos, VerkleNodeWidth) | ||
| var subIndex byte | ||
| if len(subIndexMod) != 0 { | ||
| // uint256 is broken into 4 little-endian quads, | ||
| // each with native endianness. Extract the least | ||
| // significant byte. | ||
| subIndex = byte(subIndexMod[0]) | ||
| } | ||
| return GetTreeKey(address, treeIndex, subIndex) | ||
| } | ||
|
|
There was a problem hiding this comment.
This function is not used anywhere, and is buggy. Removing it.
There was a problem hiding this comment.
not buggy, as it turns out, but still unnecessary
gballet
commented
Jan 17, 2024
trie/utils/verkle.go
Outdated
Comment on lines
270
to
273
| // Get the new offset since we now know that we are above 64. | ||
| pos.Sub(&pos, codeStorageDelta) | ||
| suffix := byte(pos[0] & 0xFF) | ||
|
|
There was a problem hiding this comment.
subtract the code storage from the offset before adding the main storage offset value. Get the last byte, because what follows is a right shift.
gballet
commented
Jan 17, 2024
gballet
changed the title
gballet
commented
Jan 17, 2024
jsign
approved these changes
Jan 18, 2024
core/state_processor_test.go
Outdated
Comment on lines
594
to
595
| blockchain, _ := NewBlockChain(bcdb, nil, gspec, nil, beacon.New(ethash.NewFaker()), vm.Config{}, nil, nil) | ||
| defer blockchain.Stop() |
There was a problem hiding this comment.
Looks like these two lines can be removed, since blockchain isn't used in this test.
gballet
commented
Jan 19, 2024
gballet
commented
Jan 19, 2024
gballet
commented
Jan 19, 2024
gballet
added a commit
that referenced
this pull request
May 7, 2024
gballet
added a commit
that referenced
this pull request
May 8, 2024
gballet
added a commit
that referenced
this pull request
May 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The suffix value for a slot was not computed properly: 64 weren't subtracted in order to account for the first 64 slots being in the account header.This PR also adds tests that verify the content of the produced witness, and ensure that the main storage slots will end up where they should. These tests add several transactions taken from Kaustinen (really
verkle-gen-devnet-2), which are contract creation transactions that fail during the execution of the init code. The first two of these transactions are in the first block, and perform aSSTOREbefore the init code reverts. This means that these values should end up in the witness. The third, in the second block, immediately reverts and so the witness should show no proof of absence at code locations.