imp: removed alloy-trie and refactored verify_membership (#391)

Author: srdtrk

Cargo.lock

@@ -89,7 +89,6 @@ alloy                  = { version = "0.11", default-features = false }
 alloy-contract         = { version = "0.11", default-features = false }
 alloy-sol-types        = { version = "0.8", default-features = false }
 alloy-primitives       = { version = "0.8", default-features = false }
-alloy-trie             = { version = "0.7", default-features = false }
 alloy-serde            = { version = "0.11", default-features = false }
 alloy-network          = { version = "0.11", default-features = false }
 alloy-signer-local     = { version = "0.11", default-features = false }

Cargo.toml

@@ -47,8 +47,8 @@ test-benchmark testname=".\\*":
 	forge test -vvv --show-progress --gas-report --match-path test/solidity-ibc/BenchmarkTest.t.sol --match-test {{testname}}
 
 # Run the cargo tests
-test-cargo:
-	cargo test --all --locked
+test-cargo testname="--all":
+	cargo test {{testname}} --locked --no-fail-fast -- --nocapture
 
 # Run the tests in abigen
 test-abigen:

e2e/interchaintestv8/wasm/cw_ics08_wasm_eth.wasm.gz

@@ -15,8 +15,7 @@ path = "src/bin/generate_json_schema.rs"
 ethereum-trie-db = { workspace = true }
 ethereum-types = { workspace = true }
 
-alloy-primitives = { workspace = true, features = ["serde", "hex-compat"] }
-alloy-trie = { workspace = true }
+alloy-primitives = { workspace = true, features = ["serde", "hex-compat", "rlp"] }
 alloy-rlp = { workspace = true, features = ["arrayvec"] }
 tree_hash = { workspace = true }
 serde = { workspace = true, features = ["derive"] }

justfile

@@ -6,8 +6,8 @@ use ethereum_types::consensus::bls::BlsPublicKey;
 #[derive(thiserror::Error, Debug, Clone, PartialEq)]
 #[allow(missing_docs, clippy::module_name_repetitions)]
 pub enum EthereumIBCError {
-    #[error("IBC path is empty")]
-    EmptyPath,
+    #[error("invalid path length, expected {expected} but found {found}")]
+    InvalidPathLength { expected: usize, found: usize },
 
     #[error("unable to decode storage proof")]
     StorageProofDecode,

packages/ethereum/ethereum-light-client/Cargo.toml

@@ -1,9 +1,8 @@
 //! This module provides [`verify_membership`] function to verify the membership of a key in the
 //! storage trie.
 
-use alloy_primitives::{keccak256, Bytes, Keccak256, U256};
-use alloy_rlp::encode_fixed_size;
-use alloy_trie::{proof::verify_proof, Nibbles};
+use alloy_primitives::{keccak256, Keccak256, U256};
+use ethereum_trie_db::trie_db::{verify_storage_exclusion_proof, verify_storage_inclusion_proof};
 use ethereum_types::execution::storage_proof::StorageProof;
 
 use crate::{client_state::ClientState, consensus_state::ConsensusState, error::EthereumIBCError};
@@ -17,67 +16,100 @@ pub fn verify_membership(
     client_state: ClientState,
     proof: Vec<u8>,
     path: Vec<Vec<u8>>,
-    raw_value: Option<Vec<u8>>,
+    raw_value: Vec<u8>,
 ) -> Result<(), EthereumIBCError> {
-    let path = path.first().ok_or(EthereumIBCError::EmptyPath)?;
-
     let storage_proof: StorageProof = serde_json::from_slice(proof.as_slice())
         .map_err(|_| EthereumIBCError::StorageProofDecode)?;
 
-    check_commitment_key(
-        path.clone(),
+    check_commitment_path(
+        &path,
         client_state.ibc_commitment_slot,
         storage_proof.key.into(),
     )?;
 
-    let value = match raw_value {
-        Some(unwrapped_raw_value) => {
-            let proof_value = storage_proof.value.to_be_bytes_vec();
-            if proof_value != unwrapped_raw_value {
-                return Err(EthereumIBCError::StoredValueMistmatch {
-                    expected: unwrapped_raw_value,
-                    actual: proof_value,
-                });
-            }
-            Some(encode_fixed_size(&storage_proof.value).to_vec())
+    ensure!(
+        storage_proof.value.to_be_bytes_vec() == raw_value,
+        EthereumIBCError::StoredValueMistmatch {
+            expected: raw_value,
+            actual: storage_proof.value.to_be_bytes_vec(),
         }
-        None => None,
-    };
+    );
+
+    let rlp_value = alloy_rlp::encode_fixed_size(&storage_proof.value);
+    verify_storage_inclusion_proof(
+        &trusted_consensus_state.storage_root,
+        &storage_proof.key,
+        &rlp_value,
+        storage_proof.proof.iter(),
+    )
+    .map_err(|err| EthereumIBCError::VerifyStorageProof(err.to_string()))
+}
+
+/// Verifies the non-membership of a key in the storage trie.
+/// # Errors
+/// Returns an error if the proof cannot be verified.
+#[allow(clippy::module_name_repetitions, clippy::needless_pass_by_value)]
+pub fn verify_non_membership(
+    trusted_consensus_state: ConsensusState,
+    client_state: ClientState,
+    proof: Vec<u8>,
+    path: Vec<Vec<u8>>,
+) -> Result<(), EthereumIBCError> {
+    let storage_proof: StorageProof = serde_json::from_slice(proof.as_slice())
+        .map_err(|_| EthereumIBCError::StorageProofDecode)?;
 
-    let proof: Vec<&Bytes> = storage_proof.proof.iter().collect();
+    check_commitment_path(
+        &path,
+        client_state.ibc_commitment_slot,
+        storage_proof.key.into(),
+    )?;
+
+    ensure!(
+        storage_proof.value.is_zero(),
+        EthereumIBCError::StoredValueMistmatch {
+            expected: vec![0],
+            actual: storage_proof.value.to_be_bytes_vec(),
+        }
+    );
 
-    verify_proof::<Vec<&Bytes>>(
-        trusted_consensus_state.storage_root,
-        Nibbles::unpack(keccak256(storage_proof.key)),
-        value,
-        proof,
+    verify_storage_exclusion_proof(
+        &trusted_consensus_state.storage_root,
+        &storage_proof.key,
+        storage_proof.proof.iter(),
     )
     .map_err(|err| EthereumIBCError::VerifyStorageProof(err.to_string()))
 }
 
-fn check_commitment_key(
-    path: Vec<u8>,
+fn check_commitment_path(
+    path: &[Vec<u8>],
     ibc_commitment_slot: U256,
     key: U256,
 ) -> Result<(), EthereumIBCError> {
-    let expected_commitment_key = ibc_commitment_key_v2(path, ibc_commitment_slot);
-
-    // Data MUST be stored to the commitment path that is defined in ICS23.
-    if expected_commitment_key == key {
-        Ok(())
-    } else {
-        Err(EthereumIBCError::InvalidCommitmentKey(
-            format!("0x{expected_commitment_key:x}"),
+    ensure!(
+        path.len() == 1,
+        EthereumIBCError::InvalidPathLength {
+            expected: 1,
+            found: path.len()
+        }
+    );
+
+    let expected_commitment_path = evm_ics26_commitment_path(&path[0], ibc_commitment_slot);
+    ensure!(
+        expected_commitment_path == key,
+        EthereumIBCError::InvalidCommitmentKey(
+            format!("0x{expected_commitment_path:x}"),
             format!("0x{key:x}"),
-        ))
-    }
+        )
+    );
+
+    Ok(())
 }
 
 // TODO: Unit test
 /// Computes the commitment key for a given path and slot.
-#[must_use = "calculating the commitment key has no effect"]
-pub fn ibc_commitment_key_v2(path: Vec<u8>, slot: U256) -> U256 {
-    let path_hash = keccak256(path);
+#[must_use = "calculating the commitment path has no effect"]
+pub fn evm_ics26_commitment_path(ibc_path: &[u8], slot: U256) -> U256 {
+    let path_hash = keccak256(ibc_path);
 
     let mut hasher = Keccak256::new();
     hasher.update(path_hash);
@@ -105,7 +137,7 @@ mod test {
 
     use prost::Message;
 
-    use super::verify_membership;
+    use super::{verify_membership, verify_non_membership};
 
     #[test]
     fn test_with_fixture() {
@@ -148,7 +180,7 @@ mod test {
             client_state,
             storage_proof,
             vec![path],
-            Some(value),
+            value,
         )
         .unwrap();
     }
@@ -199,7 +231,7 @@ mod test {
             client_state.clone(),
             storage_proof_bz,
             path.clone(),
-            Some(value.to_be_bytes_vec()),
+            value.to_be_bytes_vec(),
         )
         .unwrap();
 
@@ -208,7 +240,7 @@ mod test {
         let storage_proof = StorageProof { key, value, proof };
         let storage_proof_bz = serde_json::to_vec(&storage_proof).unwrap();
 
-        verify_membership(consensus_state, client_state, storage_proof_bz, path, None).unwrap_err();
+        verify_non_membership(consensus_state, client_state, storage_proof_bz, path).unwrap_err();
     }
 
     #[test]
@@ -246,12 +278,11 @@ mod test {
         let proof = StorageProof { key, value, proof };
         let proof_bz = serde_json::to_vec(&proof).unwrap();
 
-        verify_membership(
+        verify_non_membership(
             consensus_state.clone(),
             client_state.clone(),
             proof_bz.clone(),
             path.clone(),
-            None,
         )
         .unwrap();
 
@@ -261,7 +292,7 @@ mod test {
             client_state,
             proof_bz,
             path,
-            Some(value.to_be_bytes_vec()),
+            value.to_be_bytes_vec(),
         )
         .unwrap_err();
     }

packages/ethereum/ethereum-light-client/src/error.rs

@@ -97,15 +97,8 @@ impl RelayerMessages {
 /// A tuple with the commitment path and value
 #[must_use]
 pub fn get_packet_proof(packet: Packet) -> (Vec<u8>, Vec<u8>) {
-    let mut path = Vec::new();
-    path.extend_from_slice(packet.source_client.as_bytes());
-    path.push(1_u8);
-    path.extend_from_slice(&packet.sequence.to_be_bytes());
-
     let ics26_packet: IICS26RouterMsgs::Packet = packet.into();
-    let value = ics26_packet.commit_packet();
-
-    (path, value)
+    (ics26_packet.commitment_path(), ics26_packet.commitment())
 }
 
 impl StepsFixture {

packages/ethereum/ethereum-light-client/src/membership.rs

@@ -18,4 +18,7 @@ pub enum TrieDBError {
 
     #[error("proof is invalid due to missing value: {v}", v = hex::encode(value))]
     ValueMissing { value: Vec<u8> },
+
+    #[error("proof is invalid due to unexpected value: {v}", v = hex::encode(value))]
+    ValueShouldBeMissing { value: Vec<u8> },
 }

packages/ethereum/ethereum-light-client/src/test_utils/fixtures.rs

@@ -25,6 +25,56 @@ pub struct Account {
     pub code_hash: H256,
 }
 
+/// Verifies against `root`, if the `expected_value` is stored at `key` by using `proof`.
+///
+/// * `root`: Storage root of a contract.
+/// * `key`: Padded slot number that the `expected_value` should be stored at.
+/// * `expected_value`: Expected stored value.
+/// * `proof`: Proof that is generated to prove the storage.
+///
+/// NOTE: You must not trust the `root` unless you verified it by calling [`verify_account_storage_root`].
+///
+/// # Errors
+/// Returns an error if the verification fails.
+pub fn verify_storage_inclusion_proof(
+    root: &[u8; 32],
+    key: &[u8; 32],
+    expected_value: &[u8],
+    proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
+) -> Result<(), TrieDBError> {
+    match get_node(H256(*root), key, proof)? {
+        Some(value) if value == expected_value => Ok(()),
+        Some(value) => Err(TrieDBError::ValueMismatch {
+            expected: expected_value.into(),
+            actual: value,
+        })?,
+        None => Err(TrieDBError::ValueMissing {
+            value: expected_value.into(),
+        })?,
+    }
+}
+
+/// Verifies against `root`, that no value is stored at `key` by using `proof`.
+///
+/// * `root`: Storage root of a contract.
+/// * `key`: Padded slot number that the `expected_value` should be stored at.
+/// * `proof`: Proof that is generated to prove the storage.
+///
+/// NOTE: You must not trust the `root` unless you verified it by calling [`verify_account_storage_root`].
+///
+/// # Errors
+/// Returns an error if the verification fails.
+pub fn verify_storage_exclusion_proof(
+    root: &[u8; 32],
+    key: &[u8; 32],
+    proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
+) -> Result<(), TrieDBError> {
+    match get_node(H256(*root), key, proof)? {
+        Some(value) => Err(TrieDBError::ValueShouldBeMissing { value })?,
+        None => Ok(()),
+    }
+}
+
 /// Verifies if the `storage_root` of a contract can be verified against the state `root`.
 ///
 /// * `root`: Light client update's (attested/finalized) execution block's state root.
@@ -44,7 +94,7 @@ pub fn verify_account_storage_root(
     let storage_root: H256 = H256(storage_root.into());
     let address: H160 = H160(address.into());
 
-    match get_node(root, address.as_ref(), proof)? {
+    match get_node(H256(root.into()), address.as_ref(), proof)? {
         Some(account) => {
             let account =
                 rlp::decode::<Account>(account.as_ref()).map_err(TrieDBError::RlpDecode)?;
@@ -63,7 +113,7 @@ pub fn verify_account_storage_root(
 }
 
 fn get_node(
-    root: B256,
+    root: H256,
     key: impl AsRef<[u8]>,
     proof: impl IntoIterator<Item = impl AsRef<[u8]>>,
 ) -> Result<Option<Vec<u8>>, TrieDBError> {
@@ -72,10 +122,7 @@ fn get_node(
         db.insert(hash_db::EMPTY_PREFIX, n.as_ref());
     });
 
-    let root: H256 = H256(root.into());
-
     let trie = TrieDBBuilder::<EthLayout>::new(&db, &root).build();
-
     trie.get(&keccak_256(key.as_ref()))
         .map_err(|e| TrieDBError::GetTrieNodeFailed(e.to_string()))
 }