feat(icp_index): FI-1219: Add SettledTransaction with TimeStamp field to ICP index

Author: mbjorkqvist

rs/rosetta-api/icp_ledger/index/BUILD.bazel

@@ -20,6 +20,7 @@ DEPENDENCIES = [
     "@crate_index//:serde",
     "@crate_index//:ic-stable-structures",
     "@crate_index//:ic-metrics-encoder",
+    "@crate_index//:serde_bytes",
     "@crate_index//:serde_json",
 ]
 
@@ -32,7 +33,6 @@ DEV_DEPENDENCIES = [
     "@crate_index//:assert_matches",
     "@crate_index//:candid_parser",
     "@crate_index//:proptest",
-    "@crate_index//:serde_bytes",
 ]
 
 MACRO_DEPENDENCIES = [

rs/rosetta-api/icp_ledger/index/Cargo.toml

@@ -26,6 +26,7 @@ serde = { workspace = true }
 ic-canisters-http-types = { path = "../../../rust_canisters/http_types" }
 ic-metrics-encoder = "1.1"
 ic-canister-log = { path = "../../../rust_canisters/canister_log" }
+serde_bytes = { workspace = true }
 serde_json = { workspace = true }
 ic-icrc1-index-ng = {path ="../../icrc1/index-ng"}
 
@@ -37,5 +38,4 @@ ic-state-machine-tests = { path = "../../../state_machine_tests" }
 ic-test-utilities-load-wasm = { path = "../../../test_utilities/load_wasm" }
 ic-icrc1 = { path = "../../icrc1" }
 ic-icp-index = { path = "./" }
-serde_bytes = { workspace = true }
 

rs/rosetta-api/icp_ledger/index/index.did

@@ -16,7 +16,7 @@ type GetAccountTransactionsArgs = record {
 type GetAccountIdentifierTransactionsError = record { message : text };
 type GetAccountIdentifierTransactionsResponse = record {
   balance : nat64;
-  transactions : vec TransactionWithId;
+  transactions : vec SettledTransactionWithId;
   oldest_tx_id : opt nat64;
 };
 type GetBlocksRequest = record { start : nat; length : nat };
@@ -53,13 +53,14 @@ type GetAccountIdentifierTransactionsResult = variant {
 type Status = record { num_blocks_synced : nat64 };
 type TimeStamp = record { timestamp_nanos : nat64 };
 type Tokens = record { e8s : nat64 };
-type Transaction = record {
+type SettledTransaction = record {
   memo : nat64;
   icrc1_memo : opt vec nat8;
   operation : Operation;
   created_at_time : opt TimeStamp;
+  timestamp : TimeStamp;
 };
-type TransactionWithId = record { id : nat64; transaction : Transaction };
+type SettledTransactionWithId = record { id : nat64; transaction : SettledTransaction };
 service : (InitArg) -> {
   get_account_identifier_balance : (text) -> (nat64) query;
   get_account_identifier_transactions : (

rs/rosetta-api/icp_ledger/index/src/lib.rs

@@ -1,6 +1,9 @@
 use candid::{CandidType, Deserialize, Principal};
 use ic_ledger_core::block::EncodedBlock;
-use icp_ledger::{AccountIdentifier, BlockIndex, Transaction};
+use ic_ledger_core::timestamp::TimeStamp;
+use icp_ledger::{AccountIdentifier, Block, BlockIndex, Memo, Operation};
+use serde_bytes::ByteBuf;
+
 pub mod logs;
 
 #[derive(CandidType, Debug, Deserialize)]
@@ -29,15 +32,39 @@ pub struct GetAccountIdentifierTransactionsArgs {
 }
 
 #[derive(CandidType, Clone, Debug, Deserialize, PartialEq, Eq)]
-pub struct TransactionWithId {
+pub struct SettledTransaction {
+    pub operation: Operation,
+    pub memo: Memo,
+    /// The time this transaction was created on the client side.
+    pub created_at_time: Option<TimeStamp>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub icrc1_memo: Option<ByteBuf>,
+    /// The time the block with this transaction was created.
+    pub timestamp: TimeStamp,
+}
+
+impl From<Block> for SettledTransaction {
+    fn from(block: Block) -> Self {
+        SettledTransaction {
+            operation: block.transaction.operation,
+            memo: block.transaction.memo,
+            created_at_time: block.transaction.created_at_time,
+            icrc1_memo: block.transaction.icrc1_memo,
+            timestamp: block.timestamp,
+        }
+    }
+}
+
+#[derive(CandidType, Clone, Debug, Deserialize, PartialEq, Eq)]
+pub struct SettledTransactionWithId {
     pub id: BlockIndex,
-    pub transaction: Transaction,
+    pub transaction: SettledTransaction,
 }
 
 #[derive(CandidType, Debug, Deserialize, PartialEq, Eq)]
 pub struct GetAccountIdentifierTransactionsResponse {
     pub balance: u64,
-    pub transactions: Vec<TransactionWithId>,
+    pub transactions: Vec<SettledTransactionWithId>,
     // The txid of the oldest transaction the account_identifier has
     pub oldest_tx_id: Option<BlockIndex>,
 }

rs/rosetta-api/icp_ledger/index/src/main.rs

@@ -7,7 +7,7 @@ use ic_icp_index::logs::{P0, P1};
 use ic_icp_index::{
     GetAccountIdentifierTransactionsArgs, GetAccountIdentifierTransactionsResponse,
     GetAccountIdentifierTransactionsResult, GetAccountTransactionsResult, InitArg, Log, LogEntry,
-    Priority, Status, TransactionWithId,
+    Priority, SettledTransaction, SettledTransactionWithId, Status,
 };
 use ic_icrc1_index_ng::GetAccountTransactionsArgs;
 use ic_ledger_core::block::{BlockType, EncodedBlock};
@@ -593,7 +593,7 @@ fn get_account_identifier_transactions(
     // TODO: deal with the user setting start to u64::MAX
     let start = arg.start.map_or(u64::MAX, |n| n);
     let key = account_identifier_block_ids_key(arg.account_identifier, start);
-    let mut transactions = vec![];
+    let mut settled_transactions = vec![];
     let indices = with_account_identifier_block_ids(|account_identifier_block_ids| {
         account_identifier_block_ids
             .range(key..)
@@ -613,21 +613,22 @@ fn get_account_identifier_transactions(
                 ));
             })
         });
-        let transaction = decode_encoded_block(id, block.into())
+        let settled_transaction = SettledTransaction::from(decode_encoded_block(id, block.into())
             .unwrap_or_else(|_| {
                 ic_cdk::api::trap(&format!(
-                "Block {} not found in the block log, account_identifier blocks map is corrupted!",id
-            ));
-            })
-            .transaction;
-        let transaction_with_idx = TransactionWithId { id, transaction };
-        transactions.push(transaction_with_idx);
+                    "Block {} not found in the block log, account_identifier blocks map is corrupted!",id))
+            }));
+        let transaction_with_idx = SettledTransactionWithId {
+            id,
+            transaction: settled_transaction,
+        };
+        settled_transactions.push(transaction_with_idx);
     }
     let oldest_tx_id = get_oldest_tx_id(arg.account_identifier);
     let balance = get_balance(arg.account_identifier);
     Ok(GetAccountIdentifierTransactionsResponse {
         balance,
-        transactions,
+        transactions: settled_transactions,
         oldest_tx_id,
     })
 }

