From d82ae2610ffdbec8fac928253155aadfd6add0fa Mon Sep 17 00:00:00 2001 From: Grace-CODE-D Date: Wed, 26 Aug 2026 16:29:16 +0000 Subject: [PATCH 1/4] test(router-timelock): add test coverage for cancel() error paths --- contracts/router-timelock/src/lib.rs | 68 ++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/contracts/router-timelock/src/lib.rs b/contracts/router-timelock/src/lib.rs index d56dd5a1..557faca2 100644 --- a/contracts/router-timelock/src/lib.rs +++ b/contracts/router-timelock/src/lib.rs @@ -1110,6 +1110,74 @@ mod tests { assert_eq!(topic, Symbol::new(&env, router_common::EVENT_OP_CANCELLED)); } + #[test] + fn test_cancel_nonexistent_op_fails() { + let (env, admin, client) = setup(); + let fake_id = Bytes::from_array(&env, &[0u8; 32]); + assert_eq!( + client.try_cancel(&admin, &fake_id), + Err(Ok(TimelockError::NotFound)) + ); + } + + #[test] + fn test_cancel_unauthorized_fails() { + let (env, admin, client) = setup(); + let attacker = Address::generate(&env); + let target = Address::generate(&env); + let op_id = client.queue( + &admin, + &String::from_str(&env, "d"), + &target, + &3600, + &GRACE, + &Vec::new(&env), + ); + assert_eq!( + client.try_cancel(&attacker, &op_id), + Err(Ok(TimelockError::Unauthorized)) + ); + } + + #[test] + fn test_cancel_already_cancelled_fails() { + let (env, admin, client) = setup(); + let target = Address::generate(&env); + let op_id = client.queue( + &admin, + &String::from_str(&env, "d"), + &target, + &3600, + &GRACE, + &Vec::new(&env), + ); + client.cancel(&admin, &op_id); + assert_eq!( + client.try_cancel(&admin, &op_id), + Err(Ok(TimelockError::Cancelled)) + ); + } + + #[test] + fn test_cancel_already_executed_fails() { + let (env, admin, client) = setup(); + let target = Address::generate(&env); + let op_id = client.queue( + &admin, + &String::from_str(&env, "d"), + &target, + &3600, + &GRACE, + &Vec::new(&env), + ); + env.ledger().with_mut(|l| l.timestamp += 3601); + client.execute(&admin, &op_id); + assert_eq!( + client.try_cancel(&admin, &op_id), + Err(Ok(TimelockError::AlreadyExecuted)) + ); + } + // ── validation ──────────────────────────────────────────────────────────── #[test] From 3e7d0b639262ccb5ce88f5593ab309c0acaacc73 Mon Sep 17 00:00:00 2001 From: Grace-CODE-D Date: Wed, 26 Aug 2026 16:29:48 +0000 Subject: [PATCH 2/4] fix(router-execution): add missing args and amount fields to ExecutionRequest in tests --- contracts/router-execution/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/router-execution/src/lib.rs b/contracts/router-execution/src/lib.rs index 38e9ad16..40fcb137 100644 --- a/contracts/router-execution/src/lib.rs +++ b/contracts/router-execution/src/lib.rs @@ -1721,6 +1721,8 @@ mod tests { function: function.clone(), simulate_first: false, max_retries: 0, + args: Vec::new(&env), + amount: 1_000_000, }; let result = client.execute(&caller, &request); @@ -1773,6 +1775,8 @@ mod tests { function: function.clone(), simulate_first: false, max_retries: 2, + args: Vec::new(&env), + amount: 1_000_000, }; let result = client.execute(&caller, &request); From 8c613748c537334222729f28ec3a3e75c0460861 Mon Sep 17 00:00:00 2001 From: Grace-CODE-D Date: Wed, 26 Aug 2026 16:30:07 +0000 Subject: [PATCH 3/4] refactor(router-access): compute expiry timestamp once in grant_role_internal --- contracts/router-access/src/lib.rs | 37 +++++++++++------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/contracts/router-access/src/lib.rs b/contracts/router-access/src/lib.rs index 16f48b13..4f4f0645 100644 --- a/contracts/router-access/src/lib.rs +++ b/contracts/router-access/src/lib.rs @@ -805,28 +805,28 @@ impl RouterAccess { let raw_key = DataKey::HasRole(role.clone(), account.clone()); let has_raw_assignment = env.storage().instance().has(&raw_key); + let expiry_timestamp = match expires_in { + Some(seconds) => env + .ledger() + .timestamp() + .checked_add(seconds) + .ok_or(AccessError::InvalidExpiry)?, + None => u64::MAX, + }; + // If there is an existing unexpired assignment, only treat it as a duplicate error when // the requested expiry matches the existing expiry. // // This allows admins to extend/shorten expiry (or remove it by granting with `None`). let currently_active = has_raw_assignment && Self::has_role_internal(env, account, role); if currently_active { - let existing_expiry: Option = env + let existing_expiry = env .storage() .instance() - .get::(&DataKey::RoleExpiry(role.clone(), account.clone())); - - let requested_expiry = match expires_in { - Some(seconds) => env - .ledger() - .timestamp() - .checked_add(seconds) - .ok_or(AccessError::InvalidExpiry)?, - None => u64::MAX, - }; - - let existing_expiry = existing_expiry.unwrap_or(u64::MAX); - if existing_expiry == requested_expiry { + .get::(&DataKey::RoleExpiry(role.clone(), account.clone())) + .unwrap_or(u64::MAX); + + if existing_expiry == expiry_timestamp { return Err(AccessError::AlreadyHasRole); } } @@ -834,15 +834,6 @@ impl RouterAccess { // Track this role in AllRoles if it's the first time we've seen it Self::track_role_in_all_roles(env, role)?; - let expiry_timestamp = match expires_in { - Some(seconds) => env - .ledger() - .timestamp() - .checked_add(seconds) - .ok_or(AccessError::InvalidExpiry)?, - None => u64::MAX, - }; - env.storage() .instance() .set(&DataKey::HasRole(role.clone(), account.clone()), &true); From f88a8d5c2a139dcbeb0788ca7f90e34e6cab3714 Mon Sep 17 00:00:00 2001 From: Grace-CODE-D Date: Wed, 26 Aug 2026 16:30:51 +0000 Subject: [PATCH 4/4] refactor(router-quote): extract find_insertion_index helper --- contracts/router-quote/src/lib.rs | 33 +++++++++++++++++-------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/contracts/router-quote/src/lib.rs b/contracts/router-quote/src/lib.rs index 03fc3b7a..7838cde1 100644 --- a/contracts/router-quote/src/lib.rs +++ b/contracts/router-quote/src/lib.rs @@ -111,6 +111,18 @@ const MAX_TRACKED_ROUTES: u32 = 500; #[contract] pub struct RouterQuote; +/// Returns the index at which `key` should be inserted into `len` items so +/// the sequence stays ordered, given a comparator `should_insert_before` +/// that returns true when the new item belongs before the item at `idx`. +fn find_insertion_index(len: u32, mut should_insert_before: impl FnMut(u32) -> bool) -> u32 { + for idx in 0..len { + if should_insert_before(idx) { + return idx; + } + } + len +} + #[contractimpl] impl RouterQuote { /// Initialize the quote contract with an admin address and default fee. @@ -269,14 +281,9 @@ impl RouterQuote { return Err(QuoteError::InvalidFeeBps); } - let mut position = sorted_tiers.len(); - for index in 0..sorted_tiers.len() { - let current = sorted_tiers.get(index).unwrap(); - if tier.min_amount < current.min_amount { - position = index; - break; - } - } + let position = find_insertion_index(sorted_tiers.len(), |idx| { + tier.min_amount < sorted_tiers.get(idx).unwrap().min_amount + }); sorted_tiers.insert(position, tier.clone()); } @@ -502,13 +509,9 @@ impl RouterQuote { for request in requests.iter() { if let Ok(response) = Self::get_quote(env.clone(), request) { // Insertion sort: find position where response.amount_out fits. - let mut pos = sorted.len(); - for i in 0..sorted.len() { - if response.amount_out > sorted.get(i).unwrap().amount_out { - pos = i; - break; - } - } + let pos = find_insertion_index(sorted.len(), |idx| { + response.amount_out > sorted.get(idx).unwrap().amount_out + }); sorted.insert(pos, response); } }