eqlabs / pathfinder

A Starknet full node written in Rust
https://eqlabs.github.io/pathfinder/
Other
622 stars 231 forks source link

Invalid Class proofs #2295

Open ftheirs opened 5 days ago

ftheirs commented 5 days ago

Hi there!

While running integration tests on SNOS, we’ve encountered an issue in some blocks. Specifically, we retrieve a ClassProof from Pathfinder, and then attempt to verify if the class_hash exists in the tree, but the verification fails. Out of 20k recent blocks, fewer than 2% exhibit this failure, and the code has proven to be battle-tested and reliable in various scenarios.

The issue occurs in certain Sepolia blocks (156541, 156540, 156539, 156538). After some debugging, we discovered that it’s always the same key that fails. We found that the ClassProof can be retrieved correctly up to block 56354, but from block 56355 onward, we start receiving invalid proofs for the class hash 0x5dec330eebf36c8672b60db4a718d44762d3ae6d1333e553197acb47ee5a062, which is associated with the Braavos wallet and is mentioned in the Starknet book.

To rule out potential causes, we synchronized a fresh Pathfinder node, but the issue persists, which leads us to believe this is not a code-related issue, but rather a problem with the state in the database.

I’ve created a simple test to illustrate the issue: block 56354 works fine, but block 56355 starts failing. With this test, you can see that while we can still retrieve the class, the proof appears to be invalid.

You can check out the test here: https://github.com/keep-starknet-strange/snos/tree/ft/invalid_proofs_tests

or directly use the curl command:

> curl --request POST \
     --url http://127.0.0.1:9545/rpc/pathfinder/v0.1 \
     --header 'accept: application/json' \
     --header 'content-type: application/json' \
     --data '
{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "pathfinder_getClassProof",
  "params": [
    { "block_number": 56355 }, "0x05dec330eebf36c8672b60db4a718d44762d3ae6d1333e553197acb47ee5a062"
  ]
}
'

proof_56354.json proof_56355.json

kkovaacs commented 2 days ago

First, class 0x05dec330eebf36c8672b60db4a718d44762d3ae6d1333e553197acb47ee5a062 is a Cairo 0 class which is not in the classes tree.

This means what you're getting from Pathfinder are non-existence proofs. There's actually a missing check in your verify_proof() function: you don't check that the path in an edge node matches the relevant part of the key. If you find a path mismatch in an edge node that means it's a non-existence proof.

I've used the following diff in snos to show the difference in the edge node path:

diff --git a/crates/rpc-client/src/pathfinder/proofs.rs b/crates/rpc-client/src/pathfinder/proofs.rs
index 6918026..9353fa6 100644
--- a/crates/rpc-client/src/pathfinder/proofs.rs
+++ b/crates/rpc-client/src/pathfinder/proofs.rs
@@ -139,6 +139,8 @@ pub fn verify_proof<H: HashFunctionType>(
                         index += 1;
                     }
                     TrieNode::Edge { child, path } => {
+                        dbg!(&path.len, &path.value);
+                        dbg!(&bits[index as usize..(index + path.len) as usize]);
                         parent_hash = *child;
                         index += path.len;
                     }
ftheirs commented 1 day ago

Hey @kkovaacs , thanks again for your help! We'll look into the verification for non-existence proofs. I still have a few questions about the deprecated classes. If these classes are no longer in the class tree, how are we able to retrieve them using the get_class method? Additionally, how can we explain the fact that we're able to obtain a class proof up until a certain block, but receive a non-existence proof after that point?

notlesh commented 1 day ago

I previously thought that the class tree would have a class_hash for deprecated classes but not a compiled_class_hash, do I have this backwards? If you're clear on what the class tree contains when it comes to deprecated classes, can you elaborate?

kkovaacs commented 1 day ago

Hey @kkovaacs , thanks again for your help! We'll look into the verification for non-existence proofs. I still have a few questions about the deprecated classes. If these classes are no longer in the class tree, how are we able to retrieve them using the get_class method?

The starknet_getClass method does not actually use the classes Merkle tree.

Additionally, how can we explain the fact that we're able to obtain a class proof up until a certain block, but receive a non-existence proof after that point?

As far as I've seen from the test above you were receiving non-existence proofs even before block 56355.

kkovaacs commented 1 day ago

I previously thought that the class tree would have a class_hash for deprecated classes but not a compiled_class_hash, do I have this backwards? If you're clear on what the class tree contains when it comes to deprecated classes, can you elaborate?

Unfortunately the Starknet documentation on the class trie is not very explicit but there's actually this sentence in the "note" section:

Cairo classes that are part of the state commitment are defined with Sierra, an intermediate representation between Cairo and Cairo assembly (Casm). However, the prover only works with Casm.

That implies what I'm stating: only Cairo 1+ classes are actually part of the state commitment (and thus the classes trie).