rs/rosetta-api/icp_ledger/index/tests/tests.rs

@@ -2,7 +2,7 @@ use candid::{Decode, Encode, Nat};
 use ic_base_types::{CanisterId, PrincipalId};
 use ic_icp_index::{
     GetAccountIdentifierTransactionsArgs, GetAccountIdentifierTransactionsResponse,
-    GetAccountIdentifierTransactionsResult, Status, TransactionWithId,
+    GetAccountIdentifierTransactionsResult, SettledTransaction, SettledTransactionWithId, Status,
 };
 use ic_icrc1_index_ng::GetAccountTransactionsArgs;
 use ic_ledger_canister_core::archive::ArchiveOptions;
@@ -13,7 +13,7 @@ use ic_state_machine_tests::StateMachine;
 use icp_ledger::{
     AccountIdentifier, GetBlocksArgs, QueryEncodedBlocksResponse, MAX_BLOCKS_PER_REQUEST,
 };
-use icp_ledger::{FeatureFlags, LedgerCanisterInitPayload, Memo, Operation, Transaction};
+use icp_ledger::{FeatureFlags, LedgerCanisterInitPayload, Memo, Operation};
 use icrc_ledger_types::icrc1::account::Account;
 use icrc_ledger_types::icrc1::transfer::{BlockIndex, TransferArg, TransferError};
 use icrc_ledger_types::icrc2::approve::{ApproveArgs, ApproveError};
