MPT StorageDoesNotExist #1699
MPT StorageDoesNotExist #1699
Conversation
github-actions
bot
added
the
crate-zkevm-circuits
miha-stopar
force-pushed
the
mpt-storage-not-exist
branch
from
ffdb225
to
a56c0f4
Compare
| if (i != upTo-1) || (areThereNibbles && isNonExistingProof) { // extension node | ||
| var numberOfNibbles byte | ||
| isExtension = true |
There was a problem hiding this comment.
Can you confirm if we can set isExtension like the following?
let isExtension = (i != upTo-1) || (areThereNibbles && isNonExistingProof);
If that is the case, we can set isExtension at the earliest possible line. We don't have to mutate it in multiple places.
There was a problem hiding this comment.
We need to know whether in the previous iteration of the loop there was an extension node - see:
} else {
var extNode1 []byte = nil
var extNode2 []byte = nil
if isExtension {
extNode1 = proof1[i-1]
extNode2 = proof2[i-1]
}
bNode := prepareBranchNode(proof1[i], proof2[i], extNode1, extNode2, extListRlpBytes, extValues,
key[keyIndex], key[keyIndex], false, false, isExtension)
nodes = append(nodes, bNode)
keyIndex += 1
isExtension = false
}
This is why isExtension is set to false only here (after one iteration). Does it make sense?
However, when inspecting this I realized there were some leftovers from before refactoring, I removed them in the last commit: cc98424
There was a problem hiding this comment.
That makes sense to me; thanks for elaborating on this.
| @@ -142,9 +142,10 @@ impl<F: Field> AccountLeafConfig<F> { | |||
| config.is_mod_extension[is_s.idx()] = cb.query_bool(); | |||
| } | |||
|
|
|||
| // Constraint 1 | |||
There was a problem hiding this comment.
What does the constraint number refer to?
There was a problem hiding this comment.
My intention is to name the constraints so that reading the MPT specs will be easier (the names will be used there), however, I just started with that, but I plan to equip all the constraints with such names.
There was a problem hiding this comment.
This is a fantastic idea @miha-stopar !
There was a problem hiding this comment.
Actually coming from you @adria0 :)
There was a problem hiding this comment.
I see the reasoning now. It's a good idea to add anchors to better reference the spec.
It would be great to add a one-sentence description of the constraint.
### Description While reviewing #1699, I noticed some room to improve the ergonomics of the words. ### Issue Link ### Type of change New feature (non-breaking change which adds functionality) ### Contents - Replaced the `address_word_to_expr` function with the `compress` method so that we can compress a word directly without finding that function to use. - introduced `zero_f` and `one_f` to create zero and one for `Word<F>`. ### Rationale I noticed that it is hard to create a `one()` and `zero()` method for both `Word<Expression<F>>` and `Word<F>`. The compiler complains about implementing duplicated methods. Since we already use `one()` and `zero()` in many places for `Word<Expression<F>>`, I named `zero_f` and `one_f` for the methods for `Word<F>`.
| if (i != upTo-1) || (areThereNibbles && isNonExistingProof) { // extension node | ||
| var numberOfNibbles byte | ||
| isExtension = true |
There was a problem hiding this comment.
That makes sense to me; thanks for elaborating on this.
| @@ -142,9 +142,10 @@ impl<F: Field> AccountLeafConfig<F> { | |||
| config.is_mod_extension[is_s.idx()] = cb.query_bool(); | |||
| } | |||
|
|
|||
| // Constraint 1 | |||
There was a problem hiding this comment.
I see the reasoning now. It's a good idea to add anchors to better reference the spec.
It would be great to add a one-sentence description of the constraint.
| &mut cb.base, | ||
| address.clone(), | ||
| proof_type.clone(), | ||
| Word::<Expression<F>>::new([0.expr(), 0.expr()]), |
There was a problem hiding this comment.
We can now use Word::zero() here now
| Word::<Expression<F>>::new([0.expr(), 0.expr()]), | |
| Word::zero(), |
miha-stopar
force-pushed
the
mpt-storage-not-exist
branch
from
711f774
to
cc98424
Compare
Description
A couple of
StorageDoesNotExistissues to be addressed:Witness generation fails for
getProofproofs of length 0.For
StorageDoesNotExist, when the trie is empty, the keyoccupies
33 (33 = 161 - 128)bytes, as in the example below:Currently, the length for the RLP items is set
valueLen = 34, so we don't have space for all 33 bytes.StorageDoesNotExistproof were lost on the way - currently there are only constraints for thewrongleaf, but the constraints for the case when the node in branch isnilare missing.Type of change
How Has This Been Tested?
TestStorageDoesNotExistOnlySProofinmpt-witness-generator/witness/gen_witness_from_infura_blockchain_test.go.