Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/chain_parsers/visualsign-ethereum/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Determinism enforcement for visualsign-ethereum.
#
# The rendered SignablePayload JSON is consumed downstream for hashing and
# wallet display, so iteration order over any map that ends up in the output
# MUST be stable across runs. HashMap/HashSet's randomized hasher silently
# breaks this.
#
# Use BTreeMap/BTreeSet instead. The performance delta is negligible at the
# sizes we use these maps (typically <100 entries) and the determinism
# guarantee is worth the trade.
#
# This rule is intentionally crate-wide rather than scoped to one module --
# even maps that look "lookup-only" today (e.g. the AbiRegistry's internal
# maps, the ContractRegistry's lookup tables) can leak iteration order via
# debug formatting, error messages, or future code paths that walk them.
# Forcing BTreeMap everywhere keeps the determinism story simple and
# review-friendly: no `#[allow(clippy::disallowed_types)]` escape hatches in
# the codebase.
#
# Mirror of visualsign-solana/clippy.toml (commit b723208).

disallowed-types = [
{ path = "std::collections::HashMap", reason = "use BTreeMap -- iteration order affects rendered SignablePayload output and breaks deterministic hashing. See clippy.toml comment." },
{ path = "std::collections::HashSet", reason = "use BTreeSet for the same reason as HashMap." },
]
Comment on lines +22 to +25
24 changes: 18 additions & 6 deletions src/chain_parsers/visualsign-ethereum/src/abi_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,9 @@ mod tests {

const TEST_ADDRESS: &str = "0xdAC17F958D2ee523a2206206994597C13D831ec7";

fn make_abi_mappings(entries: Vec<(&str, Abi)>) -> std::collections::HashMap<String, Abi> {
/// Builds the test fixture as a `BTreeMap` (crate determinism rule) and lets each
/// call site `.into_iter().collect()` into the proto field's `HashMap`.
fn make_abi_mappings(entries: Vec<(&str, Abi)>) -> std::collections::BTreeMap<String, Abi> {
entries
.into_iter()
.map(|(addr, abi)| (addr.to_string(), abi))
Expand Down Expand Up @@ -418,7 +420,9 @@ mod tests {
value: VALID_ABI.to_string(),
signature: None,
},
)]),
)])
.into_iter()
.collect(),
})),
};
let registry =
Expand Down Expand Up @@ -475,7 +479,9 @@ mod tests {
value: VALID_ABI.to_string(),
signature: Some(proto_sig),
},
)]),
)])
.into_iter()
.collect(),
})),
};
let registry =
Expand Down Expand Up @@ -509,7 +515,9 @@ mod tests {
value: VALID_ABI.to_string(),
signature: None,
},
)]),
)])
.into_iter()
.collect(),
})),
};
// Invalid entries are skipped; with no valid entries left, result is None
Expand All @@ -527,7 +535,9 @@ mod tests {
value: "not valid json".to_string(),
signature: None,
},
)]),
)])
.into_iter()
.collect(),
})),
};
// Invalid ABI JSON is skipped; with no valid entries left, result is None
Expand Down Expand Up @@ -555,7 +565,9 @@ mod tests {
signature: None,
},
),
]),
])
.into_iter()
.collect(),
})),
};
// The valid entry should be registered; the invalid one skipped
Expand Down
10 changes: 5 additions & 5 deletions src/chain_parsers/visualsign-ethereum/src/abi_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! - Performance: No file I/O or JSON parsing overhead
//! - Determinism: Same binary always uses same ABIs

use std::collections::HashMap;
use std::collections::BTreeMap;
use std::sync::Arc;

use alloy_json_abi::JsonAbi;
Expand All @@ -35,17 +35,17 @@ pub type ChainId = u64;
#[derive(Clone)]
pub struct AbiRegistry {
/// Maps ABI name -> parsed JsonAbi
abis: Arc<HashMap<String, Arc<JsonAbi>>>,
abis: Arc<BTreeMap<String, Arc<JsonAbi>>>,
/// Maps (chain_id, contract_address) -> ABI name
address_mappings: Arc<HashMap<(ChainId, Address), String>>,
address_mappings: Arc<BTreeMap<(ChainId, Address), String>>,
}

