Refactor storage proofs

[ArielElp]

This PR makes the following changes to the getStorageProof endpoint:

• add block_id argument
• include storage proofs "metadata" for contracts, namely the nonce and class hash
• add the contracts and classes roots to the response
• changed MerkleNode to speak in terms of binary/edge/leaf node rather than in the (length, path, value) terminology

---

This change is 

[pnowosie]

@ArielElp you can also address #229 in one go (simple result.description change). Thanks 🙏

[cchudant]

when implementing it on madara i realized that leaf nodes can be removed entirely, as their value is the same as their hash and during verification you already know that you've hit depth 251 - if that makes sense

[pnowosie]

+1 I'm voting for removing leaf nodes.
They looks oddly in NODE_HASH_TO_NODE_MAPPING

  {
    "node_hash": "<leaf hash value>",
    "node": {
      "value": "<leaf hash value>"
    }
  }

[cchudant]

for edge nodes: i believe the path could be 251 bit long in the worst case, which won't fit in the type "integer" i think. I think a Felt here makes sense?

[pnowosie]

Right now I'm using: Path -> Felt -> Int so the result is be garbage if overflowed 🤓

[cchudant]

I was curious about a very dumb edge case: When a client says that it wants a contract storage proof for a key in a specific contract, and that same contract does not appear in contracts_proof, just in contracts_storage_proofs - does that mean that we have to return a proof for that contract in the contract trie anyway?
if we don't that would mean that storage proof is not verifiable by itself, but maybe that's the client's fault for not putting it in the contracts_proof too

[pnowosie]

You're right regards verification. IMO best is to return whatever requested and no guessing how it's used on the client side

[cchudant]

how do we know which contract corresponds to which contract leaf data here? just using the index of the contract address in the request?

[pnowosie]

+1 for the index, but more convenient would be add an address here as well.

[cchudant]

for context, I am sorting the request keys before traversing the trie in db, so that I can do the whole trie proof using the optimal least amount of backtracking/revisiting nodes possible - I would have a preference toward letting full nodes return the data in whatever order they want
I can re-sort the resulting array in the requested order if the spec really needs that, I just find it weird that it needs it

EDIT: actually nevermind it doesnt matter at all for this field, we can do either

[ArielElp]

Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 4 unresolved discussions (waiting on @cchudant)

---

api/starknet_api_openrpc.json line 948 at r2 (raw file):

does that mean that we have to return a proof for that contract in the contract trie anyway

No. You're right that it won't be verifiable by itself when I need to connect a value to the state commitment, but maybe I just need the storage key --> contract root link.

---

api/starknet_api_openrpc.json line 986 at r2 (raw file):

just using the index of the contract address in the request?

Yes. I added it to the description.

---

api/starknet_api_openrpc.json line 3719 at r2 (raw file):

Previously, pnowosie (Pawel Nowosielski) wrote…

Right now I'm using: Path -> Felt -> Int so the result is be garbage if overflowed 🤓

Felt can also exceed 2**251, and I don't think there's a clear way to specify bounds in json rpc. Added to the description.

---

api/starknet_api_openrpc.json line 3734 at r2 (raw file):

Previously, pnowosie (Pawel Nowosielski) wrote…

+1 I'm voting for removing leaf nodes.
They looks oddly in NODE_HASH_TO_NODE_MAPPING

  {
    "node_hash": "<leaf hash value>",
    "node": {
      "value": "<leaf hash value>"
    }
  }

True, useless in the current form. I still think there should be a way to get the bottom-up path from the result, maybe each node should have a parent hash, in which case the above will not be useless.

[pnowosie]

I still think there should be a way to get the bottom-up path from the result, maybe each node should have a parent hash, in which case the above will not be useless.

Please note that the bottom-up path can also be discovered with current design by building the reverse mapping or lookup the NODE_HASH_TO_NODE_MAPPING array. The hash of the leaf node is stored in the last edge, there is no need for redundant leaf-nodes.

[ArielElp]

Reviewed 1 of 1 files at r3, all commit messages.
Dismissed @cchudant from 4 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)

[ArielElp]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ArielElp)