Skip to content

Commit

Permalink
feat(starknet_patricia,starknet_committer): stop using custom Contrac…
Browse files Browse the repository at this point in the history
…tAddress type
  • Loading branch information
dorimedini-starkware committed Feb 6, 2025
1 parent c654b17 commit c211f84
Show file tree
Hide file tree
Showing 18 changed files with 135 additions and 81 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/starknet_committer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pretty_assertions.workspace = true
rstest.workspace = true
serde_json.workspace = true
starknet-types-core = { workspace = true, features = ["hash"] }
starknet_api.workspace = true
starknet_patricia = { workspace = true, features = ["testing"] }
thiserror.workspace = true
tokio = { workspace = true, features = ["rt"] }
Expand Down
26 changes: 19 additions & 7 deletions crates/starknet_committer/src/block_committer/commit.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
use std::collections::HashMap;

use starknet_api::core::ContractAddress;
use starknet_patricia::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices};
use starknet_patricia::storage::map_storage::MapStorage;
use tracing::{info, warn};

use crate::block_committer::errors::BlockCommitmentError;
use crate::block_committer::input::{Config, ConfigImpl, ContractAddress, Input, StateDiff};
use crate::block_committer::input::{
from_contract_address_for_node_index,
Config,
ConfigImpl,
Input,
StateDiff,
};
use crate::forest::filled_forest::FilledForest;
use crate::forest::original_skeleton_forest::{ForestSortedIndices, OriginalSkeletonForest};
use crate::forest::updated_skeleton_forest::UpdatedSkeletonForest;
Expand Down Expand Up @@ -80,17 +87,20 @@ fn check_trivial_nonce_and_class_hash_updates(
) {
for (address, nonce) in address_to_nonce.iter() {
if original_contracts_trie_leaves
.get(&address.into())
.get(&from_contract_address_for_node_index(&address))
.is_some_and(|previous_contract_state| previous_contract_state.nonce == *nonce)
{
warn!("Encountered a trivial nonce update of contract {:?}", address)
}
}

for (address, class_hash) in address_to_class_hash.iter() {
if original_contracts_trie_leaves.get(&address.into()).is_some_and(
|previous_contract_state| previous_contract_state.class_hash == *class_hash,
) {
if original_contracts_trie_leaves
.get(&from_contract_address_for_node_index(&address))
.is_some_and(|previous_contract_state| {
previous_contract_state.class_hash == *class_hash
})
{
warn!("Encountered a trivial class hash update of contract {:?}", address)
}
}
Expand All @@ -105,8 +115,10 @@ pub(crate) fn get_all_modified_indices(
state_diff: &StateDiff,
) -> (StorageTriesIndices, ContractsTrieIndices, ClassesTrieIndices) {
let accessed_addresses = state_diff.accessed_addresses();
let contracts_trie_indices: Vec<NodeIndex> =
accessed_addresses.iter().map(|address| NodeIndex::from(*address)).collect();
let contracts_trie_indices: Vec<NodeIndex> = accessed_addresses
.iter()
.map(|address| from_contract_address_for_node_index(*address))
.collect();
let classes_trie_indices: Vec<NodeIndex> =
state_diff.class_hash_to_compiled_class_hash.keys().map(NodeIndex::from).collect();
let storage_tries_indices: HashMap<ContractAddress, Vec<NodeIndex>> = accessed_addresses
Expand Down
39 changes: 16 additions & 23 deletions crates/starknet_committer/src/block_committer/input.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::collections::{HashMap, HashSet};
use std::fmt::Debug;

use starknet_api::core::ContractAddress;
use starknet_patricia::hash::hash_trait::HashOutput;
use starknet_patricia::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf};
use starknet_patricia::patricia_merkle_tree::types::NodeIndex;
Expand All @@ -14,32 +15,24 @@ use crate::patricia_merkle_tree::types::{ClassHash, CompiledClassHash, Nonce};
#[path = "input_test.rs"]
pub mod input_test;

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
// TODO(Nimrod, 1/6/2025): Use the ContractAddress defined in starknet-types-core when available.
pub struct ContractAddress(pub Felt);

impl TryFrom<&NodeIndex> for ContractAddress {
type Error = String;

fn try_from(node_index: &NodeIndex) -> Result<ContractAddress, Self::Error> {
if !node_index.is_leaf() {
return Err("NodeIndex is not a leaf.".to_string());
}
let result = Felt::try_from(*node_index - NodeIndex::FIRST_LEAF);
match result {
Ok(felt) => Ok(ContractAddress(felt)),
Err(error) => Err(format!(
"Tried to convert node index to felt and got the following error: {:?}",
error.to_string()
)),
}
pub fn try_from_node_index_for_contract_address(
node_index: &NodeIndex,
) -> Result<ContractAddress, String> {
if !node_index.is_leaf() {
return Err("NodeIndex is not a leaf.".to_string());
}
let result = Felt::try_from(*node_index - NodeIndex::FIRST_LEAF);
match result {
Ok(felt) => Ok(ContractAddress::try_from(felt).map_err(|error| error.to_string())?),
Err(error) => Err(format!(
"Tried to convert node index to felt and got the following error: {:?}",
error.to_string()
)),
}
}

impl From<&ContractAddress> for NodeIndex {
fn from(address: &ContractAddress) -> NodeIndex {
NodeIndex::from_leaf_felt(&address.0)
}
pub fn from_contract_address_for_node_index(address: &ContractAddress) -> NodeIndex {
NodeIndex::from_leaf_felt(&address.0)
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
Expand Down
25 changes: 18 additions & 7 deletions crates/starknet_committer/src/block_committer/input_test.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
use rstest::rstest;
use starknet_api::core::ContractAddress;
use starknet_patricia::patricia_merkle_tree::types::NodeIndex;
use starknet_types_core::felt::Felt;

use crate::block_committer::input::ContractAddress;
use crate::block_committer::input::try_from_node_index_for_contract_address;

#[rstest]
fn test_node_index_to_contract_address_conversion() {
// Positive flow.
assert_eq!(ContractAddress::try_from(&NodeIndex::FIRST_LEAF), Ok(ContractAddress(Felt::ZERO)));
assert_eq!(
ContractAddress::try_from(&(NodeIndex::FIRST_LEAF + NodeIndex(1_u32.into()))),
Ok(ContractAddress(Felt::ONE))
try_from_node_index_for_contract_address(&NodeIndex::FIRST_LEAF),
Ok(ContractAddress::try_from(Felt::ZERO).unwrap())
);
assert_eq!(
ContractAddress::try_from(&NodeIndex::MAX),
Ok(ContractAddress(Felt::try_from(NodeIndex::MAX - NodeIndex::FIRST_LEAF).unwrap()))
try_from_node_index_for_contract_address(
&(NodeIndex::FIRST_LEAF + NodeIndex(1_u32.into()))
),
Ok(ContractAddress::try_from(Felt::ONE).unwrap())
);
assert_eq!(
try_from_node_index_for_contract_address(&NodeIndex::MAX),
Ok(ContractAddress::try_from(
Felt::try_from(NodeIndex::MAX - NodeIndex::FIRST_LEAF).unwrap()
)
.unwrap())
);

// Negative flow.
assert_eq!(
ContractAddress::try_from(&(NodeIndex::FIRST_LEAF - NodeIndex(1_u32.into()))),
try_from_node_index_for_contract_address(
&(NodeIndex::FIRST_LEAF - NodeIndex(1_u32.into()))
),
Err("NodeIndex is not a leaf.".to_string())
);
}
23 changes: 15 additions & 8 deletions crates/starknet_committer/src/forest/filled_forest.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;

use starknet_api::core::ContractAddress;
use starknet_patricia::hash::hash_trait::HashOutput;
use starknet_patricia::patricia_merkle_tree::filled_tree::tree::FilledTree;
use starknet_patricia::patricia_merkle_tree::node_data::leaf::LeafModifications;
Expand All @@ -8,7 +9,11 @@ use starknet_patricia::patricia_merkle_tree::updated_skeleton_tree::tree::Update
use starknet_patricia::storage::storage_trait::Storage;
use tracing::info;

use crate::block_committer::input::{ContractAddress, StarknetStorageValue};
use crate::block_committer::input::{
from_contract_address_for_node_index,
try_from_node_index_for_contract_address,
StarknetStorageValue,
};
use crate::forest::forest_errors::{ForestError, ForestResult};
use crate::forest::updated_skeleton_forest::UpdatedSkeletonForest;
use crate::hash_function::hash::ForestHashFunction;
Expand Down Expand Up @@ -94,12 +99,14 @@ impl FilledForest {
.into_iter()
.map(|(node_index, storage_trie)| {
(
ContractAddress::try_from(&node_index).unwrap_or_else(|error| {
panic!(
"Got the following error when trying to convert node index \
{node_index:?} to a contract address: {error:?}",
)
}),
try_from_node_index_for_contract_address(&node_index).unwrap_or_else(
|error| {
panic!(
"Got the following error when trying to convert node index \
{node_index:?} to a contract address: {error:?}",
)
},
),
storage_trie,
)
})
Expand Down Expand Up @@ -132,7 +139,7 @@ impl FilledForest {
// `contract_address_to_storage_updates` includes all modified contracts, even those with
// unmodified storage, see StateDiff::actual_storage_updates().
for (contract_address, storage_updates) in contract_address_to_storage_updates {
let node_index = NodeIndex::from(&contract_address);
let node_index = from_contract_address_for_node_index(&contract_address);
let original_contract_state = original_contracts_trie_leaves
.get(&node_index)
.ok_or(ForestError::MissingContractCurrentState(contract_address))?;
Expand Down
3 changes: 1 addition & 2 deletions crates/starknet_committer/src/forest/forest_errors.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use starknet_api::core::ContractAddress;
use starknet_patricia::patricia_merkle_tree::filled_tree::errors::FilledTreeError;
use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::errors::OriginalSkeletonTreeError;
use starknet_patricia::patricia_merkle_tree::updated_skeleton_tree::errors::UpdatedSkeletonTreeError;
use thiserror::Error;
use tokio::task::JoinError;

use crate::block_committer::input::ContractAddress;

pub(crate) type ForestResult<T> = Result<T, ForestError>;

#[derive(Debug, Error)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;

use starknet_api::core::ContractAddress;
use starknet_patricia::hash::hash_trait::HashOutput;
use starknet_patricia::patricia_merkle_tree::node_data::leaf::LeafModifications;
use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::tree::{
Expand All @@ -9,7 +10,11 @@ use starknet_patricia::patricia_merkle_tree::original_skeleton_tree::tree::{
use starknet_patricia::patricia_merkle_tree::types::{NodeIndex, SortedLeafIndices};
use starknet_patricia::storage::storage_trait::Storage;

use crate::block_committer::input::{Config, ContractAddress, StarknetStorageValue};
use crate::block_committer::input::{
from_contract_address_for_node_index,
Config,
StarknetStorageValue,
};
use crate::forest::forest_errors::{ForestError, ForestResult};
use crate::patricia_merkle_tree::leaf::leaf_impl::ContractState;
use crate::patricia_merkle_tree::tree::{
Expand Down Expand Up @@ -94,7 +99,7 @@ impl<'a> OriginalSkeletonForest<'a> {
.get(address)
.ok_or(ForestError::MissingSortedLeafIndices(*address))?;
let contract_state = original_contracts_trie_leaves
.get(&address.into())
.get(&from_contract_address_for_node_index(&address))
.ok_or(ForestError::MissingContractCurrentState(*address))?;
let config =
OriginalSkeletonStorageTrieConfig::new(config.warn_on_trivial_modifications());
Expand Down
18 changes: 10 additions & 8 deletions crates/starknet_committer/src/forest/skeleton_forest_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::HashMap;

use pretty_assertions::assert_eq;
use rstest::rstest;
use starknet_api::core::ContractAddress;
use starknet_patricia::hash::hash_trait::HashOutput;
use starknet_patricia::patricia_merkle_tree::external_test_utils::{
create_32_bytes_entry,
Expand All @@ -23,8 +24,8 @@ use tracing::level_filters::LevelFilter;

use crate::block_committer::commit::get_all_modified_indices;
use crate::block_committer::input::{
from_contract_address_for_node_index,
ConfigImpl,
ContractAddress,
Input,
StarknetStorageKey,
StarknetStorageValue,
Expand Down Expand Up @@ -222,7 +223,7 @@ pub(crate) fn create_contract_state_leaf_entry(val: u128) -> (StorageKey, Storag
},
storage_tries: HashMap::from([
(
ContractAddress(Felt::ZERO),
ContractAddress::try_from(Felt::ZERO).unwrap(),
OriginalSkeletonTreeImpl {
nodes: create_expected_skeleton_nodes(
vec![
Expand All @@ -241,7 +242,7 @@ pub(crate) fn create_contract_state_leaf_entry(val: u128) -> (StorageKey, Storag
}
),
(
ContractAddress(Felt::from(6_u128)),
ContractAddress::try_from(Felt::from(6_u128)).unwrap(),
OriginalSkeletonTreeImpl {
nodes: create_expected_skeleton_nodes(
vec![
Expand All @@ -259,7 +260,7 @@ pub(crate) fn create_contract_state_leaf_entry(val: u128) -> (StorageKey, Storag
}
),
(
ContractAddress(Felt::from(7_u128)),
ContractAddress::try_from(Felt::from(7_u128)).unwrap(),
OriginalSkeletonTreeImpl {
nodes: create_expected_skeleton_nodes(
vec![
Expand Down Expand Up @@ -317,7 +318,7 @@ fn test_create_original_skeleton_forest(
.unwrap();
let expected_original_contracts_trie_leaves = expected_original_contracts_trie_leaves
.into_iter()
.map(|(address, state)| ((&address).into(), state))
.map(|(address, state)| (from_contract_address_for_node_index(&address), state))
.collect();
assert_eq!(original_contracts_trie_leaves, expected_original_contracts_trie_leaves);

Expand All @@ -334,7 +335,7 @@ fn test_create_original_skeleton_forest(
);

for (contract, indices) in expected_storage_tries_sorted_indices {
let contract_address = ContractAddress(Felt::from(contract));
let contract_address = ContractAddress::try_from(Felt::from(contract)).unwrap();
compare_skeleton_tree!(
&actual_forest.storage_tries[&contract_address],
&expected_forest.storage_tries[&contract_address],
Expand All @@ -348,7 +349,8 @@ fn create_contract_leaves(leaves: &[(u128, u128)]) -> HashMap<ContractAddress, C
.iter()
.map(|(idx, root)| {
(
ContractAddress(Felt::from_bytes_be_slice(&create_32_bytes_entry(*idx))),
ContractAddress::try_from(Felt::from_bytes_be_slice(&create_32_bytes_entry(*idx)))
.unwrap(),
ContractState {
nonce: Nonce(Felt::from(*root)),
storage_root_hash: HashOutput(Felt::from(*root)),
Expand All @@ -366,7 +368,7 @@ fn create_storage_updates(
.iter()
.map(|(address, address_indices)| {
(
ContractAddress(Felt::from(u128::from(*address))),
ContractAddress::try_from(Felt::from(u128::from(*address))).unwrap(),
address_indices
.iter()
.map(|val| {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::HashMap;

use starknet_api::core::ContractAddress;
use starknet_patricia::patricia_merkle_tree::node_data::leaf::{LeafModifications, SkeletonLeaf};
use starknet_patricia::patricia_merkle_tree::types::NodeIndex;
use starknet_patricia::patricia_merkle_tree::updated_skeleton_tree::tree::{
Expand All @@ -8,7 +9,7 @@ use starknet_patricia::patricia_merkle_tree::updated_skeleton_tree::tree::{
};
use starknet_types_core::felt::Felt;

use crate::block_committer::input::ContractAddress;
use crate::block_committer::input::from_contract_address_for_node_index;
use crate::forest::forest_errors::{ForestError, ForestResult};
use crate::forest::original_skeleton_forest::OriginalSkeletonForest;
use crate::patricia_merkle_tree::leaf::leaf_impl::ContractState;
Expand Down Expand Up @@ -43,6 +44,7 @@ impl UpdatedSkeletonForest {
let mut storage_tries = HashMap::new();

for (address, updates) in storage_updates {
let address_as_node_index = from_contract_address_for_node_index(&address);
let original_storage_trie = original_skeleton_forest
.storage_tries
.get_mut(address)
Expand All @@ -55,7 +57,7 @@ impl UpdatedSkeletonForest {
storage_tries.insert(*address, updated_storage_trie);

let current_leaf = original_contracts_trie_leaves
.get(&address.into())
.get(&address_as_node_index)
.ok_or(ForestError::MissingContractCurrentState(*address))?;

let skeleton_leaf = Self::updated_contract_skeleton_leaf(
Expand All @@ -64,7 +66,7 @@ impl UpdatedSkeletonForest {
current_leaf,
storage_trie_becomes_empty,
);
contracts_trie_leaves.insert(address.into(), skeleton_leaf);
contracts_trie_leaves.insert(address_as_node_index, skeleton_leaf);
}

// Contracts trie.
Expand Down
Loading

0 comments on commit c211f84

Please sign in to comment.