diff --git a/rs/ethereum/cketh/minter/src/dashboard.rs b/rs/ethereum/cketh/minter/src/dashboard.rs index df16e62e89d..02022e8ee6f 100644 --- a/rs/ethereum/cketh/minter/src/dashboard.rs +++ b/rs/ethereum/cketh/minter/src/dashboard.rs @@ -125,7 +125,8 @@ impl DashboardPendingDeposit { token_symbol: match event { ReceivedEvent::Eth(_) => CkTokenSymbol::cketh_symbol_from_state(state), ReceivedEvent::Erc20(e) => state - .ckerc20_token_symbol(&e.erc20_contract_address) + .ckerc20_tokens + .get_alt(&e.erc20_contract_address) .expect("BUG: unknown ERC-20 token") .clone(), }, @@ -204,16 +205,20 @@ impl DashboardTemplate { token_symbol: CkTokenSymbol::cketh_symbol_from_state(state), created_at: req.created_at, }, - WithdrawalRequest::CkErc20(req) => DashboardWithdrawalRequest { - cketh_ledger_burn_index: req.cketh_ledger_burn_index, - destination: req.destination, - value: req.withdrawal_amount.into(), - token_symbol: state - .ckerc20_token_symbol(&req.erc20_contract_address) - .expect("BUG: unknown ERC-20 token") - .clone(), - created_at: Some(req.created_at), - }, + WithdrawalRequest::CkErc20(req) => { + let erc20_contract_address = &req.erc20_contract_address; + DashboardWithdrawalRequest { + cketh_ledger_burn_index: req.cketh_ledger_burn_index, + destination: req.destination, + value: req.withdrawal_amount.into(), + token_symbol: state + .ckerc20_tokens + .get_alt(erc20_contract_address) + .expect("BUG: unknown ERC-20 token") + .clone(), + created_at: Some(req.created_at), + } + } }) .collect(); withdrawal_requests.sort_unstable_by_key(|req| Reverse(req.cketh_ledger_burn_index)); @@ -284,7 +289,8 @@ impl DashboardTemplate { } => ( *cketh_ledger_burn_index, state - .ckerc20_token_symbol_for_ledger(ledger_id) + .ckerc20_tokens + .get(ledger_id) .expect("BUG: unknown ERC-20 token") .clone(), ), @@ -357,7 +363,8 @@ fn to_dashboard_transaction>( let destination = to; let value = value.into(); let token_symbol = state - .ckerc20_token_symbol(&tx.destination) + .ckerc20_tokens + .get_alt(&tx.destination) .expect("BUG: unknown ERC-20 token") .clone(); (destination, value, token_symbol) diff --git a/rs/ethereum/cketh/minter/src/deposit.rs b/rs/ethereum/cketh/minter/src/deposit.rs index 7f084e159f8..4751db27ce2 100644 --- a/rs/ethereum/cketh/minter/src/deposit.rs +++ b/rs/ethereum/cketh/minter/src/deposit.rs @@ -52,7 +52,7 @@ async fn mint() { if let Some(result) = read_state(|s| { s.ckerc20_tokens .get_entry_alt(&event.erc20_contract_address) - .map(|(symbol, principal)| (symbol.to_string(), *principal)) + .map(|(principal, symbol)| (symbol.to_string(), *principal)) }) { result } else { diff --git a/rs/ethereum/cketh/minter/src/main.rs b/rs/ethereum/cketh/minter/src/main.rs index 1e68ae2eb65..f3c100163ae 100644 --- a/rs/ethereum/cketh/minter/src/main.rs +++ b/rs/ethereum/cketh/minter/src/main.rs @@ -15,6 +15,7 @@ use ic_cketh_minter::endpoints::{ RetrieveEthRequest, RetrieveEthStatus, WithdrawalArg, WithdrawalDetail, WithdrawalError, WithdrawalSearchParameter, }; +use ic_cketh_minter::erc20::CkTokenSymbol; use ic_cketh_minter::eth_logs::{EventSource, ReceivedErc20Event, ReceivedEthEvent}; use ic_cketh_minter::guard::retrieve_withdraw_guard; use ic_cketh_minter::ledger_client::{LedgerBurnError, LedgerClient}; @@ -300,9 +301,10 @@ async fn withdrawal_status(parameter: WithdrawalSearchParameter) -> Vec "ckETH".to_string(), + CkEth(_) => CkTokenSymbol::cketh_symbol_from_state(s).to_string(), CkErc20(r) => s - .ckerc20_token_symbol(&r.erc20_contract_address) + .ckerc20_tokens + .get_alt(&r.erc20_contract_address) .unwrap() .to_string(), }, diff --git a/rs/ethereum/cketh/minter/src/map.rs b/rs/ethereum/cketh/minter/src/map.rs index 822485de4ba..de7e565d63f 100644 --- a/rs/ethereum/cketh/minter/src/map.rs +++ b/rs/ethereum/cketh/minter/src/map.rs @@ -104,20 +104,6 @@ impl MultiKeyMap { .and_then(|alt_key| self.by_alt_key.get(alt_key).map(|v| (alt_key, v))) } - pub fn get_entry_alt(&self, alt_key: &Q) -> Option<(&Key, &V)> - where - AltKey: Borrow, - Q: Ord, - { - self.iter().find_map(|(key, alt_key_, v)| { - if alt_key_.borrow() == alt_key { - Some((key, v)) - } else { - None - } - }) - } - pub fn get_mut(&mut self, key: &Q) -> Option<&mut V> where Key: Borrow, @@ -227,3 +213,105 @@ impl FromIterator<(Key, AltKey, V)> Self { by_key, by_alt_key } } } + +/// A map with two keys: a primary key `Key` and an alternative key `AltKey`. +/// The stored value `V` is indexed by both keys and can be efficiently retrieved from either key. +/// +/// In comparison to `MultiKeyMap`, which only duplicates the alternative key, +/// both the primary key and the alternative key are duplicated. +/// This makes it possible to efficiently retrieve both the entry (AltKey, V) given the primary key +/// and the entry (Key, V) given the alternative key. +/// +/// Internally, this struct is a simple wrapper around `MultiKeyMap`. +#[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub struct DedupMultiKeyMap +where + Key: Ord, + AltKey: Ord, +{ + map: MultiKeyMap, +} + +impl DedupMultiKeyMap { + pub fn new() -> Self { + Self { + map: Default::default(), + } + } + + pub fn try_insert( + &mut self, + key: Key, + alt_key: AltKey, + value: V, + ) -> Result<(), OccupiedError> + where + Key: Clone, + AltKey: Clone, + { + self.map + .try_insert(key.clone(), alt_key.clone(), (key, value)) + .map_err(|err| OccupiedError { + occupied_key: err.occupied_key, + value: err.value.1, + }) + } + + pub fn get(&self, key: &Q) -> Option<&V> + where + Key: Borrow, + Q: Ord, + { + self.map.get(key).map(|(_key, value)| value) + } + + pub fn get_alt(&self, alt_key: &Q) -> Option<&V> + where + AltKey: Borrow, + Q: Ord, + { + self.map.get_alt(alt_key).map(|(_key, value)| value) + } + + pub fn get_entry(&self, key: &Q) -> Option<(&AltKey, &V)> + where + Key: Borrow, + Q: Ord, + { + self.map + .get_entry(key) + .map(|(alt_key, (_key, value))| (alt_key, value)) + } + + pub fn get_entry_alt(&self, alt_key: &Q) -> Option<(&Key, &V)> + where + AltKey: Borrow, + Q: Ord, + { + self.map.get_alt(alt_key).map(|(key, value)| (key, value)) + } + + pub fn contains_alt(&self, alt_key: &Q) -> bool + where + AltKey: Borrow, + Q: Ord, + { + self.map.contains_alt(alt_key) + } + + pub fn alt_keys(&self) -> impl Iterator { + self.map.alt_keys() + } + + pub fn iter(&self) -> impl Iterator { + self.map + .iter() + .map(|(key, alt_key, (_key, value))| (key, alt_key, value)) + } +} + +impl Default for DedupMultiKeyMap { + fn default() -> Self { + Self::new() + } +} diff --git a/rs/ethereum/cketh/minter/src/map/tests.rs b/rs/ethereum/cketh/minter/src/map/tests.rs index 6ce29f68cae..a8725aaf69b 100644 --- a/rs/ethereum/cketh/minter/src/map/tests.rs +++ b/rs/ethereum/cketh/minter/src/map/tests.rs @@ -15,10 +15,6 @@ fn should_insert_and_retrieve_by_key_and_alt_key() { Some((&AltKey::new('a'), &1)) ); assert_eq!(map.get_alt(&AltKey::new('a')), Some(&1)); - assert_eq!( - map.get_entry_alt(&AltKey::new('a')), - Some((&PrimaryKey::new(1), &1)) - ); assert_eq!(map.get(&PrimaryKey::new(2)), Some(&2)); assert_eq!( @@ -26,10 +22,6 @@ fn should_insert_and_retrieve_by_key_and_alt_key() { Some((&AltKey::new('b'), &2)) ); assert_eq!(map.get_alt(&AltKey::new('b')), Some(&2)); - assert_eq!( - map.get_entry_alt(&AltKey::new('b')), - Some((&PrimaryKey::new(2), &2)) - ); } #[test] diff --git a/rs/ethereum/cketh/minter/src/state.rs b/rs/ethereum/cketh/minter/src/state.rs index a3806205be0..1a9183fd98d 100644 --- a/rs/ethereum/cketh/minter/src/state.rs +++ b/rs/ethereum/cketh/minter/src/state.rs @@ -6,7 +6,7 @@ use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus}; use crate::lifecycle::upgrade::UpgradeArg; use crate::lifecycle::EthereumNetwork; use crate::logs::DEBUG; -use crate::map::MultiKeyMap; +use crate::map::DedupMultiKeyMap; use crate::numeric::{ BlockNumber, Erc20Value, LedgerBurnIndex, LedgerMintIndex, TransactionNonce, Wei, }; @@ -93,10 +93,10 @@ pub struct State { pub ledger_suite_orchestrator_id: Option, /// ERC-20 tokens that the minter can mint: - /// - primary key: ckERC20 token symbol + /// - primary key: ledger ID for the ckERC20 token /// - secondary key: ERC-20 contract address on Ethereum - /// - value: ledger ID for the ckERC20 token - pub ckerc20_tokens: MultiKeyMap, + /// - value: ckERC20 token symbol + pub ckerc20_tokens: DedupMultiKeyMap, } #[derive(Debug, Eq, PartialEq)] @@ -223,24 +223,20 @@ impl State { &self, ckerc20_ledger_id: &Principal, ) -> Option { - //TODO XC-83: refactor data structure for ckerc20_tokens - let results: Vec<_> = self - .supported_ck_erc20_tokens() - .filter(|supported_ckerc20_token| { - &supported_ckerc20_token.ckerc20_ledger_id == ckerc20_ledger_id + self.ckerc20_tokens + .get_entry(ckerc20_ledger_id) + .map(|(erc20_address, symbol)| CkErc20Token { + erc20_contract_address: *erc20_address, + ckerc20_ledger_id: *ckerc20_ledger_id, + erc20_ethereum_network: self.ethereum_network, + ckerc20_token_symbol: symbol.clone(), }) - .collect(); - assert!( - results.len() <= 1, - "BUG: multiple ckERC20 tokens with the same ledger ID {ckerc20_ledger_id}" - ); - results.into_iter().next() } pub fn supported_ck_erc20_tokens(&self) -> impl Iterator + '_ { self.ckerc20_tokens .iter() - .map(|(symbol, erc20_address, ledger_id)| CkErc20Token { + .map(|(ledger_id, erc20_address, symbol)| CkErc20Token { erc20_contract_address: *erc20_address, ckerc20_ledger_id: *ledger_id, erc20_ethereum_network: self.ethereum_network, @@ -248,19 +244,6 @@ impl State { }) } - pub fn ckerc20_token_symbol(&self, erc20_contract_address: &Address) -> Option<&CkTokenSymbol> { - self.ckerc20_tokens - .get_entry_alt(erc20_contract_address) - .map(|(symbol, _)| symbol) - } - - pub fn ckerc20_token_symbol_for_ledger(&self, ledger_id: &Principal) -> Option<&CkTokenSymbol> { - self.ckerc20_tokens - .iter() - .find(|(_, _, id)| *id == ledger_id) - .map(|(symbol, _, _)| symbol) - } - /// Quarantine the deposit event to prevent double minting. /// WARNING!: It's crucial that this method does not panic, /// since it's called inside the clean-up callback, when an unexpected panic did occur before. @@ -410,27 +393,25 @@ impl State { "ERROR: Expected {}, but got {}", self.ethereum_network, ckerc20_token.erc20_ethereum_network ); - let duplicate_ledger_id: MultiKeyMap<_, _, _> = self - .ckerc20_tokens - .iter() - .filter(|(_erc20_address, _ckerc20_token_symbol, &ledger_id)| { - ledger_id == ckerc20_token.ckerc20_ledger_id - }) - .collect(); + let ckerc20_with_same_symbol = self + .supported_ck_erc20_tokens() + .filter(|ckerc20| ckerc20.ckerc20_token_symbol == ckerc20_token.ckerc20_token_symbol) + .collect::>(); assert_eq!( - duplicate_ledger_id, - MultiKeyMap::default(), - "ERROR: ledger ID {} is already in use", - ckerc20_token.ckerc20_ledger_id + ckerc20_with_same_symbol, + vec![], + "ERROR: ckERC20 token symbol {} is already used by {:?}", + ckerc20_token.ckerc20_token_symbol, + ckerc20_with_same_symbol ); assert_eq!( self.ckerc20_tokens.try_insert( - ckerc20_token.ckerc20_token_symbol, - ckerc20_token.erc20_contract_address, ckerc20_token.ckerc20_ledger_id, + ckerc20_token.erc20_contract_address, + ckerc20_token.ckerc20_token_symbol, ), Ok(()), - "ERROR: some ckERC20 tokens use the same ERC-20 address or symbol" + "ERROR: some ckERC20 tokens use the same ckERC20 ledger ID or ERC-20 address" ); } @@ -440,7 +421,8 @@ impl State { .iter() .map(|(erc20_contract, balance)| { let symbol = self - .ckerc20_token_symbol(erc20_contract) + .ckerc20_tokens + .get_alt(erc20_contract) .unwrap_or_else(|| { panic!("BUG: missing symbol for ERC-20 contract {}", erc20_contract) }); diff --git a/rs/ethereum/cketh/minter/src/state/tests.rs b/rs/ethereum/cketh/minter/src/state/tests.rs index 4546ee0db0d..46adf1b9b39 100644 --- a/rs/ethereum/cketh/minter/src/state/tests.rs +++ b/rs/ethereum/cketh/minter/src/state/tests.rs @@ -6,6 +6,7 @@ use crate::eth_rpc_client::responses::{TransactionReceipt, TransactionStatus}; use crate::lifecycle::init::InitArg; use crate::lifecycle::upgrade::UpgradeArg; use crate::lifecycle::EthereumNetwork; +use crate::map::DedupMultiKeyMap; use crate::numeric::{ wei_from_milli_ether, BlockNumber, CkTokenAmount, Erc20Value, GasAmount, LedgerBurnIndex, LedgerMintIndex, LogIndex, TransactionNonce, Wei, WeiPerGas, @@ -430,10 +431,13 @@ mod erc20 { state.record_add_ckerc20_token(ckerc20.clone()); assert_eq!( - state - .ckerc20_tokens - .get_alt(&ckerc20.erc20_contract_address), - Some(&ckerc20.ckerc20_ledger_id) + state.supported_ck_erc20_tokens().collect::>(), + vec![CkErc20Token { + erc20_ethereum_network: EthereumNetwork::Mainnet, + erc20_contract_address: ckerc20.erc20_contract_address, + ckerc20_token_symbol: ckerc20.ckerc20_token_symbol, + ckerc20_ledger_id: ckerc20.ckerc20_ledger_id, + }] ); } @@ -450,7 +454,7 @@ mod erc20 { }; expect_panic_with_message( || state.record_add_ckerc20_token(ckusdt_with_wrong_ledger_id), - "ERROR: ledger ID", + "same ckERC20 ledger ID", ); } @@ -467,7 +471,7 @@ mod erc20 { }; expect_panic_with_message( || state.record_add_ckerc20_token(ckusdt_with_wrong_address), - "address", + "ERC-20 address", ); } @@ -977,14 +981,14 @@ fn state_equivalence() { }), }, }; - let mut ckerc20_tokens = MultiKeyMap::default(); + let mut ckerc20_tokens = DedupMultiKeyMap::default(); ckerc20_tokens .try_insert( - "ckUSDC".parse().unwrap(), + "mxzaz-hqaaa-aaaar-qaada-cai".parse().unwrap(), "0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48" .parse() .unwrap(), - "mxzaz-hqaaa-aaaar-qaada-cai".parse().unwrap(), + "ckUSDC".parse().unwrap(), ) .unwrap(); let state = State {