@@ -323,6 +323,8 @@ fn get_account_identifier_transactions(
     accountidentifier_txs
 }
 
+const SYNC_STEP_SECONDS: Duration = Duration::from_secs(60);
+
 // Helper function that calls tick on env until either
 // the index canister has synced all the blocks up to the
 // last one in the ledger or enough attempts passed and therefore
@@ -332,7 +334,7 @@ fn wait_until_sync_is_completed(env: &StateMachine, index_id: CanisterId, ledger
     let mut num_blocks_synced = u64::MAX;
     let mut chain_length = u64::MAX;
     for _i in 0..MAX_ATTEMPTS {
-        env.advance_time(Duration::from_secs(60));
+        env.advance_time(SYNC_STEP_SECONDS);
         env.tick();
         num_blocks_synced = status(env, index_id).num_blocks_synced;
         chain_length = icp_get_blocks(env, ledger_id).len() as u64;
@@ -344,22 +346,22 @@ fn wait_until_sync_is_completed(env: &StateMachine, index_id: CanisterId, ledger
 }
 
 #[track_caller]
-fn assert_tx_eq(tx1: &Transaction, tx2: &Transaction) {
+fn assert_tx_eq(tx1: &SettledTransaction, tx2: &SettledTransaction) {
     assert_eq!(tx1.operation, tx2.operation);
     assert_eq!(tx1.memo, tx2.memo);
-    assert_eq!(tx1.operation, tx2.operation);
-    assert_eq!(tx1.operation, tx2.operation);
+    assert_eq!(tx1.icrc1_memo, tx2.icrc1_memo);
+    assert_eq!(tx1.timestamp, tx2.timestamp);
 }
 
 // checks that two txs are equal minus the fields set by the ledger (e.g. timestamp)
 #[track_caller]
-fn assert_tx_with_id_eq(tx1: &TransactionWithId, tx2: &TransactionWithId) {
+fn assert_tx_with_id_eq(tx1: &SettledTransactionWithId, tx2: &SettledTransactionWithId) {
     assert_eq!(tx1.id, tx2.id, "id");
     assert_tx_eq(&tx1.transaction, &tx2.transaction);
 }
 
 #[track_caller]
-fn assert_txs_with_id_eq(txs1: Vec<TransactionWithId>, txs2: Vec<TransactionWithId>) {
+fn assert_txs_with_id_eq(txs1: Vec<SettledTransactionWithId>, txs2: Vec<SettledTransactionWithId>) {
     assert_eq!(
         txs1.len(),
         txs2.len(),
@@ -442,6 +444,18 @@ fn test_archive_indexing() {
     assert_ledger_index_parity(env, ledger_id, index_id);
 }
 
+fn expected_block_timestamp(phase: u32, start_time: SystemTime) -> TimeStamp {
+    TimeStamp::from(
+        start_time
+            .checked_add(
+                SYNC_STEP_SECONDS
+                    .checked_mul(phase)
+                    .expect("checked_mul should not overflow"),
+            )
+            .expect("checked_add should not overflow"),
+    )
+}
+
 #[test]
 fn test_get_account_identifier_transactions() {
     let mut initial_balances = HashMap::new();
@@ -454,22 +468,26 @@ fn test_get_account_identifier_transactions() {
     let index_id = install_index(env, ledger_id);
 
     // List of the transactions that the test is going to add. This exists to make
-    // the test easier to read
-    let tx0 = TransactionWithId {
+    // the test easier to read. The transactions are executed in separate phases, where the block
+    // timestamp is a function of the phase.
+    let mut phase = 0u32;
+    let tx0 = SettledTransactionWithId {
         id: 0u64,
-        transaction: Transaction {
+        transaction: SettledTransaction {
             operation: Operation::Mint {
                 to: account(1, 0).into(),
                 amount: Tokens::from_e8s(1_000_000_000_000_u64),
             },
             memo: Memo(0),
             created_at_time: None,
             icrc1_memo: None,
+            timestamp: expected_block_timestamp(phase, env.time()),
         },
     };
-    let tx1 = TransactionWithId {
+    phase = 1;
+    let tx1 = SettledTransactionWithId {
         id: 1u64,
-        transaction: Transaction {
+        transaction: SettledTransaction {
             operation: Operation::Transfer {
                 to: account(2, 0).into(),
                 from: account(1, 0).into(),
@@ -480,11 +498,13 @@ fn test_get_account_identifier_transactions() {
             memo: Memo(0),
             created_at_time: None,
             icrc1_memo: None,
+            timestamp: expected_block_timestamp(phase, env.time()),
         },
     };
-    let tx2 = TransactionWithId {
+    phase = 2;
+    let tx2 = SettledTransactionWithId {
         id: 2u64,
-        transaction: Transaction {
+        transaction: SettledTransaction {
             operation: Operation::Transfer {
                 to: account(2, 0).into(),
                 from: account(1, 0).into(),
@@ -495,11 +515,12 @@ fn test_get_account_identifier_transactions() {
             memo: Memo(0),
             created_at_time: None,
             icrc1_memo: None,
+            timestamp: expected_block_timestamp(phase, env.time()),
         },
     };
-    let tx3 = TransactionWithId {
+    let tx3 = SettledTransactionWithId {
         id: 3u64,
-        transaction: Transaction {
+        transaction: SettledTransaction {
             operation: Operation::Transfer {
                 to: account(1, 1).into(),
                 from: account(2, 0).into(),
@@ -510,17 +531,19 @@ fn test_get_account_identifier_transactions() {
             memo: Memo(0),
             created_at_time: None,
             icrc1_memo: None,
+            timestamp: expected_block_timestamp(phase, env.time()),
         },
     };
+    phase = 3;
     let expires_at = env
         .time()
         .duration_since(SystemTime::UNIX_EPOCH)
         .unwrap()
         .as_nanos() as u64
         + Duration::from_secs(3600).as_nanos() as u64;
-    let tx4 = TransactionWithId {
+    let tx4 = SettledTransactionWithId {
         id: 4u64,
-        transaction: Transaction {
+        transaction: SettledTransaction {
             operation: Operation::Approve {
                 from: account(1, 0).into(),
                 spender: account(4, 4).into(),
@@ -532,6 +555,7 @@ fn test_get_account_identifier_transactions() {
             memo: Memo(0),
             created_at_time: None,
             icrc1_memo: None,
+            timestamp: expected_block_timestamp(phase, env.time()),
         },
     };
 
@@ -643,16 +667,17 @@ fn test_get_account_transactions_start_length() {
         );
     }
     let expected_txs: Vec<_> = (0..10)
-        .map(|i| TransactionWithId {
+        .map(|i| SettledTransactionWithId {
             id: i,
-            transaction: Transaction {
+            transaction: SettledTransaction {
                 operation: Operation::Mint {
                     to: account(1, 0).into(),
                     amount: Tokens::from_e8s(i * 10_000),
                 },
                 memo: Memo(0),
                 created_at_time: None,
                 icrc1_memo: None,
+                timestamp: TimeStamp::from(env.time()),
             },
         })
         .collect();
@@ -731,7 +756,7 @@ fn test_get_account_identifier_transactions_pagination() {
         }
 
         let mut last_seen_txid = start;
-        for TransactionWithId { id, transaction } in &res.transactions {
+        for SettledTransactionWithId { id, transaction } in &res.transactions {
             // transactions ids must be unique and in descending order
             if let Some(last_seen_txid) = last_seen_txid {
                 assert!(*id < last_seen_txid);
@@ -740,14 +765,19 @@ fn test_get_account_identifier_transactions_pagination() {
 
             // check the transaction itself
             assert_tx_eq(
-                &Transaction {
+                &SettledTransaction {
                     operation: Operation::Mint {
                         to: account(1, 0).into(),
                         amount: Tokens::from_e8s(*id * 10_000),
                     },
                     memo: Memo(0),
                     created_at_time: None,
                     icrc1_memo: None,
+                    timestamp: TimeStamp::from(
+                        env.time()
+                            .checked_sub(SYNC_STEP_SECONDS)
+                            .expect("should not underflow"),
+                    ),
                 },
                 transaction,
             );