From 6049fec7224c94c2f600be82066057e6b0173a56 Mon Sep 17 00:00:00 2001 From: Tiago Loureiro <19423709+tiagoicp@users.noreply.github.com> Date: Fri, 29 Dec 2023 20:15:04 +0000 Subject: [PATCH 1/4] added filter of duplicated txs --- src/ICRC1/Canisters/Archive.mo | 13 ++++++++----- tests/ICRC1/Archive.ActorTest.mo | 22 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/ICRC1/Canisters/Archive.mo b/src/ICRC1/Canisters/Archive.mo index 74f7022..9ff8110 100644 --- a/src/ICRC1/Canisters/Archive.mo +++ b/src/ICRC1/Canisters/Archive.mo @@ -44,7 +44,6 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive stable let txStore = StableTrieMap.new(); public shared ({ caller }) func append_transactions(txs : [Transaction]) : async Result.Result<(), Text> { - if (caller != ledger_canister_id) { return #err("Unauthorized Access: Only the ledger canister can access this archive canister"); }; @@ -61,20 +60,24 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive switch (last_bucket) { case (?last_bucket) { + // Ensure no pre-registered transaction is stored + let last_tx_index = get_tx(last_bucket[last_bucket.size() - 1]).index; + let filtered_txs = Array.filter(txs, func tx = tx.index > last_tx_index); + let new_bucket = Iter.toArray( Itertools.take( Itertools.chain( last_bucket.vals(), - Iter.map(txs.vals(), store_tx), + Iter.map(filtered_txs.vals(), store_tx), ), BUCKET_SIZE, - ), + ) ); if (new_bucket.size() == BUCKET_SIZE) { let offset = (BUCKET_SIZE - last_bucket.size()) : Nat; - txs_iter := Itertools.fromArraySlice(txs, offset, txs.size()); + txs_iter := Itertools.fromArraySlice(filtered_txs, offset, txs.size()); } else { txs_iter := Itertools.empty(); }; @@ -160,7 +163,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive Iter.map( Itertools.take(iter, MAX_TRANSACTIONS_PER_REQUEST), get_tx, - ), + ) ); { transactions }; diff --git a/tests/ICRC1/Archive.ActorTest.mo b/tests/ICRC1/Archive.ActorTest.mo index 477197a..51dd758 100644 --- a/tests/ICRC1/Archive.ActorTest.mo +++ b/tests/ICRC1/Archive.ActorTest.mo @@ -7,6 +7,7 @@ import Float "mo:base/Float"; import Nat64 "mo:base/Nat64"; import Principal "mo:base/Principal"; import EC "mo:base/ExperimentalCycles"; +import Buffer "mo:base/Buffer"; import Archive "../../src/ICRC1/Canisters/Archive"; import T "../../src/ICRC1/Types"; @@ -55,7 +56,7 @@ module { func create_canister_and_add_cycles(n : Float) { EC.add( - CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)), + CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)) ); }; @@ -78,6 +79,25 @@ module { ]); }, ), + it( + "append_transactions() doesn't duplicate", + do { + create_canister_and_add_cycles(0.1); + let archive = await Archive.Archive(); + + let txs = new_txs(500); + ignore await archive.append_transactions(txs); + + // added 1 extra transaction that is not duplicated, and should be appended + let duplicated_txs = new_txs(501); + + assertAllTrue([ + (await archive.total_transactions()) == 500, + (await archive.append_transactions(duplicated_txs)) == #ok(), + (await archive.total_transactions()) == 501, + ]); + }, + ), it( "get_transaction()", do { From acdf3f6f28c9d3abdef06c069341da09733bd97a Mon Sep 17 00:00:00 2001 From: Tiago Loureiro <19423709+tiagoicp@users.noreply.github.com> Date: Fri, 29 Dec 2023 20:19:55 +0000 Subject: [PATCH 2/4] revert format/style changes --- src/ICRC1/Canisters/Archive.mo | 5 +++-- tests/ICRC1/Archive.ActorTest.mo | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ICRC1/Canisters/Archive.mo b/src/ICRC1/Canisters/Archive.mo index 9ff8110..c775c46 100644 --- a/src/ICRC1/Canisters/Archive.mo +++ b/src/ICRC1/Canisters/Archive.mo @@ -44,6 +44,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive stable let txStore = StableTrieMap.new(); public shared ({ caller }) func append_transactions(txs : [Transaction]) : async Result.Result<(), Text> { + if (caller != ledger_canister_id) { return #err("Unauthorized Access: Only the ledger canister can access this archive canister"); }; @@ -71,7 +72,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive Iter.map(filtered_txs.vals(), store_tx), ), BUCKET_SIZE, - ) + ), ); if (new_bucket.size() == BUCKET_SIZE) { @@ -163,7 +164,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive Iter.map( Itertools.take(iter, MAX_TRANSACTIONS_PER_REQUEST), get_tx, - ) + ), ); { transactions }; diff --git a/tests/ICRC1/Archive.ActorTest.mo b/tests/ICRC1/Archive.ActorTest.mo index 51dd758..428a9d5 100644 --- a/tests/ICRC1/Archive.ActorTest.mo +++ b/tests/ICRC1/Archive.ActorTest.mo @@ -56,7 +56,7 @@ module { func create_canister_and_add_cycles(n : Float) { EC.add( - CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)) + CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)), ); }; From 0e52175ef391c0ce761e0190b56b15acc01e3cf4 Mon Sep 17 00:00:00 2001 From: Tiago Loureiro <19423709+tiagoicp@users.noreply.github.com> Date: Sat, 30 Dec 2023 09:31:23 +0000 Subject: [PATCH 3/4] fixed edge case: when trailing_txs == 0 and refactored --- src/ICRC1/Canisters/Archive.mo | 36 +++++++++++++++++++++++++------- tests/ICRC1/Archive.ActorTest.mo | 16 ++++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/ICRC1/Canisters/Archive.mo b/src/ICRC1/Canisters/Archive.mo index c775c46..9e080e9 100644 --- a/src/ICRC1/Canisters/Archive.mo +++ b/src/ICRC1/Canisters/Archive.mo @@ -49,7 +49,10 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive return #err("Unauthorized Access: Only the ledger canister can access this archive canister"); }; - var txs_iter = txs.vals(); + // Ensure no pre-registered transaction is stored + let last_tx_index = get_last_tx_index(); + let filtered_txs = Array.filter(txs, func tx = tx.index > last_tx_index); + var txs_iter = filtered_txs.vals(); if (trailing_txs > 0) { let last_bucket = StableTrieMap.get( @@ -61,10 +64,6 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive switch (last_bucket) { case (?last_bucket) { - // Ensure no pre-registered transaction is stored - let last_tx_index = get_tx(last_bucket[last_bucket.size() - 1]).index; - let filtered_txs = Array.filter(txs, func tx = tx.index > last_tx_index); - let new_bucket = Iter.toArray( Itertools.take( Itertools.chain( @@ -72,7 +71,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive Iter.map(filtered_txs.vals(), store_tx), ), BUCKET_SIZE, - ), + ) ); if (new_bucket.size() == BUCKET_SIZE) { @@ -96,6 +95,29 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive #ok(); }; + func get_last_tx_index() : Int { + if (total_txs() == 0) return -1; + + let bucket_index = if (trailing_txs > 0) filled_buckets else Nat.max(filled_buckets - 1, 0); + let last_bucket_opt = StableTrieMap.get( + txStore, + Nat.equal, + U.hash, + bucket_index, + ); + + let last_bucket = switch (last_bucket_opt) { + case (?last_bucket) { last_bucket }; + case (null) { + Debug.trap("Unexpected Error: Last Bucket not found"); + }; + }; + + if (last_bucket.size() == 0) Debug.trap("Unexpected Error: Last Bucket is not filled"); + + get_tx(last_bucket[last_bucket.size() - 1]).index; + }; + func total_txs() : Nat { (filled_buckets * BUCKET_SIZE) + trailing_txs; }; @@ -164,7 +186,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive Iter.map( Itertools.take(iter, MAX_TRANSACTIONS_PER_REQUEST), get_tx, - ), + ) ); { transactions }; diff --git a/tests/ICRC1/Archive.ActorTest.mo b/tests/ICRC1/Archive.ActorTest.mo index 428a9d5..c8f2560 100644 --- a/tests/ICRC1/Archive.ActorTest.mo +++ b/tests/ICRC1/Archive.ActorTest.mo @@ -56,7 +56,7 @@ module { func create_canister_and_add_cycles(n : Float) { EC.add( - CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)), + CREATE_CANISTER + Int.abs(Float.toInt(n * 1_000_000_000_000)) ); }; @@ -85,16 +85,20 @@ module { create_canister_and_add_cycles(0.1); let archive = await Archive.Archive(); - let txs = new_txs(500); + let txs = new_txs(1000); ignore await archive.append_transactions(txs); // added 1 extra transaction that is not duplicated, and should be appended - let duplicated_txs = new_txs(501); + let duplicated_txs = new_txs(1001); + ignore await archive.append_transactions(duplicated_txs); + + // also testing when tx falls inside the bucket + let duplicated_and_inside_bucket_txs = new_txs(1501); assertAllTrue([ - (await archive.total_transactions()) == 500, - (await archive.append_transactions(duplicated_txs)) == #ok(), - (await archive.total_transactions()) == 501, + (await archive.total_transactions()) == 1001, + (await archive.append_transactions(duplicated_and_inside_bucket_txs)) == #ok(), + (await archive.total_transactions()) == 1501, ]); }, ), From 440d8cb9672dda82ba1d4ef685426a4449e85852 Mon Sep 17 00:00:00 2001 From: Tiago Loureiro <19423709+tiagoicp@users.noreply.github.com> Date: Sat, 30 Dec 2023 09:40:18 +0000 Subject: [PATCH 4/4] chore: removed unnecessary {} --- src/ICRC1/Canisters/Archive.mo | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ICRC1/Canisters/Archive.mo b/src/ICRC1/Canisters/Archive.mo index 9e080e9..cbcc185 100644 --- a/src/ICRC1/Canisters/Archive.mo +++ b/src/ICRC1/Canisters/Archive.mo @@ -107,10 +107,8 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive ); let last_bucket = switch (last_bucket_opt) { - case (?last_bucket) { last_bucket }; - case (null) { - Debug.trap("Unexpected Error: Last Bucket not found"); - }; + case (?last_bucket) last_bucket; + case (null) Debug.trap("Unexpected Error: Last Bucket not found"); }; if (last_bucket.size() == 0) Debug.trap("Unexpected Error: Last Bucket is not filled");