Skip to content

Commit

Permalink
add deduplication to send
Browse files Browse the repository at this point in the history
  • Loading branch information
NikolasHaimerl committed Aug 11, 2023
1 parent e6fcbce commit 1f6bd83
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 10 deletions.
3 changes: 1 addition & 2 deletions cycles-ledger/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ fn validate_memo(memo: Option<Memo>) -> Option<Memo> {
fn deposit(arg: endpoints::DepositArg) -> endpoints::DepositResult {
let cycles_available = msg_cycles_available128();

// TODO(FI-767): Implement deduplication.
// TODO(FI-884): Implement deduplication.

let amount = msg_cycles_accept128(cycles_available);
if amount <= config::FEE {
Expand Down Expand Up @@ -219,7 +219,6 @@ async fn send(args: endpoints::SendArg) -> Result<Nat, SendError> {
canister_id: args.to,
};

// TODO(FI-767): Implement deduplication.
let amount = match args.amount.0.to_u128() {
Some(value) => value,
None => {
Expand Down
24 changes: 20 additions & 4 deletions cycles-ledger/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ pub fn deduplicate(
) -> Result<(), TransferError> {
// TODO: purge old transactions
if let (Some(created_at_time), tx_hash) = (created_at_timestamp, tx_hash) {
// The caller requested deduplication.
// If the created timestamp is outside of the Transaction
if created_at_time + (config::TRANSACTION_WINDOW.as_nanos() as u64) < now {
return Err(TransferError::TooOld);
}
Expand All @@ -261,9 +261,25 @@ pub fn deduplicate(
}

if let Some(block_height) = read_state(|state| state.operations.get(&tx_hash)) {
return Err(TransferError::Duplicate {
duplicate_of: Nat::from(block_height),
});
if let Some(block) = read_state(|state| state.blocks.get(block_height)) {
if block
.0
.timestamp
.saturating_add(config::TRANSACTION_WINDOW.as_nanos() as u64)
.saturating_add(config::PERMITTED_DRIFT.as_nanos() as u64)
< now
{
if !mutate_state(|state| state.operations.remove(&tx_hash)).is_some() {
ic_cdk::trap(&format!("Could not remove tx hash {:?}", tx_hash))
}
} else {
return Err(TransferError::Duplicate {
duplicate_of: Nat::from(block_height),
});
}
} else {
ic_cdk::trap(&format!("Could not find block index {}", block_height))
}
}
}
Ok(())
Expand Down
15 changes: 11 additions & 4 deletions cycles-ledger/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,11 @@ fn test_send_flow() {
);

// send cycles from subaccount with created_at_time set
let now = env
.time()
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_nanos() as u64;
let send_receiver_balance = env.cycle_balance(send_receiver);
let send_amount = 300_000_000_u128;
let _send_idx = send(
Expand All @@ -238,7 +243,7 @@ fn test_send_flow() {
from_subaccount: Some(*user_subaccount_3.effective_subaccount()),
to: send_receiver,
fee: None,
created_at_time: Some(100_u64),
created_at_time: Some(now),
memo: None,
amount: Nat::from(send_amount),
},
Expand Down Expand Up @@ -481,7 +486,7 @@ fn test_transfer() {
let fee = fee(env, ledger_id);

let transfer_amount = Nat::from(100_000);
transfer(
let idx = transfer(
env,
ledger_id,
user1,
Expand Down Expand Up @@ -568,7 +573,9 @@ fn test_transfer() {
.unwrap_err()
);

// Should be able to make a transfer when created time is valid
// When making the same transaction again
assert_eq!(
TransferError::Duplicate { duplicate_of: idx },
transfer(
env,
ledger_id,
Expand All @@ -582,5 +589,5 @@ fn test_transfer() {
amount: transfer_amount.clone(),
},
)
.unwrap();
.unwrap_err());
}

0 comments on commit 1f6bd83

Please sign in to comment.