fix: fix get_storage_proof

[GMKrieger]

The storage proofs were being returned out of order, and certain use cases of the endpoint where there are no storage keys but the root was still needed were not implemented.

Fixes #2458

---

Unable to Request Contract Storage State Root Without Storage Values in RPC v0.8

Problem Statement
In RPC v0.8, it is not possible to request a contract's storage state root without also requesting storage values. This is a regression from previous functionality and breaks certain use cases where only the root hash is needed.

Previous Behavior (pathfinder_getStorageProof)
The ContractData struct previously provided both pieces of information:

pub struct ContractData {
    pub root: Felt,
    pub storage_proofs: Vec<Vec<TrieNode>>,
}

Importantly, the endpoint accepted an empty array of keys, making it possible to retrieve just the root for contracts without storage keys (e.g., newly deployed but unused contracts).

Current Implementation (v0.8)
The new modular approach separates proofs into distinct components:

pub struct StorageProof {
    pub classes_proof: Vec<NodeHashToNodeMappingItem>,
    pub contracts_proof: ContractsProof,
    pub contracts_storage_proofs: Vec<Vec<NodeHashToNodeMappingItem>>,
    pub global_roots: GlobalRoots,
}

Issue Details
In the current implementation there's no way to retrieve the root when there are no storage keys to request
This limitation breaks functionality for cases where:

• Only the contract state root is needed
• Working with newly deployed contracts without storage
• Verifying contract existence without needing storage values

[sistemd]

Can I ask you to please do a few things:

