Skip to content

Fix/invoice treasury hardening #530

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

Merged
levoski1 merged 5 commits into from
Aug 30, 2026
Merged

Fix/invoice treasury hardening #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f7b3ae4
fix(invoice): guard mark_paids against terminal/refund-state overrides
rdj-savyy Aug 27, 2026
1a307c7
docs(invoice): add state-transition diagram and fill error-code gaps
rdj-savyy Aug 27, 2026
a5eb738
test(treasury): cover rotate_signer / total_signer_weight interaction
rdj-savyy Aug 27, 2026
fa6dcb6
feat(treasury): add per-token daily withdrawal limit
rdj-savyy Aug 27, 2026
f0a482e
Merge branch 'main' into fix/invoice-treasury-hardening
levoski1 Aug 30, 2026
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
28 changes: 28 additions & 0 deletions ARCHITECTURE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,34 @@ A working clone for end-to-end development looks like this (see `docs/dev-enviro

When working inside this repository alone, the in-tree `COMEBACKHERE-contracts/` checkout acts as the canonical contracts tree; the sibling-clone step is optional for backend/frontend contributors.

## Invoice state machine

The invoice contract (`COMEBACKHERE-contracts/contracts/invoice/src/lib.rs`, mirrored at `contracts/invoice/src/lib.rs`) is the source of truth every other layer reads from: backend status displays, the indexer, and both frontend apps all ultimately derive their view of an invoice from this state machine. The diagram below shows every `InvoiceStatus` value and the function whose call transitions an invoice into it. Any transition not shown here is illegal and the calling function returns an `InvalidStateTransition` / `NotPending` style error (see [docs/error-codes.md](docs/error-codes.md) for the exact variant and code per contract).

```mermaid
stateDiagram-v2
[*] --> Pending: create_invoice

Pending --> Paid: mark_paids / pay_invoice
Pending --> Expired: batch_expire
Pending --> Cancelled: cancel_invoiced / cancel_invoice

Paid --> RefundRequested: request_refund
Paid --> RefundRequested: cancel_invoiced / cancel_invoice

RefundRequested --> Released: release_escrow (after grace window)

Expired --> [*]
Cancelled --> [*]
Released --> [*]
```

Notes on edges that are deliberately absent from this diagram:

- **`Paid` is never re-entered.** Once an invoice leaves `Pending`, no function transitions it back to `Paid`. In particular, `mark_paids` guards against being called on an invoice that is `RefundRequested`, `Released`, `Cancelled`, or `Expired`, so a stale or replayed payment confirmation can never silently override a refund already in progress.
- **`RefundRequested`, `Released`, `Cancelled`, and `Expired` are terminal with respect to payment and cancellation** — `cancel_invoiced`, `request_refund`, and `mark_paids` all reject calls made once an invoice has reached one of these states.
- **`release_escrow` is time-gated**, not just state-gated: it additionally requires `ledger.timestamp() >= invoice.created_at + grace_window`.

## Further reading

- [docs/dev-environment.md](docs/dev-environment.md) — full local setup.
Expand Down
228 changes: 59 additions & 169 deletions COMEBACKHERE-contracts/contracts/invoice/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,13 @@ pub enum ContractError {
DuplicateNonce = 13,
TreasuryNotConfigured = 14,
NotAParty = 15,
ReferenceTooLong = 16,
Overflow = 17,
AddressBlocked = 18,
AmountPrecision = 19,
Overflow = 16,
AddressBlocked = 17,
/// A state-changing call was rejected because the invoice is in a terminal
/// or refund-related state that does not permit the requested transition
/// (e.g. `mark_paids` called on an invoice that is `RefundRequested`,
/// `Released`, `Cancelled`, or `Expired`).
InvalidStateTransition = 18,
}

#[contracttype]
Expand Down Expand Up @@ -332,7 +335,10 @@ impl InvoiceContract {
/// # Errors
/// - [`ContractError::ContractPaused`] if the contract is currently paused.
/// - [`ContractError::InvoiceNotFound`] if any ID in the batch does not exist.
/// - [`ContractError::InvoiceAlreadyPaid`] if any invoice is not in `Pending` status.
/// - [`ContractError::InvalidStateTransition`] if any invoice is `RefundRequested`,
/// `Released`, `Cancelled`, or `Expired` — a payment confirmation must never
/// silently override a refund already in progress or a closed invoice.
/// - [`ContractError::InvoiceAlreadyPaid`] if any invoice is already `Paid`.
/// - [`ContractError::InvoiceExpired`] if any invoice's `expires_at` has passed.
///
/// # Events
Expand All @@ -353,6 +359,20 @@ impl InvoiceContract {
.persistent()
.get::<DataKey, Invoice>(&DataKey::Invoice(id))
.ok_or(ContractError::InvoiceNotFound)?;
// Terminal and refund-related states must never be silently
// overridden by a stale payment confirmation: a payer's refund
// request (or an already-settled/cancelled/expired invoice) is
// rejected with a distinct error rather than falling through to
// the generic "already paid" case below.
if matches!(
invoice.status,
InvoiceStatus::RefundRequested
| InvoiceStatus::Released
| InvoiceStatus::Cancelled
| InvoiceStatus::Expired
) {
return Err(ContractError::InvalidStateTransition);
}
if invoice.status != InvoiceStatus::Pending {
return Err(ContractError::InvoiceAlreadyPaid);
}
Expand Down Expand Up @@ -1186,197 +1206,67 @@ mod tests {
assert_eq!(res, Err(Ok(ContractError::ContractPaused)));
}

// ── get_invoices_by_merchant ─────────────────────────────────────────────

/// Only invoice IDs belonging to the queried merchant are returned.
#[test]
fn test_get_invoices_by_merchant_filters_by_merchant() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant_a = Address::generate(&env);
let merchant_b = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);

client.create_invoice(&merchant_a, &customer, &10_000_000i128, &token, &5000, &1, &None);
client.create_invoice(&merchant_b, &customer, &10_000_000i128, &token, &5000, &1, &None);
client.create_invoice(&merchant_a, &customer, &10_000_000i128, &token, &5000, &2, &None);

let ids = client.get_invoices_by_merchant(&merchant_a, &None, &10u32);
assert_eq!(ids, soroban_sdk::vec![&env, 1u64, 3u64]);
}

/// A merchant with no invoices gets an empty page back.
#[test]
fn test_get_invoices_by_merchant_no_invoices_returns_empty() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);

let ids = client.get_invoices_by_merchant(&merchant, &None, &10u32);
assert_eq!(ids, Vec::new(&env));
}

/// `start_after` skips the given number of already-seen matches for pagination.
#[test]
fn test_get_invoices_by_merchant_pagination_start_after() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);

for nonce in 1..=5u64 {
client.create_invoice(&merchant, &customer, &10_000_000i128, &token, &5000, &nonce, &None);
}

let page1 = client.get_invoices_by_merchant(&merchant, &None, &2u32);
assert_eq!(page1, soroban_sdk::vec![&env, 1u64, 2u64]);

let page2 = client.get_invoices_by_merchant(&merchant, &Some(2u32), &2u32);
assert_eq!(page2, soroban_sdk::vec![&env, 3u64, 4u64]);
// ── mark_paids terminal/refund-state guard tests ─────────────────────────

let page3 = client.get_invoices_by_merchant(&merchant, &Some(4u32), &2u32);
assert_eq!(page3, soroban_sdk::vec![&env, 5u64]);
}

/// `limit` is capped at 100 even when a caller requests more.
/// A stale mark_paids call must not silently override a refund already
/// requested by the customer — it should be rejected, not re-marked Paid.
#[test]
fn test_get_invoices_by_merchant_limit_capped_at_100() {
fn test_mark_paids_on_refund_requested_returns_invalid_state_transition() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);

for nonce in 1..=105u64 {
client.create_invoice(&merchant, &customer, &10_000_000i128, &token, &5000, &nonce, &None);
}
let (_merchant, customer, id) = create_test_invoice(&client, &env);

let ids = client.get_invoices_by_merchant(&merchant, &None, &1000u32);
assert_eq!(ids.len(), 100);
}
client.mark_paids(&soroban_sdk::vec![&env, id]);
client.request_refund(&id, &customer);

// ── optional `reference` field ───────────────────────────────────────────

/// A reference within the length limit is stored and returned as-is.
#[test]
fn test_create_invoice_with_reference_is_stored() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);
let reference = String::from_str(&env, "order-12345");

let id = client.create_invoice(
&merchant,
&customer,
&10_000_000i128,
&token,
&5000,
&1,
&Some(reference.clone()),
);
let res = client.try_mark_paids(&soroban_sdk::vec![&env, id]);
assert_eq!(res, Err(Ok(ContractError::InvalidStateTransition)));

// The refund request must survive the stale confirmation untouched.
let invoice = client.get_invoice(&id);
assert_eq!(invoice.reference, Some(reference));
assert_eq!(invoice.status, InvoiceStatus::RefundRequested);
}

/// Omitting the reference leaves it `None`.
/// mark_paids on a Released (escrow already released) invoice is rejected.
#[test]
fn test_create_invoice_without_reference_defaults_to_none() {
let (env, cid, _admin) = setup_contract(1000);
fn test_mark_paids_on_released_returns_invalid_state_transition() {
let (env, cid, admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);
let (merchant, customer, id) = create_test_invoice(&client, &env);

let id = client.create_invoice(&merchant, &customer, &10_000_000i128, &token, &5000, &1, &None);
client.mark_paids(&soroban_sdk::vec![&env, id]);
client.request_refund(&id, &customer);
client.set_grace_window(&admin, &0u64);
client.release_escrow(&id, &merchant);

let invoice = client.get_invoice(&id);
assert_eq!(invoice.reference, None);
let res = client.try_mark_paids(&soroban_sdk::vec![&env, id]);
assert_eq!(res, Err(Ok(ContractError::InvalidStateTransition)));
}

/// A reference longer than MAX_REFERENCE_LEN (64 bytes) is rejected.
/// mark_paids on a Cancelled invoice is rejected with the same distinct error.
#[test]
fn test_create_invoice_reference_too_long_returns_error() {
fn test_mark_paids_on_cancelled_returns_invalid_state_transition() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);
let too_long = String::from_str(
&env,
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
);

let result = client.try_create_invoice(
&merchant,
&customer,
&10_000_000i128,
&token,
&5000,
&1,
&Some(too_long),
);
assert_eq!(result, Err(Ok(ContractError::ReferenceTooLong)));
}
let (merchant, _customer, id) = create_test_invoice(&client, &env);

/// A reference exactly at MAX_REFERENCE_LEN (64 bytes) is accepted.
#[test]
fn test_create_invoice_reference_at_max_length_succeeds() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);
let exact = String::from_str(
&env,
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
);
client.cancel_invoiced(&id, &merchant);

let id = client.create_invoice(
&merchant,
&customer,
&10_000_000i128,
&token,
&5000,
&1,
&Some(exact.clone()),
);
let invoice = client.get_invoice(&id);
assert_eq!(invoice.reference, Some(exact));
let res = client.try_mark_paids(&soroban_sdk::vec![&env, id]);
assert_eq!(res, Err(Ok(ContractError::InvalidStateTransition)));
}

// ── minimum amount boundary (MIN_AMOUNT_USDC) ────────────────────────────

/// An invoice amount exactly at the minimum (10,000,000 stroops / 1 USDC) is accepted.
/// mark_paids on an already-Paid invoice still returns the more specific
/// InvoiceAlreadyPaid error, distinct from the terminal/refund-state guard.
#[test]
fn test_create_invoice_at_min_amount_succeeds() {
fn test_mark_paids_on_already_paid_returns_invoice_already_paid() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);

let id = client.create_invoice(&merchant, &customer, &10_000_000i128, &token, &5000, &1, &None);

let invoice = client.get_invoice(&id);
assert_eq!(invoice.amount, 10_000_000i128);
}
let (_merchant, _customer, id) = create_test_invoice(&client, &env);

/// An invoice amount one stroop below the minimum is rejected with `AmountPrecision`.
#[test]
fn test_create_invoice_below_min_amount_returns_amount_precision() {
let (env, cid, _admin) = setup_contract(1000);
let client = InvoiceContractClient::new(&env, &cid);
let merchant = Address::generate(&env);
let customer = Address::generate(&env);
let token = Address::generate(&env);
client.mark_paids(&soroban_sdk::vec![&env, id]);

let result =
client.try_create_invoice(&merchant, &customer, &9_999_999i128, &token, &5000, &1, &None);
assert_eq!(result, Err(Ok(ContractError::AmountPrecision)));
let res = client.try_mark_paids(&soroban_sdk::vec![&env, id]);
assert_eq!(res, Err(Ok(ContractError::InvoiceAlreadyPaid)));
}
}
Loading