impl AbiRegistry {
/// Creates a new empty ABI registry
pub fn new() -> Self {
Self {
abis: Arc::new(HashMap::new()),
address_mappings: Arc::new(HashMap::new()),
abis: Arc::new(BTreeMap::new()),
address_mappings: Arc::new(BTreeMap::new()),
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/chain_parsers/visualsign-ethereum/src/networks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,17 +497,17 @@ mod tests {

#[test]
fn test_constants_are_unique() {
let mut seen_ids = std::collections::HashSet::new();
let mut seen_ids = std::collections::BTreeSet::new();
for &(chain_id, _, _, _) in ALL_NETWORKS {
assert!(seen_ids.insert(chain_id));
}

let mut seen_names = std::collections::HashSet::new();
let mut seen_names = std::collections::BTreeSet::new();
for &(_, network_id, _, _) in ALL_NETWORKS {
assert!(seen_names.insert(network_id));
}

let mut seen_displays = std::collections::HashSet::new();
let mut seen_displays = std::collections::BTreeSet::new();
for &(_, _, display, _) in ALL_NETWORKS {
assert!(seen_displays.insert(display));
}
Expand Down
28 changes: 14 additions & 14 deletions src/chain_parsers/visualsign-ethereum/src/registry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::token_metadata::{ChainMetadata, TokenMetadata, parse_network_id};
use alloy_primitives::{Address, utils::format_units};
use std::collections::HashMap;
use std::collections::BTreeMap;

/// Type alias for chain ID to avoid depending on external chain types
pub type ChainId = u64;
Expand All @@ -9,7 +9,7 @@ pub type ChainId = u64;
///
/// This enum provides type safety for well-known contract addresses,
/// preventing typos and enabling compile-time checks.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum WellKnownAddress {
/// Permit2 contract - universal across all chains
Permit2,
Expand Down Expand Up @@ -78,24 +78,24 @@ pub trait ContractType: 'static {
#[derive(Clone)]
pub struct ContractRegistry {
/// Maps (chain_id, address) to contract type
address_to_type: HashMap<(ChainId, Address), String>,
address_to_type: BTreeMap<(ChainId, Address), String>,
/// Maps (chain_id, contract_type) to list of addresses
type_to_addresses: HashMap<(ChainId, String), Vec<Address>>,
type_to_addresses: BTreeMap<(ChainId, String), Vec<Address>>,
/// Maps (chain_id, token_address) to token metadata
token_metadata: HashMap<(ChainId, Address), TokenMetadata>,
token_metadata: BTreeMap<(ChainId, Address), TokenMetadata>,
/// Maps (well_known_address, optional_chain_id) to address
/// For chain-specific addresses, use Some(chain_id); for universal addresses, use None
well_known_addresses: HashMap<(WellKnownAddress, Option<ChainId>), Address>,
well_known_addresses: BTreeMap<(WellKnownAddress, Option<ChainId>), Address>,
}

impl ContractRegistry {
/// Creates a new empty registry
pub fn new() -> Self {
Self {
address_to_type: HashMap::new(),
type_to_addresses: HashMap::new(),
token_metadata: HashMap::new(),
well_known_addresses: HashMap::new(),
address_to_type: BTreeMap::new(),
type_to_addresses: BTreeMap::new(),
token_metadata: BTreeMap::new(),
well_known_addresses: BTreeMap::new(),
}
}

Expand Down Expand Up @@ -550,7 +550,7 @@ mod tests {
fn test_load_chain_metadata() {
let mut registry = ContractRegistry::new();

let mut assets = HashMap::new();
let mut assets = BTreeMap::new();
assets.insert(
"USDC".to_string(),
create_token_metadata(
Expand Down Expand Up @@ -715,7 +715,7 @@ mod tests {
fn test_load_chain_metadata_with_invalid_addresses() {
let mut registry = ContractRegistry::new();

let mut assets = HashMap::new();
let mut assets = BTreeMap::new();
// Valid token
assets.insert(
"USDC".to_string(),
Expand Down Expand Up @@ -779,7 +779,7 @@ mod tests {

let metadata = ChainMetadata {
network_id: "UNKNOWN_NETWORK".to_string(),
assets: HashMap::new(),
assets: BTreeMap::new(),
};

let result = registry.load_chain_metadata(&metadata);
Expand Down Expand Up @@ -925,7 +925,7 @@ mod tests {
}
WellKnownAddress::Weth => {
// WETH should be chain-specific - different addresses per chain
let mut addresses = std::collections::HashSet::new();
let mut addresses = std::collections::BTreeSet::new();

for &chain_id in &test_chains {
let addr = registry
Expand Down
6 changes: 3 additions & 3 deletions src/chain_parsers/visualsign-ethereum/src/token_metadata.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use std::collections::HashMap;
use std::collections::BTreeMap;

/// Standard for ERC token types
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -56,7 +56,7 @@ pub struct ChainMetadata {
/// Network identifier as string (e.g., "ETHEREUM_MAINNET")
pub network_id: String,
/// Map of token symbol to token metadata
pub assets: HashMap<String, TokenMetadata>,
pub assets: BTreeMap<String, TokenMetadata>,
}

Comment thread
pepe-anchor marked this conversation as resolved.
/// Error type for token metadata operations
Expand Down Expand Up @@ -248,7 +248,7 @@ mod tests {
fn test_chain_metadata_serialization() {
let mut metadata = ChainMetadata {
network_id: "ETHEREUM_MAINNET".to_string(),
assets: HashMap::new(),
assets: BTreeMap::new(),
};

let usdc = TokenMetadata {
Expand Down
8 changes: 4 additions & 4 deletions src/chain_parsers/visualsign-ethereum/src/visualizer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::context::VisualizerContext;
use std::collections::HashMap;
use std::collections::BTreeMap;
use visualsign::AnnotatedPayloadField;
use visualsign::vsptrait::VisualSignError;

Expand Down Expand Up @@ -62,7 +62,7 @@ pub trait CalldataVisualizer: Send + Sync {
/// This registry is designed to be built once and shared immutably (e.g., in an Arc).
/// Use `EthereumVisualizerRegistryBuilder` to construct a registry.
pub struct EthereumVisualizerRegistry {
visualizers: HashMap<String, Box<dyn ContractVisualizer>>,
visualizers: BTreeMap<String, Box<dyn ContractVisualizer>>,
}

impl EthereumVisualizerRegistry {
Expand Down Expand Up @@ -92,14 +92,14 @@ impl crate::context::VisualizerRegistry for EthereumVisualizerRegistry {
/// Once all visualizers are registered, call `build()` to create an immutable registry.
#[derive(Default)]
pub struct EthereumVisualizerRegistryBuilder {
visualizers: HashMap<String, Box<dyn ContractVisualizer>>,
visualizers: BTreeMap<String, Box<dyn ContractVisualizer>>,
}

impl EthereumVisualizerRegistryBuilder {
/// Creates a new empty builder
pub fn new() -> Self {
Self {
visualizers: HashMap::new(),
visualizers: BTreeMap::new(),
}
}

Expand Down
9 changes: 6 additions & 3 deletions src/chain_parsers/visualsign-ethereum/tests/lib_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use alloy_primitives::U256;
use alloy_rlp::Encodable;
use alloy_sol_types::{SolCall, sol};
use generated::parser::{Abi, ChainMetadata, EthereumMetadata, chain_metadata::Metadata};
use std::collections::HashMap;
use std::collections::BTreeMap;
use std::fs;
use std::path::PathBuf;
use visualsign::vsptrait::{VisualSignConverterFromString, VisualSignOptions};
Expand Down Expand Up @@ -221,7 +221,7 @@ fn test_abi_from_metadata_decodes_function() {
"stateMutability": "nonpayable"
}]"#;

let mut abi_mappings = HashMap::new();
let mut abi_mappings = BTreeMap::new();
abi_mappings.insert(
unknown_contract.to_string(),
Abi {
Expand All @@ -230,13 +230,16 @@ fn test_abi_from_metadata_decodes_function() {
},
);

// Boundary conversion: proto `EthereumMetadata.abi_mappings` is still
// `HashMap` in generated code. The crate-wide rule (clippy.toml) keeps us
// on `BTreeMap` internally and we collect at the FFI point.
let options = VisualSignOptions {
decode_transfers: true,
transaction_name: None,
metadata: Some(ChainMetadata {
metadata: Some(Metadata::Ethereum(EthereumMetadata {
network_id: Some("ETHEREUM_MAINNET".to_string()),
abi_mappings,
abi_mappings: abi_mappings.into_iter().collect(),
})),
}),
developer_config: None,
Expand Down
Loading