Skip to content

[CO-07] Fix off-by-one expiry in loyalty_token approve() causing immediate silent expiration#291

Merged
Leothosine merged 3 commits into
tosirano:mainfrom
bellabuks:fix/co-07-loyalty-token-expiry
Jun 25, 2026
Merged

[CO-07] Fix off-by-one expiry in loyalty_token approve() causing immediate silent expiration#291
Leothosine merged 3 commits into
tosirano:mainfrom
bellabuks:fix/co-07-loyalty-token-expiry

Conversation

@bellabuks

Copy link
Copy Markdown
Contributor

Summary

Fixes the misaligned boundary in loyalty_token approve() that allowed callers to set expiry windows as short as 1 ledger (~5 seconds), causing approvals to silently expire almost immediately.

Changes in contracts/loyalty_token/src/lib.rs

  • Added MIN_APPROVAL_VALIDITY_LEDGERS: u32 = 17_280 (~24 hours at 5-second ledger close)
  • Added MAX_APPROVAL_VALIDITY_LEDGERS: u32 = 6_307_200 (~1 year) to prevent indefinite approvals
  • Modified approve() to panic with "expiration too soon" when expiration_ledger < current + MIN_APPROVAL_VALIDITY_LEDGERS
  • Modified approve() to panic with "expiration too far in the future" when above the cap
  • Added doc comment clarifying that expiration_ledger is inclusive (valid through that ledger, expires at expiration_ledger + 1)
  • Added matching inline comment to get_allowance() to make the inclusive semantics explicit

Tests updated / added

  • Updated test_approve_and_transfer_from to use + 17_280 instead of the old + 1_000 (which was below the new minimum)
  • test_approve_expiry_too_soon_panics - expiry below minimum must panic with "expiration too soon"
  • test_approve_minimum_valid_expiry - expiry exactly at minimum is accepted
  • test_allowance_valid_at_expiry_ledger - allowance is still non-zero at the exact expiry ledger (inclusive boundary)
  • test_allowance_zero_after_expiry_ledger - allowance returns 0 one ledger past expiry

closes #285

@Leothosine Leothosine left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you

…er with expiry validation

- Keep MIN/MAX_APPROVAL_VALIDITY_LEDGERS constants and updated approve() from PR
- Preserve two-step admin transfer (PendingAdmin, accept_admin, cancel_transfer) from main
- Update test_approve_and_transfer_from to use 17_280 ledger window (MIN_APPROVAL_VALIDITY_LEDGERS)
- Retain all expiry boundary tests from PR and two-step admin tests from main
@Leothosine Leothosine merged commit 364e850 into tosirano:main Jun 25, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CO-07] Fix off-by-one expiry in loyalty_token approve() causing immediate silent expiration

2 participants