diff --git a/rs/ledger_suite/icrc1/test_utils/src/lib.rs b/rs/ledger_suite/icrc1/test_utils/src/lib.rs index 100445935b35..572a7b46800d 100644 --- a/rs/ledger_suite/icrc1/test_utils/src/lib.rs +++ b/rs/ledger_suite/icrc1/test_utils/src/lib.rs @@ -146,11 +146,21 @@ fn operation_strategy( fee, }); + let fee_collector_strategy = ( + prop::option::of(principal_strategy()), + prop::option::of(account_strategy()), + ) + .prop_map(move |(caller, fee_collector)| Operation::FeeCollector { + fee_collector, + caller, + }); + prop_oneof![ mint_strategy, burn_strategy, transfer_strategy, approve_strategy, + fee_collector_strategy, ] }) } diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_client.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_client.rs index 016e0e432f64..849f5f3aa49e 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_client.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_client.rs @@ -335,11 +335,12 @@ impl StorageClient { ) } - /// Retrieves the highest block index in the account balance table. - /// Returns None if the account balance table is empty. - pub fn get_highest_block_idx_in_account_balance_table(&self) -> Result> { + /// Retrieves the highest block index that was fully processed, + /// i.e. the block was synced and balances were updated according the the tx in the block. + /// Returns None if there are no processed blocks. + pub fn get_highest_processed_block_idx(&self) -> Result> { let open_connection = self.storage_connection.lock().unwrap(); - storage_operations::get_highest_block_idx_in_account_balance_table(&open_connection) + storage_operations::get_highest_processed_block_idx(&open_connection) } // Retrieves the account balance at a certain block height @@ -405,7 +406,8 @@ mod tests { use ic_icrc1::blocks::encoded_block_to_generic_block; use ic_icrc1::blocks::generic_block_to_encoded_block; use ic_icrc1_test_utils::{ - arb_amount, blocks_strategy, metadata_strategy, valid_blockchain_with_gaps_strategy, + arb_amount, blocks_strategy, metadata_strategy, valid_blockchain_strategy, + valid_blockchain_with_gaps_strategy, }; use ic_icrc1_tokens_u64::U64; use ic_icrc1_tokens_u256::U256; @@ -432,7 +434,7 @@ mod tests { proptest! { #[test] - fn test_read_and_write_blocks_u64(blockchain in prop::collection::vec(blocks_strategy::(arb_amount()),0..5)){ + fn test_read_and_write_blocks_u64(blockchain in valid_blockchain_strategy::(5)){ let storage_client_memory = StorageClient::new_in_memory().unwrap(); let mut rosetta_blocks = vec![]; for (index,block) in blockchain.into_iter().enumerate(){ @@ -459,7 +461,7 @@ mod tests { } #[test] - fn test_read_and_write_blocks_u256(blockchain in prop::collection::vec(blocks_strategy::(arb_amount()),0..5)){ + fn test_read_and_write_blocks_u256(blockchain in valid_blockchain_strategy::(5)){ let storage_client_memory = StorageClient::new_in_memory().unwrap(); let mut rosetta_blocks = vec![]; for (index,block) in blockchain.into_iter().enumerate(){ @@ -508,10 +510,11 @@ mod tests { // Duplicate the last transaction generated let duplicate_tx_block = RosettaBlock::from_generic_block(last_block.get_generic_block(), last_block.index + 1).unwrap(); + let count_before = storage_client_memory.get_transactions_by_hash(duplicate_tx_block.clone().get_transaction_hash()).unwrap().len(); storage_client_memory.store_blocks([duplicate_tx_block.clone()].to_vec()).unwrap(); - // The hash of the duplicated transaction should still be the same --> There should be two transactions with the same transaction hash. - assert_eq!(storage_client_memory.get_transactions_by_hash(duplicate_tx_block.clone().get_transaction_hash()).unwrap().len(),2); + // The hash of the duplicated transaction should still be the same --> There should be one more transaction with the same transaction hash. + assert_eq!(storage_client_memory.get_transactions_by_hash(duplicate_tx_block.clone().get_transaction_hash()).unwrap().len(), count_before + 1); } } diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs index 4d64b136c0d7..529860676417 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_operations.rs @@ -1,6 +1,6 @@ use crate::MetadataEntry; use crate::common::storage::types::{RosettaBlock, RosettaCounter}; -use anyhow::{Context, bail}; +use anyhow::{Context, anyhow, bail}; use candid::Nat; use ic_base_types::PrincipalId; use ic_ledger_core::tokens::Zero; @@ -15,6 +15,8 @@ use std::str::FromStr; use tracing::{info, trace}; pub const METADATA_SCHEMA_VERSION: &str = "schema_version"; +pub const METADATA_FEE_COL: &str = "fee_collector_107"; +pub const METADATA_BLOCK_IDX: &str = "highest_processed_block_index"; /// Gets the current value of a counter from the database. /// Returns None if the counter doesn't exist. @@ -108,6 +110,20 @@ pub fn initialize_counter_if_missing( Ok(()) } +pub fn get_107_fee_collector_or_legacy( + rosetta_block: &RosettaBlock, + connection: &Connection, + fee_collector_107: Option>, +) -> anyhow::Result> { + // First check if we have a 107 fee collector + if let Some(fee_collector_107) = fee_collector_107 { + return Ok(fee_collector_107); + } + + // There is no 107 fee collector, check legacy fee collector in the block + get_fee_collector_from_block(rosetta_block, connection) +} + // Helper function to resolve the fee collector account from a block pub fn get_fee_collector_from_block( rosetta_block: &RosettaBlock, @@ -269,7 +285,7 @@ pub fn update_account_balances( // The next block to be updated is the highest block index in the account balance table + 1 if the table is not empty and 0 otherwise let next_block_to_be_updated = - get_highest_block_idx_in_account_balance_table(connection)?.map_or(0, |idx| idx + 1); + get_highest_processed_block_idx(connection)?.map_or(0, |idx| idx + 1); let highest_block_idx = get_block_with_highest_block_idx(connection)?.map_or(0, |block| block.index); @@ -285,9 +301,22 @@ pub fn update_account_balances( // This also makes the inserting of the account balances batchable and therefore faster let mut account_balances_cache: HashMap> = HashMap::new(); + let mut current_fee_collector_107 = get_rosetta_metadata(connection, METADATA_FEE_COL)? + .map(|value| candid::decode_one::>(&value)) + .transpose()?; + let collector_before = current_fee_collector_107; + // As long as there are blocks to be fetched, keep on iterating over the blocks in the database with the given BATCH_SIZE interval while !rosetta_blocks.is_empty() { + let mut last_block_index = batch_start_idx; for rosetta_block in rosetta_blocks { + if rosetta_block.index < last_block_index { + bail!(format!( + "Processing blocks not in order, previous processed block: {last_block_index}, current block {}", + rosetta_block.index + )); + } + last_block_index = rosetta_block.index; match rosetta_block.get_transaction().operation { crate::common::storage::types::IcrcOperation::Burn { from, @@ -309,9 +338,11 @@ pub fn update_account_balances( connection, &mut account_balances_cache, )?; - if let Some(collector) = - get_fee_collector_from_block(&rosetta_block, connection)? - { + if let Some(collector) = get_107_fee_collector_or_legacy( + &rosetta_block, + connection, + current_fee_collector_107, + )? { credit( collector, fee, @@ -336,9 +367,11 @@ pub fn update_account_balances( connection, &mut account_balances_cache, )?; - if let Some(collector) = - get_fee_collector_from_block(&rosetta_block, connection)? - { + if let Some(collector) = get_107_fee_collector_or_legacy( + &rosetta_block, + connection, + current_fee_collector_107, + )? { credit( collector, fee, @@ -361,11 +394,21 @@ pub fn update_account_balances( .unwrap_or(Nat(BigUint::zero())); debit( from, - fee, + fee.clone(), rosetta_block.index, connection, &mut account_balances_cache, )?; + + if let Some(Some(collector)) = current_fee_collector_107 { + credit( + collector, + fee, + rosetta_block.index, + connection, + &mut account_balances_cache, + )?; + } } crate::common::storage::types::IcrcOperation::Transfer { from, @@ -397,9 +440,11 @@ pub fn update_account_balances( &mut account_balances_cache, )?; - if let Some(collector) = - get_fee_collector_from_block(&rosetta_block, connection)? - { + if let Some(collector) = get_107_fee_collector_or_legacy( + &rosetta_block, + connection, + current_fee_collector_107, + )? { credit( collector, fee, @@ -409,6 +454,12 @@ pub fn update_account_balances( )?; } } + crate::common::storage::types::IcrcOperation::FeeCollector { + fee_collector, + caller: _, + } => { + current_fee_collector_107 = Some(fee_collector); + } } } @@ -426,6 +477,13 @@ pub fn update_account_balances( })?; } } + if collector_before != current_fee_collector_107 { + let collector = current_fee_collector_107 + .ok_or_else(|| anyhow!("Cannot switch from fee collector 107 to legacy"))?; + insert_tx.prepare_cached("INSERT INTO rosetta_metadata (key, value) VALUES (?1, ?2) ON CONFLICT (key) DO UPDATE SET value = excluded.value;")?.execute(params![METADATA_FEE_COL, candid::encode_one(collector)?])?; + } + let last_block_index_bytes = last_block_index.to_le_bytes(); + insert_tx.prepare_cached("INSERT INTO rosetta_metadata (key, value) VALUES (?1, ?2) ON CONFLICT (key) DO UPDATE SET value = excluded.value;")?.execute(params![METADATA_BLOCK_IDX, last_block_index_bytes])?; insert_tx.commit()?; if flush_cache_and_shrink_memory { @@ -435,7 +493,7 @@ pub fn update_account_balances( } // Fetch the next batch of blocks - batch_start_idx = get_highest_block_idx_in_account_balance_table(connection)? + batch_start_idx = get_highest_processed_block_idx(connection)? .context("No blocks in account balance table after inserting")? + 1; batch_end_idx = batch_start_idx + batch_size; @@ -533,6 +591,22 @@ pub fn store_blocks( fee, expires_at, ), + crate::common::storage::types::IcrcOperation::FeeCollector { + fee_collector: _, + caller: _, + } => ( + "107feecol", + None, + None, + None, + None, + None, + None, + Nat::from(0u64), + None, + None, + None, + ), }; // SQLite doesn't support unsigned 64-bit integers. We need to convert the timestamps to signed @@ -685,16 +759,19 @@ pub fn get_blocks_by_transaction_hash( read_blocks(&mut stmt, params![hash.as_slice().to_vec()]) } -pub fn get_highest_block_idx_in_account_balance_table( - connection: &Connection, -) -> anyhow::Result> { - match connection - .prepare_cached("SELECT block_idx FROM account_balances WHERE block_idx = (SELECT MAX(block_idx) FROM account_balances)")? - .query_map(params![], |row| row.get(0))? - .next() - { - None => Ok(None), - Some(res) => Ok(res?), +pub fn get_highest_processed_block_idx(connection: &Connection) -> anyhow::Result> { + match get_rosetta_metadata(connection, METADATA_BLOCK_IDX)? { + Some(value) => Ok(Some(u64::from_le_bytes(value.as_slice().try_into()?))), + None => { + match connection + .prepare_cached("SELECT block_idx FROM account_balances WHERE block_idx = (SELECT MAX(block_idx) FROM account_balances)")? + .query_map(params![], |row| row.get(0))? + .next() + { + None => Ok(None), + Some(res) => Ok(res?), + } + } } } @@ -884,6 +961,10 @@ pub fn repair_fee_collector_balances( info!("Starting balance reconciliation..."); connection.execute("DELETE FROM account_balances", params![])?; + connection.execute( + &format!("DELETE FROM rosetta_metadata WHERE key = '{METADATA_BLOCK_IDX}' OR key = '{METADATA_FEE_COL}'"), + params![], + )?; if block_count > 0 { info!("Reprocessing all blocks..."); diff --git a/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs b/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs index 4f372bf3447a..52d7f8ef1d08 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/storage_operations_test.rs @@ -54,6 +54,7 @@ fn create_test_rosetta_block( effective_fee: None, fee_collector: None, fee_collector_block_index: None, + btype: None, }; RosettaBlock { @@ -108,6 +109,7 @@ fn create_test_approve_block( effective_fee: None, fee_collector: None, fee_collector_block_index: None, + btype: None, }; RosettaBlock { @@ -439,6 +441,20 @@ fn test_fee_collector_resolution_and_repair() -> anyhow::Result<()> { // Manually create broken balances (missing fee collector credits for block 2) connection.execute("DELETE FROM account_balances", params![])?; + // Insert metadata that needs to be cleared + connection.execute( + "INSERT INTO rosetta_metadata (key, value) VALUES (?1, ?2)", + params![METADATA_BLOCK_IDX, 100_000_000u64.to_le_bytes()], + )?; + let no_fee_col: Option = None; + connection.execute( + "INSERT INTO rosetta_metadata (key, value) VALUES (?1, ?2)", + params![ + METADATA_FEE_COL, + candid::encode_one(no_fee_col).expect("failed to encode fee collector") + ], + )?; + // Correct balances for mint and block 1 connection.execute("INSERT INTO account_balances (block_idx, principal, subaccount, amount) VALUES (0, ?1, ?2, '1000000000')", params![from_account.owner.as_slice(), from_account.effective_subaccount().as_slice()])?; diff --git a/rs/rosetta-api/icrc1/src/common/storage/types.rs b/rs/rosetta-api/icrc1/src/common/storage/types.rs index 3648a7bb985f..e4e8a0cad1a3 100644 --- a/rs/rosetta-api/icrc1/src/common/storage/types.rs +++ b/rs/rosetta-api/icrc1/src/common/storage/types.rs @@ -1,8 +1,6 @@ use anyhow::anyhow; use anyhow::bail; -use candid::CandidType; -use candid::Nat; -use candid::{Decode, Encode}; +use candid::{CandidType, Decode, Encode, Nat, Principal}; use ic_icrc1::blocks::encoded_block_to_generic_block; use ic_ledger_core::block::EncodedBlock; use ic_ledger_core::tokens::TokensType; @@ -138,6 +136,7 @@ impl RosettaBlock { IcrcOperation::Transfer { fee, .. } => fee, IcrcOperation::Approve { fee, .. } => fee, IcrcOperation::Burn { fee, .. } => fee, + IcrcOperation::FeeCollector { .. } => None, })) } @@ -182,6 +181,7 @@ pub struct IcrcBlock { pub timestamp: u64, pub fee_collector: Option, pub fee_collector_block_index: Option, + pub btype: Option, } impl IcrcBlock { @@ -217,6 +217,7 @@ impl TryFrom for IcrcBlock { .transpose()?; let timestamp = get_field::(&map, &[], "ts")?; let effective_fee = get_opt_field::(&map, &[], "fee")?; + let btype = get_opt_field::(&map, &[], "btype")?; let fee_collector = get_opt_field::(&map, &[], "fee_col")?; let fee_collector_block_index = get_opt_field::(&map, &[], "fee_col_block")?; let transaction = map.get("tx").ok_or(anyhow!("Missing field 'tx'"))?.clone(); @@ -229,6 +230,7 @@ impl TryFrom for IcrcBlock { timestamp, fee_collector, fee_collector_block_index, + btype, }) } } @@ -249,6 +251,9 @@ impl From for Value { if let Some(fee_col) = block.fee_collector { map.insert("fee_col".to_string(), Value::from(fee_col)); } + if let Some(btype) = block.btype { + map.insert("btype".to_string(), Value::Text(btype)); + } if let Some(fee_col_block) = block.fee_collector_block_index { map.insert( "fee_col_block".to_string(), @@ -363,6 +368,10 @@ pub enum IcrcOperation { expires_at: Option, fee: Option, }, + FeeCollector { + fee_collector: Option, + caller: Option, + }, } impl TryFrom> for IcrcOperation { @@ -370,7 +379,7 @@ impl TryFrom> for IcrcOperation { fn try_from(map: BTreeMap) -> anyhow::Result { const FIELD_PREFIX: &[&str] = &["tx"]; - let amount: Nat = get_field(&map, FIELD_PREFIX, "amt")?; + let amount: Option = get_opt_field(&map, FIELD_PREFIX, "amt")?; let fee: Option = get_opt_field(&map, FIELD_PREFIX, "fee")?; match get_field::(&map, FIELD_PREFIX, "op")?.as_str() { "burn" => { @@ -379,13 +388,19 @@ impl TryFrom> for IcrcOperation { Ok(Self::Burn { from, spender, - amount, + amount: amount + .ok_or_else(|| anyhow!("Missing field 'amt' for Burn operation"))?, fee, }) } "mint" => { let to: Account = get_field(&map, FIELD_PREFIX, "to")?; - Ok(Self::Mint { to, amount, fee }) + Ok(Self::Mint { + to, + amount: amount + .ok_or_else(|| anyhow!("Missing field 'amt' for Mint operation"))?, + fee, + }) } "xfer" => { let from: Account = get_field(&map, FIELD_PREFIX, "from")?; @@ -395,7 +410,8 @@ impl TryFrom> for IcrcOperation { from, to, spender, - amount, + amount: amount + .ok_or_else(|| anyhow!("Missing field 'amt' for Transfer operation"))?, fee, }) } @@ -408,12 +424,22 @@ impl TryFrom> for IcrcOperation { Ok(Self::Approve { from, spender, - amount, + amount: amount + .ok_or_else(|| anyhow!("Missing field 'amt' for Approve operation"))?, fee, expected_allowance, expires_at, }) } + "107set_fee_collector" => { + let fee_collector: Option = + get_opt_field(&map, FIELD_PREFIX, "fee_collector")?; + let caller: Option = get_opt_field(&map, FIELD_PREFIX, "caller")?; + Ok(Self::FeeCollector { + fee_collector, + caller, + }) + } found => { bail!( "Expected field 'op' to be 'burn', 'mint', 'xfer' or 'approve' but found {found}" @@ -495,6 +521,18 @@ impl From for BTreeMap { map.insert("fee".to_string(), Value::Nat(fee)); } } + Op::FeeCollector { + fee_collector, + caller, + } => { + map.insert("op".to_string(), Value::text("107set_fee_collector")); + if let Some(fee_collector) = fee_collector { + map.insert("fee_collector".to_string(), Value::from(fee_collector)); + } + if let Some(caller) = caller { + map.insert("caller".to_string(), Value::from(caller)); + } + } } map } @@ -608,9 +646,13 @@ where amount: amount.into(), fee: fee.map(Into::into), }, - Op::FeeCollector { .. } => { - panic!("FeeCollector107 not implemented") - } + Op::FeeCollector { + fee_collector, + caller, + } => Self::FeeCollector { + fee_collector, + caller, + }, } } } @@ -653,14 +695,15 @@ mod tests { use serde_bytes::ByteBuf; use std::collections::BTreeMap; + fn arb_principal() -> impl Strategy { + (vec(any::(), 0..30)).prop_map(|principal| Principal::from_slice(principal.as_slice())) + } + fn arb_account() -> impl Strategy { - (vec(any::(), 0..30), option::of(vec(any::(), 32))).prop_map( - |(owner, subaccount)| { - let owner = Principal::from_slice(owner.as_slice()); - let subaccount = subaccount.map(|v| v.try_into().unwrap()); - Account { owner, subaccount } - }, - ) + (arb_principal(), option::of(vec(any::(), 32))).prop_map(|(owner, subaccount)| { + let subaccount = subaccount.map(|v| v.try_into().unwrap()); + Account { owner, subaccount } + }) } fn arb_nat() -> impl Strategy { @@ -731,8 +774,25 @@ mod tests { }) } + fn arb_fee_collector() -> impl Strategy { + ( + option::of(arb_account()), // fee_collector + option::of(arb_principal()), // caller + ) + .prop_map(|(fee_collector, caller)| IcrcOperation::FeeCollector { + fee_collector, + caller, + }) + } + fn arb_op() -> impl Strategy { - prop_oneof![arb_approve(), arb_burn(), arb_mint(), arb_transfer(),] + prop_oneof![ + arb_approve(), + arb_burn(), + arb_mint(), + arb_transfer(), + arb_fee_collector(), + ] } fn arb_memo() -> impl Strategy { @@ -768,11 +828,18 @@ mod tests { fee_collector_block_index, )| IcrcBlock { parent_hash, - transaction, + transaction: transaction.clone(), effective_fee, timestamp, fee_collector, fee_collector_block_index, + btype: match transaction.operation { + IcrcOperation::FeeCollector { + fee_collector: _, + caller: _, + } => Some("107feecol".to_string()), + _ => None, + }, }, ) } @@ -972,6 +1039,19 @@ mod tests { assert_eq!(amount.into(), rosetta_amount, "amount"); assert_eq!(fee.map(|t| t.into()), rosetta_fee, "fee"); } + ( + ic_icrc1::Operation::FeeCollector { + fee_collector, + caller, + }, + IcrcOperation::FeeCollector { + fee_collector: rosetta_fee_collector, + caller: rosetta_caller, + }, + ) => { + assert_eq!(fee_collector, rosetta_fee_collector, "fee_collector"); + assert_eq!(caller, rosetta_caller, "caller"); + } (l, r) => panic!( "Found different type of operations. Operation:{l:?} rosetta's Operation:{r:?}" ), diff --git a/rs/rosetta-api/icrc1/src/common/types.rs b/rs/rosetta-api/icrc1/src/common/types.rs index 6f3605ab922c..3e62c473636f 100644 --- a/rs/rosetta-api/icrc1/src/common/types.rs +++ b/rs/rosetta-api/icrc1/src/common/types.rs @@ -1,6 +1,8 @@ use anyhow::Context; use axum::{Json, http::StatusCode, response::IntoResponse}; use candid::Deserialize; +use candid::Principal; +use icrc_ledger_types::icrc1::account::Account; use num_bigint::BigInt; use rosetta_core::identifiers::*; use rosetta_core::objects::*; @@ -308,6 +310,9 @@ pub struct BlockMetadata { // The Rosetta API standard field for timestamp is required in milliseconds // To ensure a lossless conversion we need to store the nano seconds for the timestamp pub block_created_at_nano_seconds: u64, + + #[serde(skip_serializing_if = "Option::is_none")] + pub btype: Option, } impl TryFrom for ObjectMap { @@ -346,6 +351,7 @@ impl BlockMetadata { effective_fee: block .effective_fee .map(|fee| Amount::new(BigInt::from(fee), currency)), + btype: block.btype, }) } } @@ -387,3 +393,36 @@ impl TryFrom for FeeMetadata { .context("Could not parse FeeMetadata from JSON object") } } + +#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)] +pub struct FeeCollectorMetadata { + pub fee_collector: Option, + pub caller: Option, +} + +impl TryFrom for ObjectMap { + type Error = anyhow::Error; + fn try_from(d: FeeCollectorMetadata) -> Result { + match serde_json::to_value(d) { + Ok(v) => match v { + serde_json::Value::Object(ob) => Ok(ob), + _ => anyhow::bail!( + "Could not convert FeeCollectorMetadata to ObjectMap. Expected type Object but received: {:?}", + v + ), + }, + Err(err) => anyhow::bail!( + "Could not convert FeeCollectorMetadata to ObjectMap: {:?}", + err + ), + } + } +} + +impl TryFrom for FeeCollectorMetadata { + type Error = anyhow::Error; + fn try_from(o: ObjectMap) -> anyhow::Result { + serde_json::from_value(serde_json::Value::Object(o)) + .context("Could not parse FeeCollectorMetadata from JSON object") + } +} diff --git a/rs/rosetta-api/icrc1/src/common/utils/utils.rs b/rs/rosetta-api/icrc1/src/common/utils/utils.rs index e522537b919e..6f7699976971 100644 --- a/rs/rosetta-api/icrc1/src/common/utils/utils.rs +++ b/rs/rosetta-api/icrc1/src/common/utils/utils.rs @@ -1,5 +1,5 @@ use crate::common::storage::types::{IcrcOperation, RosettaBlock}; -use crate::common::types::{FeeMetadata, FeeSetter}; +use crate::common::types::{FeeCollectorMetadata, FeeMetadata, FeeSetter}; use crate::{ AppState, MultiTokenAppState, common::{ @@ -9,7 +9,8 @@ use crate::{ }, }; use anyhow::{Context, bail}; -use candid::Nat; +use candid::{Nat, Principal}; +use icrc_ledger_types::icrc1::account::Account; use indicatif::{ProgressBar, ProgressState, ProgressStyle}; use num_bigint::BigInt; use rosetta_core::identifiers::*; @@ -149,6 +150,7 @@ pub fn rosetta_core_operations_to_icrc1_operation( Burn, Transfer, Approve, + FeeCollector, } // A builder which helps depict the icrc1 Operation and allows for an arbitrary order of rosetta_core Operations @@ -162,6 +164,8 @@ pub fn rosetta_core_operations_to_icrc1_operation( expected_allowance: Option, expires_at: Option, allowance: Option, + fee_collector: Option, + caller: Option, } impl IcrcOperationBuilder { @@ -176,6 +180,8 @@ pub fn rosetta_core_operations_to_icrc1_operation( expected_allowance: None, expires_at: None, allowance: None, + fee_collector: None, + caller: None, } } @@ -224,6 +230,16 @@ pub fn rosetta_core_operations_to_icrc1_operation( self } + pub fn with_fee_collector(mut self, fee_collector: Option) -> Self { + self.fee_collector = fee_collector; + self + } + + pub fn with_caller(mut self, caller: Option) -> Self { + self.caller = caller; + self + } + pub fn build(self) -> anyhow::Result { Ok(match self.icrc_operation.context("Icrc Operation type needs to be of type Mint, Burn, Transfer or Approve")? { IcrcOperation::Mint => { @@ -267,6 +283,10 @@ pub fn rosetta_core_operations_to_icrc1_operation( expected_allowance: self.expected_allowance, expires_at: self.expires_at, }}, + IcrcOperation::FeeCollector => crate::common::storage::types::IcrcOperation::FeeCollector{ + fee_collector: self.fee_collector, + caller: self.caller, + }, }) } } @@ -348,7 +368,6 @@ pub fn rosetta_core_operations_to_icrc1_operation( let fee = operation .amount .context("Amount field needs to be populated for Approve operation")?; - // The fee inside of icrc1 operation is always the fee set by the user let icrc1_operation_fee_set = match operation.metadata { Some(metadata) => { @@ -374,8 +393,16 @@ pub fn rosetta_core_operations_to_icrc1_operation( )?; icrc1_operation_builder.with_spender_accountidentifier(spender) } - // We do not have to convert this Operation on the icrc1 side as the crate::common::storage::types::IcrcOperation does not know anything about the FeeCollector - OperationType::FeeCollector => icrc1_operation_builder, + OperationType::FeeCollector => { + let metadata = operation + .metadata + .context("metadata should be set for fee collector operations")?; + let fc_metadata = FeeCollectorMetadata::try_from(metadata)?; + icrc1_operation_builder + .with_icrc_operation(IcrcOperation::FeeCollector) + .with_fee_collector(fc_metadata.fee_collector) + .with_caller(fc_metadata.caller) + } }; } icrc1_operation_builder.build() @@ -608,6 +635,25 @@ pub fn icrc1_operation_to_rosetta_core_operations( )); } } + crate::common::storage::types::IcrcOperation::FeeCollector { + fee_collector, + caller, + } => { + operations.push(rosetta_core::objects::Operation::new( + 0, + OperationType::FeeCollector.to_string(), + None, + None, + None, + Some( + FeeCollectorMetadata { + fee_collector, + caller, + } + .try_into()?, + ), + )); + } }; Ok(operations) @@ -622,32 +668,12 @@ pub fn icrc1_rosetta_block_to_rosetta_core_operations( ) -> anyhow::Result> { let icrc1_transaction = rosetta_block.get_transaction(); - let mut operations = icrc1_operation_to_rosetta_core_operations( + let operations = icrc1_operation_to_rosetta_core_operations( icrc1_transaction.operation, currency.clone(), rosetta_block.get_fee_paid()?, )?; - if let Some(fee_collector) = rosetta_block.get_fee_collector() - && let Some(_fee_payed) = rosetta_block.get_fee_paid()? - { - operations.push(rosetta_core::objects::Operation::new( - operations.len().try_into().unwrap(), - OperationType::FeeCollector.to_string(), - Some(fee_collector.into()), - Some(rosetta_core::objects::Amount::new( - BigInt::from( - rosetta_block - .get_fee_paid()? - .context("Fee payed needs to be populated for FeeCollector operation")? - .0, - ), - currency.clone(), - )), - None, - None, - )); - } Ok(operations) } @@ -714,6 +740,7 @@ pub fn rosetta_core_block_to_icrc1_block( ) }, fee_collector_block_index: block_metadata.fee_collector_block_index, + btype: block_metadata.btype, }) } diff --git a/rs/rosetta-api/icrc1/src/construction_api/utils.rs b/rs/rosetta-api/icrc1/src/construction_api/utils.rs index b88d3746f8d3..90eaaf56ddef 100644 --- a/rs/rosetta-api/icrc1/src/construction_api/utils.rs +++ b/rs/rosetta-api/icrc1/src/construction_api/utils.rs @@ -272,6 +272,9 @@ pub fn build_icrc1_ledger_canister_method_args( }) } } + crate::common::storage::types::IcrcOperation::FeeCollector { .. } => { + bail!("FeeCollector Operation not supported") + } } .context("Unable to encode canister method args") } @@ -298,6 +301,9 @@ fn extract_caller_principal_from_icrc1_ledger_operation( crate::common::storage::types::IcrcOperation::Transfer { from, spender, .. } => { spender.unwrap_or(*from).owner } + crate::common::storage::types::IcrcOperation::FeeCollector { .. } => { + bail!("FeeCollector Operation not supported") + } }) } diff --git a/rs/rosetta-api/icrc1/src/data_api/services.rs b/rs/rosetta-api/icrc1/src/data_api/services.rs index d622f755177e..015993dea61b 100644 --- a/rs/rosetta-api/icrc1/src/data_api/services.rs +++ b/rs/rosetta-api/icrc1/src/data_api/services.rs @@ -86,7 +86,7 @@ pub fn network_options(ledger_id: &Principal) -> NetworkOptionsResponse { pub fn network_status(storage_client: &StorageClient) -> Result { let highest_processed_block = storage_client - .get_highest_block_idx_in_account_balance_table() + .get_highest_processed_block_idx() .map_err(|e| Error::unable_to_find_block(&e))? .ok_or_else(|| { Error::unable_to_find_block(&"Highest processed block not found".to_owned()) @@ -538,7 +538,7 @@ pub fn initial_sync_is_completed( synched.unwrap() } else { let block_count = storage_client.get_block_count(); - let highest_index = storage_client.get_highest_block_idx_in_account_balance_table(); + let highest_index = storage_client.get_highest_processed_block_idx(); *synched = Some(match (block_count, highest_index) { // If the blockchain contains no blocks we mark it as not completed (Ok(block_count), Ok(Some(highest_index))) if block_count == highest_index + 1 => true, @@ -1167,15 +1167,27 @@ mod test { }; // We make sure that the service returns the correct number of transactions for each account - search_transactions_request.account_identifier = Some( - match rosetta_blocks[0].block.transaction.operation { - IcrcOperation::Transfer { from, .. } => from, - IcrcOperation::Mint { to, .. } => to, - IcrcOperation::Burn { from, .. } => from, - IcrcOperation::Approve { from, .. } => from, + for block in &rosetta_blocks { + search_transactions_request.account_identifier = + match block.block.transaction.operation { + IcrcOperation::Transfer { from, .. } => Some(from.into()), + IcrcOperation::Mint { to, .. } => Some(to.into()), + IcrcOperation::Burn { from, .. } => Some(from.into()), + IcrcOperation::Approve { from, .. } => Some(from.into()), + IcrcOperation::FeeCollector { + fee_collector: _, + caller: _, + } => None, + }; + if search_transactions_request.account_identifier.is_some() { + break; } - .into(), - ); + } + if search_transactions_request.account_identifier.is_none() { + // Only fee collector blocks found, we cannot search for transactions by accounts. + // This situation is similar to blockchain.is_empty() above. + return Ok(()); + } let num_of_transactions_with_account = rosetta_blocks .iter() @@ -1219,6 +1231,10 @@ mod test { .try_into() .unwrap(), ), + IcrcOperation::FeeCollector { + fee_collector: _, + caller: _, + } => false, }) .count(); @@ -1545,6 +1561,7 @@ mod test { timestamp: 1000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 0, )]; @@ -1613,6 +1630,7 @@ mod test { timestamp: 1000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 0, ), @@ -1632,6 +1650,7 @@ mod test { timestamp: 2000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 1, ), @@ -1782,6 +1801,7 @@ mod test { timestamp: 1000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, }, // Block 1: Transfer 300 from main account to subaccount1 @@ -1804,6 +1824,7 @@ mod test { timestamp: 2000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, }, // Block 2: Transfer 200 from main account to subaccount2 @@ -1826,6 +1847,7 @@ mod test { timestamp: 3000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, }, // Block 3: Transfer 150 from subaccount1 to other_account @@ -1848,6 +1870,7 @@ mod test { timestamp: 4000, fee_collector: None, fee_collector_block_index: None, + btype: None, }, }, ]; @@ -2138,6 +2161,7 @@ mod test { timestamp: 1, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 0, ), @@ -2158,6 +2182,7 @@ mod test { timestamp: 2, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 1, ), @@ -2178,6 +2203,7 @@ mod test { timestamp: 3, fee_collector: None, fee_collector_block_index: None, + btype: None, }, 2, ), @@ -2290,6 +2316,7 @@ mod test { timestamp: 1, fee_collector: None, fee_collector_block_index: None, + btype: None, }, block_id, )]; @@ -2317,6 +2344,7 @@ mod test { timestamp: 1, fee_collector: None, fee_collector_block_index: None, + btype: None, }, block_id, )]; diff --git a/rs/rosetta-api/icrc1/tests/system_tests.rs b/rs/rosetta-api/icrc1/tests/system_tests.rs index 51ea41998273..7a9bcf8e4f22 100644 --- a/rs/rosetta-api/icrc1/tests/system_tests.rs +++ b/rs/rosetta-api/icrc1/tests/system_tests.rs @@ -882,6 +882,222 @@ fn test_error_backoff() { }); } +async fn set_fee_col_107( + agent: &Agent, + env: &RosettaTestingEnvironment, + ledger_canister_id: &Principal, + start_index: u64, + prev_block_hash: Option>, + fee_col: Option, +) -> (u64, Vec) { + let mut builder = BlockBuilder::::new(start_index, start_index); + if let Some(prev_block_hash) = prev_block_hash { + builder = builder.with_parent_hash(prev_block_hash); + } + let fee_col_block = builder + .with_btype("107feecol".to_string()) + .fee_collector(fee_col, None, None) + .build(); + let block_index = add_block(agent, ledger_canister_id, &fee_col_block) + .await + .expect("failed to add block"); + assert_eq!(block_index, Nat::from(start_index)); + + let latest_rosetta_block = wait_for_rosetta_block( + &env.rosetta_client, + env.network_identifier.clone(), + start_index, + ) + .await + .expect("Unable to call wait_for_rosetta_block"); + assert_eq!(latest_rosetta_block, start_index); + + (start_index + 1, fee_col_block.hash().to_vec()) +} + +async fn transfer_and_check_collected_fees( + agent: &Agent, + env: &RosettaTestingEnvironment, + ledger_canister_id: &Principal, + start_index: u64, + prev_block_hash: Option>, + legacy_fee_col: Account, + expected_balances: Vec<(Account, u64)>, +) -> (u64, Vec) { + let mut idx = start_index; + let mut builder = BlockBuilder::new(idx, idx); + if let Some(prev_block_hash) = prev_block_hash { + builder = builder.with_parent_hash(prev_block_hash); + } + let mint = builder + .with_fee(Tokens::from(1u64)) + .with_fee_collector(legacy_fee_col) + .mint(*TEST_ACCOUNT, Tokens::from(1_000u64)) + .build(); + let block_index = add_block(agent, ledger_canister_id, &mint) + .await + .expect("failed to add block"); + assert_eq!(block_index, Nat::from(idx)); + idx += 1; + + let transfer = BlockBuilder::new(idx, idx) + .with_parent_hash(mint.hash().to_vec()) + .with_fee_collector_block(start_index) + .with_fee(Tokens::from(1u64)) + .transfer(*TEST_ACCOUNT, *TEST_ACCOUNT, Tokens::from(1u64)) + .build(); + let block_index = add_block(agent, ledger_canister_id, &transfer) + .await + .expect("failed to add block"); + assert_eq!(block_index, Nat::from(idx)); + idx += 1; + + let approve = BlockBuilder::new(idx, idx) + .with_parent_hash(transfer.hash().to_vec()) + .with_fee_collector_block(start_index) + .with_fee(Tokens::from(1u64)) + .approve(*TEST_ACCOUNT, *TEST_ACCOUNT, Tokens::from(u64::MAX)) + .build(); + let block_index = add_block(agent, ledger_canister_id, &approve) + .await + .expect("failed to add block"); + assert_eq!(block_index, Nat::from(idx)); + idx += 1; + + let transfer_from = BlockBuilder::new(idx, idx) + .with_parent_hash(approve.hash().to_vec()) + .with_fee(Tokens::from(1u64)) + .transfer(*TEST_ACCOUNT, *TEST_ACCOUNT, Tokens::from(1u64)) + .with_spender(*TEST_ACCOUNT) + .build(); + let block_index = add_block(agent, ledger_canister_id, &transfer_from) + .await + .expect("failed to add block"); + assert_eq!(block_index, Nat::from(idx)); + + for (account, balance) in expected_balances { + assert_rosetta_balance( + account, + idx, + balance, + &env.rosetta_client, + env.network_identifier.clone(), + ) + .await; + } + + (idx + 1, transfer_from.hash().to_vec()) +} + +#[test] +fn test_fee_collector_107() { + let rt = Runtime::new().unwrap(); + let setup = Setup::builder() + .with_custom_ledger_wasm(icrc3_test_ledger()) + .build(); + + rt.block_on(async { + let env = RosettaTestingEnvironmentBuilder::new(&setup).build().await; + + let agent = get_custom_agent(Arc::new(test_identity()), setup.port).await; + + let fc_legacy = Account { + owner: PrincipalId::new_user_test_id(111).into(), + subaccount: None, + }; + + let fc_107 = Account { + owner: PrincipalId::new_user_test_id(107).into(), + subaccount: None, + }; + + // There are 4 transactions with fees, but approve fees are not collected + // by the legacy fee collector and transfer from does not specify + // the fee collector or its index. + let (block_index, block_hash) = transfer_and_check_collected_fees( + &agent, + &env, + &env.icrc1_ledger_id, + 0, + None, + fc_legacy, + [(fc_legacy, 2), (fc_107, 0)].to_vec(), + ) + .await; + + let (block_index, block_hash) = set_fee_col_107( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + Some(fc_107), + ) + .await; + + // The 107 fee collector should collect all 4 fees. + let (block_index, block_hash) = transfer_and_check_collected_fees( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + fc_legacy, + [(fc_legacy, 2), (fc_107, 4)].to_vec(), + ) + .await; + + let (block_index, block_hash) = set_fee_col_107( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + None, + ) + .await; + + // Fee collecor was set to None, no fees should be collectes. + let (block_index, block_hash) = transfer_and_check_collected_fees( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + fc_legacy, + [(fc_legacy, 2), (fc_107, 4)].to_vec(), + ) + .await; + + let fc_107_new = Account { + owner: PrincipalId::new_user_test_id(108).into(), + subaccount: None, + }; + + let (block_index, block_hash) = set_fee_col_107( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + Some(fc_107_new), + ) + .await; + + // The new fee collector should collect all fees. + transfer_and_check_collected_fees( + &agent, + &env, + &env.icrc1_ledger_id, + block_index, + Some(block_hash), + fc_legacy, + [(fc_legacy, 2), (fc_107, 4), (fc_107_new, 4)].to_vec(), + ) + .await; + }); +} + const NUM_BLOCKS: u64 = 6; async fn verify_unrecognized_block_handling(setup: &Setup, bad_block_index: u64) { let mut log_file = NamedTempFile::new().expect("failed to create a temp file");