diff --git a/common/src/cache_transactions.rs b/common/src/cache_transactions.rs index 36a7543..c763712 100644 --- a/common/src/cache_transactions.rs +++ b/common/src/cache_transactions.rs @@ -10,19 +10,13 @@ use std::collections::BTreeMap; use crate::FundsTransfer; -const CACHE_MAX_LENGTH: usize = 100_000; // Keep at most 100k entries with the highest ids in the cache +const CACHE_MAX_LENGTH: usize = 100_000_000; // Keep at most 100M entries with the highest ids in the cache thread_local! { - /// Total count of transactions that were committed and are in stable memory - pub static CACHE_TXS_NUM_COMMITTED: RefCell = const { RefCell::new(0) }; - /// Recently committed transactions, that can be served from the get_transactions endpoint + /// Recently committed transactions, to serve from the get_transactions endpoint static RECENT_CACHE: RefCell> = const { RefCell::new(BTreeMap::new()) }; } -pub fn get_ledger_txs_num_committed() -> u64 { - CACHE_TXS_NUM_COMMITTED.with(|n| *n.borrow()) -} - /// Caches up to entries with the highest entry number. /// The entry key can be the transaction number or the block number, and we keep in the cache /// the entries with the highest entry number @@ -37,24 +31,31 @@ impl RecentCache { }) } + /// Append transaction to the cache + pub fn append_entry(tx: Transaction) { + RECENT_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + let tx_num = cache.keys().next_back().unwrap_or(&0) + 1; + Self::_add_entry(&mut cache, tx_num, tx) + }) + } + fn _add_entry(cache: &mut BTreeMap, tx_num: u64, tx: Transaction) { if cache.len() < CACHE_MAX_LENGTH || tx_num > *cache.keys().next().unwrap_or(&0) { // Only insert the entry if the cache is not full or if the new entry has a higher id than the minimal cache.insert(tx_num, tx); } - - // If the number of entries exceeds the maximum length, remove the oldest entries - while cache.len() > CACHE_MAX_LENGTH { - if let Some((&first_key, _)) = cache.iter().next() { - cache.remove(&first_key); - } - } } pub fn get_min_tx_num() -> Option { RECENT_CACHE.with(|cache| cache.borrow().keys().next().copied()) } + pub fn get_max_tx_num() -> Option { + RECENT_CACHE.with(|cache| cache.borrow().keys().next_back().copied()) + } + + // Get the current size of the cache, in number of transactions pub fn get_num_entries() -> usize { RECENT_CACHE.with(|cache| cache.borrow().len()) } @@ -87,11 +88,6 @@ impl RecentCache { }); } - // Get the current size of the cache - pub fn cache_size() -> usize { - RECENT_CACHE.with(|cache| cache.borrow().len()) - } - /// Parse transactions from a LedgerBlock and append transactions to the cache. /// tx_num_start is the lowest transaction number in the block. /// ledger_block is the LedgerBlock that contains the transactions. @@ -110,6 +106,20 @@ impl RecentCache { } }); } + + /// Ensure the cache does not exceed the maximum length. + pub fn ensure_cache_length() { + RECENT_CACHE.with(|cache| { + let mut cache = cache.borrow_mut(); + + // If the number of entries exceeds the maximum length, remove the oldest entries + while cache.len() > CACHE_MAX_LENGTH { + if let Some((&first_key, _)) = cache.iter().next() { + cache.remove(&first_key); + } + } + }); + } } #[cfg(test)] @@ -158,8 +168,10 @@ mod tests { for i in 0..CACHE_MAX_LENGTH + 10 { RecentCache::add_entry(i as u64, create_dummy_transaction(i as u64)); } + assert!(RecentCache::get_num_entries() >= CACHE_MAX_LENGTH); + RecentCache::ensure_cache_length(); - assert_eq!(RecentCache::cache_size(), CACHE_MAX_LENGTH); + assert_eq!(RecentCache::get_num_entries(), CACHE_MAX_LENGTH); assert_eq!(RecentCache::get_min_tx_num(), Some(10)); } @@ -181,10 +193,10 @@ mod tests { RecentCache::clear_cache(); RecentCache::add_entry(1, create_dummy_transaction(1)); - assert_eq!(RecentCache::cache_size(), 1); + assert_eq!(RecentCache::get_num_entries(), 1); let removed_tx = RecentCache::remove_transaction(1); assert!(removed_tx.is_some()); - assert_eq!(RecentCache::cache_size(), 0); + assert_eq!(RecentCache::get_num_entries(), 0); } #[test] @@ -194,9 +206,9 @@ mod tests { RecentCache::add_entry(i, create_dummy_transaction(i)); } - assert_eq!(RecentCache::cache_size(), 5); + assert_eq!(RecentCache::get_num_entries(), 5); RecentCache::clear_cache(); - assert_eq!(RecentCache::cache_size(), 0); + assert_eq!(RecentCache::get_num_entries(), 0); } #[test] @@ -239,7 +251,7 @@ mod tests { // Pretend that the first free transaction number is 899 RecentCache::parse_ledger_block(899, &ledger_block); - assert_eq!(RecentCache::cache_size(), 103); + assert_eq!(RecentCache::get_num_entries(), 103); assert_eq!(RecentCache::get_min_tx_num(), Some(899)); assert!(RecentCache::get_transaction(898).is_none()); assert!(RecentCache::get_transaction(899).is_some()); diff --git a/common/src/ledger_refresh.rs b/common/src/ledger_refresh.rs index 938cb1d..baecf8d 100644 --- a/common/src/ledger_refresh.rs +++ b/common/src/ledger_refresh.rs @@ -6,10 +6,10 @@ use crate::{ contracts_cache_open_remove, dcc_identity, error, reputations_apply_aging, reputations_apply_changes, reputations_clear, set_num_providers, set_num_users, set_offering_num_per_provider, AHashMap, ContractSignRequest, ContractSignRequestPayload, - ReputationAge, ReputationChange, UpdateOfferingPayload, CACHE_TXS_NUM_COMMITTED, - LABEL_CONTRACT_SIGN_REPLY, LABEL_CONTRACT_SIGN_REQUEST, LABEL_DC_TOKEN_APPROVAL, - LABEL_DC_TOKEN_TRANSFER, LABEL_NP_OFFERING, LABEL_NP_REGISTER, LABEL_REPUTATION_AGE, - LABEL_REPUTATION_CHANGE, LABEL_USER_REGISTER, PRINCIPAL_MAP, + ReputationAge, ReputationChange, UpdateOfferingPayload, LABEL_CONTRACT_SIGN_REPLY, + LABEL_CONTRACT_SIGN_REQUEST, LABEL_DC_TOKEN_APPROVAL, LABEL_DC_TOKEN_TRANSFER, + LABEL_NP_OFFERING, LABEL_NP_REGISTER, LABEL_REPUTATION_AGE, LABEL_REPUTATION_CHANGE, + LABEL_USER_REGISTER, PRINCIPAL_MAP, }; use borsh::BorshDeserialize; use candid::Principal; @@ -26,7 +26,6 @@ pub fn refresh_caches_from_ledger(ledger: &LedgerMap) -> anyhow::Result<()> { let mut replayed_blocks = 0; account_balances_clear(); reputations_clear(); - let mut num_txs = 0u64; let mut num_providers = 0u64; let mut num_users = 0u64; let mut principals: AHashMap> = HashMap::default(); @@ -73,8 +72,7 @@ pub fn refresh_caches_from_ledger(ledger: &LedgerMap) -> anyhow::Result<()> { account_balance_add(transfer.to(), transfer.amount())?; } - RecentCache::add_entry(num_txs, transfer.into()); - num_txs += 1; + RecentCache::append_entry(transfer.into()); } LABEL_DC_TOKEN_APPROVAL => { let approval = @@ -151,10 +149,13 @@ pub fn refresh_caches_from_ledger(ledger: &LedgerMap) -> anyhow::Result<()> { } replayed_blocks += 1; } - CACHE_TXS_NUM_COMMITTED.with(|n| *n.borrow_mut() = num_txs); PRINCIPAL_MAP.with(|p| *p.borrow_mut() = principals); set_num_providers(num_providers); set_num_users(num_users); - debug!("Refreshed caches from {} ledger blocks", replayed_blocks); + debug!( + "Refreshed caches from {} ledger blocks, found {} transactions", + replayed_blocks, + RecentCache::get_max_tx_num().unwrap_or_default() + ); Ok(()) } diff --git a/ic-canister/src/canister_backend/generic.rs b/ic-canister/src/canister_backend/generic.rs index 6350e98..813a797 100644 --- a/ic-canister/src/canister_backend/generic.rs +++ b/ic-canister/src/canister_backend/generic.rs @@ -1,15 +1,12 @@ use super::pre_icrc3::ledger_construct_hash_tree; -use borsh::BorshDeserialize; use candid::Principal; -use dcc_common::cache_transactions::RecentCache; use dcc_common::{ account_balance_get, account_registration_fee_e9s, blocks_until_next_halving, cursor_from_data, get_account_from_pubkey, get_num_offerings, get_num_providers, get_pubkey_from_principal, refresh_caches_from_ledger, reputation_get, reward_e9s_per_block_recalculate, rewards_current_block_checked_in, rewards_distribute, rewards_pending_e9s, set_test_config, - ContractId, ContractReqSerialized, FundsTransfer, LedgerCursor, TokenAmountE9s, - BLOCK_INTERVAL_SECS, CACHE_TXS_NUM_COMMITTED, DATA_PULL_BYTES_BEFORE_LEN, - LABEL_CONTRACT_SIGN_REQUEST, LABEL_DC_TOKEN_TRANSFER, LABEL_NP_CHECK_IN, LABEL_NP_OFFERING, + ContractId, ContractReqSerialized, LedgerCursor, TokenAmountE9s, BLOCK_INTERVAL_SECS, + DATA_PULL_BYTES_BEFORE_LEN, LABEL_CONTRACT_SIGN_REQUEST, LABEL_NP_CHECK_IN, LABEL_NP_OFFERING, LABEL_NP_PROFILE, LABEL_NP_REGISTER, LABEL_REWARD_DISTRIBUTION, LABEL_USER_REGISTER, MAX_RESPONSE_BYTES_NON_REPLICATED, }; @@ -21,7 +18,6 @@ use ledger_map::{error, info, warn, LedgerMap}; use serde::Serialize; use std::cell::RefCell; use std::io::prelude::*; -use std::ops::AddAssign; use std::time::Duration; thread_local! { @@ -71,25 +67,7 @@ fn ledger_periodic_task() { // Intentionally don't panic. If needed, transactions can be replayed and corrected. } - let mut tx_num = CACHE_TXS_NUM_COMMITTED.with(|n| *n.borrow()); - for entry in ledger.next_block_iter(Some(LABEL_DC_TOKEN_TRANSFER)) { - let transfer: FundsTransfer = BorshDeserialize::try_from_slice(entry.value()) - .unwrap_or_else(|e| { - ic_cdk::api::trap(&format!( - "Failed to deserialize transfer {:?} ==> {:?}", - entry, e - )); - }); - RecentCache::add_entry(tx_num, transfer.into()); - tx_num += 1; - } - - // Uncommitted transactions now get committed -- adjust the (cache) count of total committed transactions - let count_total_txs_uncommitted = - ledger.get_next_block_entries_count(Some(LABEL_DC_TOKEN_TRANSFER)) as u64; - - CACHE_TXS_NUM_COMMITTED.with(|n| n.borrow_mut().add_assign(count_total_txs_uncommitted)); - + // Commit the block ledger.commit_block().unwrap_or_else(|e| { error!("Failed to commit ledger: {}", e); }); @@ -98,6 +76,9 @@ fn ledger_periodic_task() { // Borrowed from https://github.com/ldclabs/ic-sft/blob/4825d760811731476ffbbb1705295a6ad4aae58f/src/ic_sft_canister/src/store.rs#L193-L210 let root_hash = ledger_construct_hash_tree(ledger).digest(); ic_cdk::api::set_certified_data(&root_hash); + + // Cleanup old transactions that are used for deduplication + crate::canister_backend::icrc1::cleanup_old_transactions(); }); } diff --git a/ic-canister/src/canister_backend/icrc1.rs b/ic-canister/src/canister_backend/icrc1.rs index 56795af..d228ca6 100644 --- a/ic-canister/src/canister_backend/icrc1.rs +++ b/ic-canister/src/canister_backend/icrc1.rs @@ -9,12 +9,16 @@ use ic_cdk::println; use crate::canister_backend::generic::LEDGER_MAP; use crate::DC_TOKEN_LOGO; +use crate::{ + DC_TOKEN_DECIMALS, DC_TOKEN_NAME, DC_TOKEN_SYMBOL, DC_TOKEN_TOTAL_SUPPLY, + DC_TOKEN_TRANSFER_FEE_E9S, MEMO_BYTES_MAX, MINTING_ACCOUNT, MINTING_ACCOUNT_ICRC1, +}; use candid::types::number::Nat; use candid::{CandidType, Principal}; use dcc_common::{ - account_balance_get, fees_sink_accounts, get_timestamp_ns, ledger_funds_transfer, - nat_to_balance, FundsTransfer, IcrcCompatibleAccount, TokenAmountE9s, PERMITTED_DRIFT, - TX_WINDOW, + account_balance_get, cache_transactions::RecentCache, fees_sink_accounts, get_timestamp_ns, + ledger_funds_transfer, nat_to_balance, AHashMap, FundsTransfer, IcrcCompatibleAccount, + TokenAmountE9s, PERMITTED_DRIFT, TX_WINDOW, }; use ic_cdk::caller; use icrc_ledger_types::icrc::generic_metadata_value::MetadataValue; @@ -22,18 +26,11 @@ use icrc_ledger_types::icrc1::account::Account as Icrc1Account; use icrc_ledger_types::icrc1::transfer::{Memo as Icrc1Memo, TransferError as Icrc1TransferError}; use serde::{Deserialize, Serialize}; use sha2::Digest; - -use crate::{ - DC_TOKEN_DECIMALS, DC_TOKEN_NAME, DC_TOKEN_SYMBOL, DC_TOKEN_TOTAL_SUPPLY, - DC_TOKEN_TRANSFER_FEE_E9S, MEMO_BYTES_MAX, MINTING_ACCOUNT, MINTING_ACCOUNT_ICRC1, -}; - use std::cell::RefCell; -use std::collections::HashMap; // Store transactions with their timestamps for cleanup thread_local! { - static RECENT_TRANSACTIONS: RefCell, u64>> = RefCell::new(HashMap::new()); + static RECENT_TRANSACTIONS: RefCell, u64>> = RefCell::new(AHashMap::default()); } fn compute_tx_hash(caller: Principal, arg: &TransferArg) -> Vec { @@ -55,8 +52,9 @@ fn compute_tx_hash(caller: Principal, arg: &TransferArg) -> Vec { hasher.finalize().to_vec() } -fn cleanup_old_transactions(now: u64) { +pub fn cleanup_old_transactions() { RECENT_TRANSACTIONS.with(|transactions| { + let now = get_timestamp_ns(); let mut txs = transactions.borrow_mut(); txs.retain(|_, timestamp| *timestamp > now.saturating_sub(TX_WINDOW)); }); @@ -72,9 +70,6 @@ fn check_duplicate_transaction( let tx_hash = compute_tx_hash(caller, arg); - // First cleanup old transactions - cleanup_old_transactions(now); - RECENT_TRANSACTIONS.with(|transactions| { let mut txs = transactions.borrow_mut(); @@ -221,24 +216,27 @@ pub fn _icrc1_transfer(arg: TransferArg) -> Result { // It's safe to subtract here because we checked above that the balance will not be negative let balance_from_after = balance_from_after.saturating_sub(fee); let mut ledger_ref = ledger.borrow_mut(); - ledger_funds_transfer( - &mut ledger_ref, - FundsTransfer::new( - from, - to, - Some(fee), - Some(fees_sink_accounts()), - Some(arg.created_at_time.unwrap_or(get_timestamp_ns())), - arg.memo.unwrap_or_default().0.into_vec(), - amount, - balance_from_after, - balance_to_after, - ), - ) - .unwrap_or_else(|err| ic_cdk::trap(&err.to_string())); - - let block_count = ledger_ref.get_blocks_count(); - Ok(block_count.into()) + let transfer = FundsTransfer::new( + from, + to, + Some(fee), + Some(fees_sink_accounts()), + Some(arg.created_at_time.unwrap_or(get_timestamp_ns())), + arg.memo.unwrap_or_default().0.into_vec(), + amount, + balance_from_after, + balance_to_after, + ); + ledger_funds_transfer(&mut ledger_ref, transfer.clone()) + .unwrap_or_else(|err| ic_cdk::trap(&err.to_string())); + + RecentCache::append_entry(transfer.into()); + RecentCache::get_max_tx_num().map(Nat::from).ok_or_else(|| { + Icrc1TransferError::GenericError { + error_code: Nat::from(10000u32), + message: "Failed to get max transaction number".to_string(), + } + }) }) } diff --git a/ic-canister/src/canister_backend/pre_icrc3.rs b/ic-canister/src/canister_backend/pre_icrc3.rs index 7d084bc..45cd0ef 100644 --- a/ic-canister/src/canister_backend/pre_icrc3.rs +++ b/ic-canister/src/canister_backend/pre_icrc3.rs @@ -1,10 +1,7 @@ use super::generic::LEDGER_MAP; use crate::canister_backend::generic::encode_to_cbor_bytes; use borsh::BorshDeserialize; -use dcc_common::{ - cache_transactions::RecentCache, get_ledger_txs_num_committed, FundsTransfer, - LABEL_DC_TOKEN_TRANSFER, -}; +use dcc_common::{cache_transactions::RecentCache, FundsTransfer, LABEL_DC_TOKEN_TRANSFER}; use ic_certification::{HashTreeNode, Label}; use icrc_ledger_types::icrc3::blocks::DataCertificate as DataCertificatePreIcrc3; use icrc_ledger_types::icrc3::transactions::{ @@ -23,12 +20,6 @@ pub fn _get_transactions(req: GetTransactionsRequest) -> GetTransactionsResponse .as_start_and_length() .unwrap_or_else(|msg| ic_cdk::api::trap(&msg)); - let count_total_txs_committed = get_ledger_txs_num_committed(); - let count_total_txs_uncommitted = LEDGER_MAP.with(|ledger| { - ledger - .borrow() - .get_next_block_entries_count(Some(LABEL_DC_TOKEN_TRANSFER)) - }) as u64; let mut txs = _get_committed_transactions(txs_from, txs_length); let txs_missing = txs_length.saturating_sub(txs.len() as u64); if txs_missing > 0 { @@ -38,7 +29,7 @@ pub fn _get_transactions(req: GetTransactionsRequest) -> GetTransactionsResponse GetTransactionsResponse { // We don't have archived transactions in this implementation, so the first_index is always the requested tx number first_index: txs_from.into(), - log_length: (count_total_txs_committed + count_total_txs_uncommitted).into(), + log_length: RecentCache::get_max_tx_num().unwrap_or_default().into(), transactions: txs, archived_transactions: vec![], } diff --git a/ic-canister/tests/test_icrc1.rs b/ic-canister/tests/test_icrc1.rs index 6b8aefd..c875be4 100644 --- a/ic-canister/tests/test_icrc1.rs +++ b/ic-canister/tests/test_icrc1.rs @@ -1,7 +1,10 @@ mod test_utils; use candid::{encode_one, Nat, Principal}; use dcc_common::{PERMITTED_DRIFT, TX_WINDOW}; -use icrc_ledger_types::icrc1::transfer::{Memo, TransferArg, TransferError}; +use icrc_ledger_types::{ + icrc1::transfer::{Memo, TransferArg, TransferError}, + icrc3::transactions::{GetTransactionsRequest, GetTransactionsResponse}, +}; use pocket_ic::WasmResult; use test_utils::{create_test_account, create_test_subaccount, TestContext}; @@ -37,7 +40,7 @@ fn test_basic_transfer() { Result ); - assert!(result.is_ok()); + assert_eq!(result.unwrap(), 1u32); // Check balances let from_balance = ctx.get_account_balance(&from); @@ -46,6 +49,85 @@ fn test_basic_transfer() { assert_eq!(to_balance, >::into(500_000_000u64)); } +#[test] +fn test_multiple_transfers() { + let ctx = TestContext::new(); + let from = create_test_account(1); + let to = create_test_account(2); + + // Mint some tokens to the sender + let original_tokens = 1_000_000_000; + ctx.mint_tokens_for_test(&from, original_tokens); + + // Get current timestamp and fee + let ts_ns = ctx.get_timestamp_ns(); + let fee = ctx.get_transfer_fee(); + let mut total_sent = 0; + let mut total_fees = Nat::from(0u64); + + let send_tx = |amount: u64| { + let transfer_arg = TransferArg { + from_subaccount: None, + to, + amount: amount.into(), + fee: Some(fee.clone()), + created_at_time: Some(ts_ns), + memo: None, + }; + update_check_and_decode!( + ctx.pic, + ctx.canister_id, + from.owner, + "icrc1_transfer", + candid::encode_one(transfer_arg).unwrap(), + Result + ) + }; + let get_tx = |start: u64, length: u64| -> GetTransactionsResponse { + let get_tx_arg = GetTransactionsRequest { + start: start.into(), + length: length.into(), + }; + query_check_and_decode!( + ctx.pic, + ctx.canister_id, + "get_transactions", + encode_one(get_tx_arg).unwrap(), + GetTransactionsResponse + ) + }; + + for block_num in 1u64..=10 { + let send_amount = 1_000u64 + block_num; + let result = send_tx(send_amount); + assert_eq!(result.unwrap(), block_num); + total_sent += send_amount; + total_fees += fee.clone(); + } + + let get_tx_response1 = get_tx(10, 1); + assert_eq!(get_tx_response1.first_index, 10u64); + assert_eq!(get_tx_response1.log_length, 10u8); + + assert_eq!(get_tx_response1.transactions.len(), 1); + assert_eq!(get_tx_response1.transactions[0].kind, "xfer".to_string()); + + ctx.ffwd_to_next_block(ts_ns); + + // should get the same result + let get_tx_response2 = get_tx(10, 1); + assert_eq!(get_tx_response1, get_tx_response2); + + // Check balances + let from_balance = ctx.get_account_balance(&from); + let to_balance = ctx.get_account_balance(&to); + assert_eq!( + from_balance, + Nat::from(original_tokens) - total_sent - total_fees + ); + assert_eq!(to_balance, total_sent); +} + #[test] fn test_duplicate_transaction() { let ctx = TestContext::new();