Merge branch 'tiago/replace-hash-data-structs' (#2685)

* origin/tiago/replace-hash-data-structs:
  Changelog for #2685
  add clippy config
  Replace std hash map and set collections with indexmap alternatives
  Re-export index map and set from core
  Add `indexmap` dep to Cargo manifest

# Conflicts:
#	crates/apps/src/lib/client/rpc.rs
#	crates/apps/src/lib/node/ledger/shell/init_chain.rs
#	crates/core/src/ibc.rs
#	crates/namada/src/ledger/protocol/mod.rs

.changelog/unreleased/bug-fixes/2685-replace-hash-data-structs.md

@@ -0,0 +1,3 @@
+- Use `indexmap` maps and sets in favor of `std` collections, to avoid
+  iteration order related bugs in the state machine code of Namada.
+  ([\#2685](https://github.com/anoma/namada/pull/2685))

Cargo.toml

@@ -107,6 +107,7 @@ ibc-derive = "0.6.0"
 ibc-testkit = {version = "0.50.0", default-features = false}
 ics23 = "0.11.0"
 index-set = { git = "https://github.com/heliaxdev/index-set", tag = "v0.8.1", features = ["serialize-borsh", "serialize-serde"] }
+indexmap = { git = "https://github.com/heliaxdev/indexmap", tag = "2.2.4-heliax-1", features = ["borsh-schema", "serde"] }
 itertools = "0.10.0"
 jubjub = "0.10"
 k256 = { version = "0.13.0", default-features = false, features = ["ecdsa", "pkcs8", "precomputed-tables", "serde", "std"]}

clippy.toml

@@ -0,0 +1,4 @@
+disallowed-types = [
+  { path = "std::collections::HashMap", reason = "Non-deterministic iter - use indexmap::IndexMap instead" },
+  { path = "std::collections::HashSet", reason = "Non-deterministic iter - use indexmap::IndexSet instead" },
+]

crates/apps/src/lib/cli.rs

@@ -2974,7 +2974,6 @@ pub mod cmds {
 }
 
 pub mod args {
-    use std::collections::HashMap;
     use std::env;
     use std::net::SocketAddr;
     use std::path::PathBuf;
@@ -2983,6 +2982,7 @@ pub mod args {
     use data_encoding::HEXUPPER;
     use namada::core::address::{Address, EstablishedAddress};
     use namada::core::chain::{ChainId, ChainIdPrefix};
+    use namada::core::collections::HashMap;
     use namada::core::dec::Dec;
     use namada::core::ethereum_events::EthAddress;
     use namada::core::keccak::KeccakHash;

crates/apps/src/lib/client/rpc.rs

@@ -1,7 +1,7 @@
 //! Client RPC queries
 
 use std::cmp::Ordering;
-use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet};
 use std::io;
 use std::str::FromStr;
 
@@ -14,6 +14,7 @@ use masp_primitives::sapling::{Node, ViewingKey};
 use masp_primitives::transaction::components::I128Sum;
 use masp_primitives::zip32::ExtendedFullViewingKey;
 use namada::core::address::{Address, InternalAddress, MASP};
+use namada::core::collections::{HashMap, HashSet};
 use namada::core::hash::Hash;
 use namada::core::ibc::{is_ibc_denom, IbcTokenHash};
 use namada::core::key::*;

crates/apps/src/lib/client/tx.rs

@@ -1,4 +1,3 @@
-use std::collections::HashSet;
 use std::fs::File;
 
 use borsh::BorshDeserialize;
@@ -7,6 +6,7 @@ use ledger_namada_rs::{BIP44Path, NamadaApp};
 use ledger_transport_hid::hidapi::HidApi;
 use ledger_transport_hid::TransportNativeHID;
 use namada::core::address::{Address, ImplicitAddress};
+use namada::core::collections::HashSet;
 use namada::core::dec::Dec;
 use namada::core::key::{self, *};
 use namada::governance::cli::onchain::{

crates/apps/src/lib/config/genesis.rs

@@ -6,14 +6,15 @@ pub mod templates;
 pub mod transactions;
 pub mod utils;
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
 use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 
 use borsh::{BorshDeserialize, BorshSerialize};
 use derivative::Derivative;
 use namada::core::address::{Address, EstablishedAddress};
 use namada::core::chain::ProposalBytes;
+use namada::core::collections::HashMap;
 use namada::core::key::*;
 use namada::core::storage;
 use namada::core::string_encoding::StringEncoded;

crates/apps/src/lib/config/genesis/transactions.rs

@@ -1,6 +1,6 @@
 //! Genesis transactions
 
-use std::collections::{BTreeMap, BTreeSet, HashSet};
+use std::collections::{BTreeMap, BTreeSet};
 use std::fmt::Debug;
 use std::net::SocketAddr;
 
@@ -13,6 +13,7 @@ use ledger_transport_hid::TransportNativeHID;
 use namada::account::AccountPublicKeysMap;
 use namada::core::address::{Address, EstablishedAddress};
 use namada::core::chain::ChainId;
+use namada::core::collections::HashSet;
 use namada::core::dec::Dec;
 use namada::core::key::{
     common, ed25519, RefTo, SerializeWithBorsh, SigScheme,

crates/apps/src/lib/config/genesis/utils.rs

@@ -1,9 +1,9 @@
-use std::collections::HashSet;
 use std::path::Path;
 
 use eyre::Context;
 use ledger_namada_rs::NamadaApp;
 use ledger_transport_hid::TransportNativeHID;
+use namada::core::collections::HashSet;
 use namada::core::key::common;
 use namada::tx::Tx;
 use namada_sdk::wallet::Wallet;

crates/apps/src/lib/config/mod.rs

@@ -5,13 +5,13 @@ pub mod genesis;
 pub mod global;
 pub mod utils;
 
-use std::collections::HashMap;
 use std::fs::{create_dir_all, File};
 use std::io::Write;
 use std::path::{Path, PathBuf};
 
 use directories::ProjectDirs;
 use namada::core::chain::ChainId;
+use namada::core::collections::HashMap;
 use namada::core::storage::BlockHeight;
 use namada::core::time::Rfc3339String;
 use serde::{Deserialize, Serialize};

crates/apps/src/lib/node/ledger/shell/finalize_block.rs

@@ -703,10 +703,11 @@ fn pos_votes_from_abci(
 /// are covered by the e2e tests.
 #[cfg(test)]
 mod test_finalize_block {
-    use std::collections::{BTreeMap, HashMap, HashSet};
+    use std::collections::BTreeMap;
     use std::num::NonZeroU64;
     use std::str::FromStr;
 
+    use namada::core::collections::{HashMap, HashSet};
     use namada::core::dec::{Dec, POS_DECIMAL_PRECISION};
     use namada::core::ethereum_events::{EthAddress, Uint as ethUint};
     use namada::core::hash::Hash;
@@ -4370,12 +4371,9 @@ mod test_finalize_block {
         )?;
         assert_eq!(
             consensus_vals,
-            HashSet::from_iter([
-                val1.clone(),
-                val2.clone(),
-                val3.clone(),
-                val4.clone()
-            ])
+            [val1.clone(), val2.clone(), val3.clone(), val4.clone()]
+                .into_iter()
+                .collect::<HashSet<_>>(),
         );
         for offset in 1..=params.pipeline_len {
             let consensus_vals = read_consensus_validator_set_addresses(
@@ -4384,7 +4382,9 @@ mod test_finalize_block {
             )?;
             assert_eq!(
                 consensus_vals,
-                HashSet::from_iter([val1.clone(), val3.clone(), val4.clone()])
+                [val1.clone(), val3.clone(), val4.clone()]
+                    .into_iter()
+                    .collect::<HashSet<_>>()
             );
             let val2_state = validator_state_handle(&val2)
                 .get(&shell.state, current_epoch + offset, &params)?

crates/apps/src/lib/node/ledger/shell/governance.rs

@@ -1,5 +1,4 @@
-use std::collections::HashMap;
-
+use namada::core::collections::HashMap;
 use namada::core::encode;
 use namada::core::event::EmitEvents;
 use namada::core::storage::Epoch;

crates/apps/src/lib/node/ledger/shell/init_chain.rs

@@ -1,11 +1,12 @@
 //! Implementation of chain initialization for the Shell
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
 use std::ops::ControlFlow;
 
 use masp_primitives::merkle_tree::CommitmentTree;
 use masp_primitives::sapling::Node;
 use masp_proofs::bls12_381;
 use namada::account::protocol_pk_key;
+use namada::core::collections::HashMap;
 use namada::core::hash::Hash as CodeHash;
 use namada::core::time::{TimeZone, Utc};
 use namada::ledger::parameters::Parameters;
@@ -312,7 +313,7 @@ where
         genesis: &genesis::chain::Finalized,
         vp_cache: &mut HashMap<String, Vec<u8>>,
     ) -> ControlFlow<(), Vec<u8>> {
-        use std::collections::hash_map::Entry;
+        use namada::core::collections::hash_map::Entry;
         let Some(vp_filename) = self
             .validate(
                 genesis

crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs

@@ -1019,10 +1019,9 @@ mod test_prepare_proposal {
         if let ShellMode::Validator { local_config, .. } = &mut shell.mode {
             // Remove the allowed btc
             *local_config = Some(ValidatorLocalConfig {
-                accepted_gas_tokens: std::collections::HashMap::from([(
-                    namada::core::address::testing::nam(),
-                    Amount::from(1),
-                )]),
+                accepted_gas_tokens: namada::core::collections::HashMap::from(
+                    [(namada::core::address::testing::nam(), Amount::from(1))],
+                ),
             });
         }
 
@@ -1125,10 +1124,12 @@ mod test_prepare_proposal {
         if let ShellMode::Validator { local_config, .. } = &mut shell.mode {
             // Remove btc and increase minimum for nam
             *local_config = Some(ValidatorLocalConfig {
-                accepted_gas_tokens: std::collections::HashMap::from([(
-                    namada::core::address::testing::nam(),
-                    Amount::from(100),
-                )]),
+                accepted_gas_tokens: namada::core::collections::HashMap::from(
+                    [(
+                        namada::core::address::testing::nam(),
+                        Amount::from(100),
+                    )],
+                ),
             });
         }
 

crates/apps/src/lib/node/ledger/shell/stats.rs

@@ -1,6 +1,7 @@
-use std::collections::HashMap;
 use std::fmt::Display;
 
+use namada::core::collections::HashMap;
+
 #[derive(Debug, Default)]
 pub struct InternalStats {
     successful_tx: u64,

crates/apps/src/lib/node/ledger/shell/testing/node.rs

@@ -1,4 +1,3 @@
-use std::collections::HashMap;
 use std::future::poll_fn;
 use std::mem::ManuallyDrop;
 use std::path::PathBuf;
@@ -11,6 +10,7 @@ use data_encoding::HEXUPPER;
 use itertools::Either;
 use lazy_static::lazy_static;
 use namada::control_flow::time::Duration;
+use namada::core::collections::HashMap;
 use namada::core::ethereum_events::EthereumEvent;
 use namada::core::ethereum_structs;
 use namada::core::hash::Hash;

crates/apps/src/lib/node/ledger/shell/vote_extensions/eth_events.rs

@@ -1,8 +1,9 @@
 //! Extend Tendermint votes with Ethereum events seen by a quorum of validators.
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::BTreeMap;
 
 use namada::vote_ext::ethereum_events::MultiSignedEthEvent;
+use namada_sdk::collections::HashMap;
 
 use super::*;
 
@@ -135,7 +136,6 @@ where
 
 #[cfg(test)]
 mod test_vote_extensions {
-
     use borsh_ext::BorshSerializeExt;
     use namada::core::address::testing::gen_established_address;
     use namada::core::ethereum_events::{

crates/apps/src/lib/node/ledger/shell/vote_extensions/val_set_update.rs

@@ -1,7 +1,7 @@
 //! Extend Tendermint votes with validator set updates, to be relayed to
 //! Namada's Ethereum bridge smart contracts.
 
-use std::collections::HashMap;
+use namada::core::collections::HashMap;
 
 use super::*;
 

crates/apps/src/lib/node/ledger/storage/mod.rs

@@ -53,11 +53,10 @@ fn new_blake2b() -> Blake2b {
 
 #[cfg(test)]
 mod tests {
-    use std::collections::HashMap;
-
     use borsh::BorshDeserialize;
     use itertools::Itertools;
     use namada::core::chain::ChainId;
+    use namada::core::collections::HashMap;
     use namada::core::ethereum_events::Uint;
     use namada::core::hash::Hash;
     use namada::core::keccak::KeccakHash;

crates/apps/src/lib/wallet/defaults.rs

@@ -10,13 +10,12 @@ pub use dev::{
 
 #[cfg(any(test, feature = "testing", feature = "benches"))]
 mod dev {
-    use std::collections::HashMap;
-
     use lazy_static::lazy_static;
     use namada::core::address::testing::{
         apfel, btc, dot, eth, kartoffel, nam, schnitzel,
     };
     use namada::core::address::Address;
+    use namada::core::collections::HashMap;
     use namada::core::key::*;
     use namada::ledger::{governance, pgf, pos};
     use namada_sdk::wallet::alias::Alias;

crates/apps/src/lib/wasm_loader/mod.rs

@@ -1,12 +1,12 @@
 //! A module for loading WASM files and downloading pre-built WASMs.
 use core::borrow::Borrow;
-use std::collections::HashMap;
 use std::fs;
 use std::path::Path;
 
 use data_encoding::HEXLOWER;
 use eyre::{eyre, WrapErr};
 use futures::future::join_all;
+use namada::core::collections::HashMap;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha256};
 use thiserror::Error;

crates/benches/host_env.rs

@@ -1,8 +1,7 @@
-use std::collections::{HashMap, HashSet};
-
 use criterion::{criterion_group, criterion_main, Criterion};
 use namada::core::account::AccountPublicKeysMap;
 use namada::core::address;
+use namada::core::collections::{HashMap, HashSet};
 use namada::ledger::storage::DB;
 use namada::token::{Amount, Transfer};
 use namada::tx::Signature;

crates/benches/native_vps.rs

@@ -1,11 +1,12 @@
 use std::cell::RefCell;
-use std::collections::{BTreeSet, HashMap};
+use std::collections::BTreeSet;
 use std::rc::Rc;
 use std::str::FromStr;
 
 use criterion::{criterion_group, criterion_main, Criterion};
 use masp_primitives::sapling::Node;
 use namada::core::address::{self, Address, InternalAddress};
+use namada::core::collections::HashMap;
 use namada::core::eth_bridge_pool::{GasFee, PendingTransfer};
 use namada::core::masp::{TransferSource, TransferTarget};
 use namada::eth_bridge::storage::whitelist;

crates/benches/txs.rs

@@ -1,9 +1,9 @@
-use std::collections::HashMap;
 use std::str::FromStr;
 
 use criterion::{criterion_group, criterion_main, Criterion};
 use namada::account::{InitAccount, UpdateAccount};
 use namada::core::address::{self, Address};
+use namada::core::collections::HashMap;
 use namada::core::eth_bridge_pool::{GasFee, PendingTransfer};
 use namada::core::hash::Hash;
 use namada::core::key::{

crates/core/Cargo.toml

@@ -47,6 +47,7 @@ ibc.workspace = true
 ics23.workspace = true
 impl-num-traits = "0.1.2"
 index-set.workspace = true
+indexmap.workspace = true
 k256.workspace = true
 linkme = {workspace = true, optional = true}
 masp_primitives.workspace = true