1. Can you add a test for the changed functionality?
2. In a few places it seems to me like you're using manual loops to collect items into a Vec. The old code was the same, but it used iterators instead and collected them into a HashSet. Can you still use iterators? It's sufficient to change from HashSet<_> to Vec<_> in the collect call (I think, again let me know if I'm missing something). I'll elaborate on this in my other comment below.

[sistemd]

This is what I'm referring to in point 2. above. You changed a bunch of code here, but the only change in functionality as far as I can see is that now it's a Vec instead of a HashSet. Could you please just do it like this?

    let nodes: Vec<NodeHashToNodeMapping> =
        ClassCommitmentTree::get_proofs(tx, block_number, class_hashes, class_root_idx)?
            .into_iter()
            .flatten()
            .map(|(node, node_hash)| NodeHashToNodeMapping {
                node_hash,
                node: ProofNode(node),
            })
            .collect::<Vec<_>>()
            .into_iter()
            .collect();
    let classes_proof = NodeHashToNodeMappings(nodes);

(Note the .collect call now uses Vec<_> instead of HashSet<_>. This will keep the order. The order was not being kept before due to us using a HashSet for the nodes.)

[CHr15F0x]

+1

[sistemd]

Why are you using unwrap_or_default here? What will happen now is if the contract root is missing from the DB, this will return a proof for some invalid 0x0 node hash. I'm pretty sure that if it's missing we should just return []? Let me know if I'm missing something.

[GMKrieger]

I don't think you're missing anything, I just copied

pathfinder/crates/rpc/src/pathfinder/methods/get_proof.rs

Lines 316 to 319 in cab975c

	let contract_root = tx
	.contract_root(header.number, input.contract_address)
	.context("Querying contract's root")?
	.unwrap_or_default();

, as it was the previous solution for fetching the root from the contract address.

[sistemd]

Same here, no need to loop yourself, please use iterators like we used to.

[sistemd]

Same here.

[CHr15F0x]

I can see 2 issues here:

1. Proof nodes being unordered, which will be fixed (with @sistemd 's suggestion to stick to iterators).

2. Contract's storage trie root being non-existent for freshly deployed contracts, which have not performed any storage or nonce updates.

This case actually works fine and you can test it, with for example this old contract: 0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5

For /v0_8

{
    "jsonrpc": "2.0",
    "method": "pathfinder_getProof",
    "params": [
        {
            "block_number": 3
        },
        "0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5",
        []
    ],
    "id": 3
}

gives

{
  "id": 3,
  "jsonrpc": "2.0",
  "result": {
    "contract_data": {
      "class_hash": "0x1b661756bf7d16210fc611626e1af4569baa1781ffc964bd018f4585ae241c1",
      "contract_state_hash_version": "0x0",
      "nonce": "0x0",
      "root": "0x0",
      "storage_proofs": []
    },
    "contract_proof": [
      {
        "binary": {
          "left": "0x7cec9cc48ff8fdf1a4f9d5235a6cae25222b6bb824f8ab3bc35f57c82e856a5",
          "right": "0x440353c43d9a3e20ba2e0a6cc09074011fc7369bdef0f5c4c1dbeb56b1544bc"
        }
      },
      {
        "edge": {
          "child": "0x4e535350f60feff5eca7ee0819d2c86029c4e987ee800c8b9fabdb0167d17ae",
          "path": {
            "len": 250,
            "value": "0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5"
          }
        }
      }
    ],
    "state_commitment": "0x1801be8c1f95003d643eb1601adad68585bae7536b7f28f7f3bd600adc39082"
  }
}

ℹ️ root is 0x0 here, which means no trie for that contract.

{
    "jsonrpc": "2.0",
    "method": "starknet_getStorageProof",
    "params": [
        {
            "block_number": 3
        },
        [],
        [
            "0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5"
        ],
        [
            {
                "contract_address": "0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5",
                "storage_keys": []
            }
        ]
    ],
    "id": 4
}

gives:

{
  "id": 4,
  "jsonrpc": "2.0",
  "result": {
    "classes_proof": [],
    "contracts_proof": {
      "contract_leaves_data": [
        {
          "class_hash": "0x1b661756bf7d16210fc611626e1af4569baa1781ffc964bd018f4585ae241c1",
          "nonce": "0x0"
        }
      ],
      "nodes": [
        {
          "node": {
            "left": "0x7cec9cc48ff8fdf1a4f9d5235a6cae25222b6bb824f8ab3bc35f57c82e856a5",
            "right": "0x440353c43d9a3e20ba2e0a6cc09074011fc7369bdef0f5c4c1dbeb56b1544bc"
          },
          "node_hash": "0x1801be8c1f95003d643eb1601adad68585bae7536b7f28f7f3bd600adc39082"
        },
        {
          "node": {
            "child": "0x4e535350f60feff5eca7ee0819d2c86029c4e987ee800c8b9fabdb0167d17ae",
            "length": 250,
            "path": "0x23be95f90bf41685e18a4356e57b0cfdc1da22bf382ead8b64108353915c1e5"
          },
          "node_hash": "0x7cec9cc48ff8fdf1a4f9d5235a6cae25222b6bb824f8ab3bc35f57c82e856a5"
        }
      ]
    },
    "contracts_storage_proofs": [
      []
    ],
    "global_roots": {
      "block_hash": "0x37644818236ee05b7e3b180bed64ea70ee3dd1553ca334a5c2a290ee276f380",
      "classes_tree_root": "0x0",
      "contracts_tree_root": "0x1801be8c1f95003d643eb1601adad68585bae7536b7f28f7f3bd600adc39082"
    }
  }
}

Which is still fine in this very case. The problem arises when a contract does have storage updates and you still need just the root of that contract's trie, which does exist. The easiest solution would be to extend the existing API and add a proper field here, and then you'd have the nonce, class_hash and storage_root for that contract.

[CHr15F0x]

I think this is incorrect. A root of the storage trie for a particular contract is not a proof. As per my main comment - we should extend the API so that when you query for a proof of a contract you get it's class, nonce and storage_root in return. So the fix would be in get_contract_proofs instead of get_contract_storage_proofs.

[GMKrieger]

@sistemd Sorry about the over correction on the loops. I've returned them to using iterators.

There's another issue when dealing with contract storage proof, that the flatten() in the iterators was sending the proofs as one giant Vec<_> instead of a Vec<Vec<_>>. It's fixed on this PR as well. I think it's worth noting that flatten() is being used on all iterators, but snos' current use cases match what is being returned. I don't see any issues to address right now, so I'm leaving them as is.

@CHr15F0x I've opened an issue for starkware explaining this problem with the api definition and we're currently talking about a potential solution. I wholeheartedly agree that what I proposed is patchwork at best, and not an issue with pathfinder itself, just the constraints imposed on it. However it's a away for snos to receive all information it requires, and we sorely needed them to properly prepare for v0_8 release.

Whatever solution is decided on the api, we'll surely need to update this in the future to properly return the data required.

Cheers.

[CHr15F0x]

@GMKrieger storage_root is already available on main. If you could rebase, the only thing remaining would be to fix the ordering or proof nodes.