Skip to content

Add protection on Archive that avoids the Duplicate batch issue #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tiagoicp wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add protection on Archive that avoids the Duplicate batch issue #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6049fec
added filter of duplicated txs
tiagoicp Dec 29, 2023
acdf3f6
revert format/style changes
tiagoicp Dec 29, 2023
0e52175
fixed edge case: when trailing_txs == 0 and refactored
tiagoicp Dec 30, 2023
440d8cb
chore: removed unnecessary {}
tiagoicp Dec 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions src/ICRC1/Canisters/Archive.mo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T.Transaction>(txs, func tx = tx.index > last_tx_index);
var txs_iter = filtered_txs.vals();

if (trailing_txs > 0) {
let last_bucket = StableTrieMap.get(
Expand All @@ -65,16 +68,16 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive
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();
};
Expand All @@ -92,6 +95,27 @@ 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;
};
Expand Down Expand Up @@ -160,7 +184,7 @@ shared ({ caller = ledger_canister_id }) actor class Archive() : async T.Archive
Iter.map(
Itertools.take(iter, MAX_TRANSACTIONS_PER_REQUEST),
get_tx,
),
)
);

{ transactions };
Expand Down
26 changes: 25 additions & 1 deletion tests/ICRC1/Archive.ActorTest.mo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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))
);
};

Expand All @@ -78,6 +79,29 @@ module {
]);
},
),
it(
"append_transactions() doesn't duplicate",
do {
create_canister_and_add_cycles(0.1);
let archive = await Archive.Archive();

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(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()) == 1001,
(await archive.append_transactions(duplicated_and_inside_bucket_txs)) == #ok(),
(await archive.total_transactions()) == 1501,
]);
},
),
it(
"get_transaction()",
do {
Expand Down