test(invoice-escrow): add negative off-chain sig expiry & multi-curre… - #302
Open
Joyyyb wants to merge 1 commit into
Open
Conversation
…ncy integration tests Closes StellarState#160 — Negative Tests for Expired Off-Chain Signature Submissions Closes StellarState#170 — Integration Tests for Multi-Currency Escrow Settlement ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ISSUE StellarState#160 — contracts/invoice-escrow/src/test.rs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Context ------- The fund_escrow_signed entry-point (PR StellarState#183) accepts an off-chain buyer approval consisting of (invoice_id, amount, nonce, expiry). The contract rejects the call when env.ledger().timestamp() > expiry, returning Error::SignatureExpired (code 22). Prior to this commit there were no tests that directly exercised the expiry rejection path, leaving a gap in negative-test coverage for this security-critical gate. What was done ------------- Seven new unit tests were appended to the existing test.rs test suite under the section heading: // ========== Issue StellarState#160: Negative Tests for Expired Off-Chain // Signature Submissions ========== 1. test_fund_escrow_signed_rejects_expired_signature Sets the ledger timestamp to expiry + 1 and asserts Error::SignatureExpired is returned. Confirms escrow remains Created. 2. test_fund_escrow_signed_rejects_at_exact_expiry_plus_one_boundary Verifies the boundary semantics: ledger_ts == expiry is NOT expired (the guard is current_ts > expiry, not >=), while ledger_ts == expiry+1 is expired. The passing half of this test exercises a successful signed fund at the exact boundary tick. 3. test_fund_escrow_signed_succeeds_just_before_expiry Sets ledger to expiry - 1 and asserts the call succeeds, the escrow transitions to Funded, and tokens are transferred to the contract. 4. test_fund_escrow_signed_expired_does_not_change_escrow_state First submits an expired signature (must fail), then submits a valid one. Asserts no state mutation or token movement occurred during the failed attempt, and that the escrow can still be funded afterwards. 5. test_fund_escrow_signed_nonce_not_consumed_on_expiry Proves that when a submission is rejected for expiry the nonce counter is NOT incremented. The same nonce is reused in the subsequent valid call and must succeed (would return NonceAlreadyUsed if the nonce had been consumed). 6. test_fund_escrow_signed_zero_expiry_always_expired Passes expiry = 0 with ledger_ts = 1 and verifies Error::SignatureExpired, covering the minimum-value edge-case for the expiry field. 7. test_fund_escrow_signed_expiry_checked_before_nonce Consumes nonce 1 via a valid signed call, then submits an already-used nonce with an expired timestamp. Asserts SignatureExpired is returned rather than NonceAlreadyUsed, confirming evaluation order: expiry is checked first, nonce second. How it was implemented ---------------------- All tests follow the existing pattern in test.rs: • env.mock_all_auths() so auth is bypassed for fund_escrow_signed. • Register InvoiceEscrow + MockInvoiceToken + Stellar asset contract. • Advance the ledger timestamp via env.ledger().with_mut(|li| ...). • Call try_fund_escrow_signed and assert the returned Err value. • For state-persistence checks, call get_escrow_status and verify TokenClient balances are unchanged after a failed attempt. ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ISSUE StellarState#170 — contracts/invoice-escrow/src/integration_test.rs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Context ------- The existing integration test suite (tests 1–25) uses a single payment token per test. There were no tests that ran two escrows with different payment tokens against the same InvoiceEscrow contract instance to verify that multi-currency scenarios are handled correctly. Token balance isolation, fee collection in the right currency, per-escrow state persistence, and correct refund routing all needed explicit coverage. What was done ------------- Five new integration tests were appended to integration_test.rs under the section heading: // Issue StellarState#170: Multi-Currency Escrow Settlement Integration Tests A private helper fn register_currency(...) was also added to reduce boilerplate: it registers a new InvoiceToken + Stellar asset contract, initializes the invoice token against the shared escrow, and returns the clients for both. 26. test_integration_multi_currency_two_escrows_settle_independently Creates two escrows (inv_a / inv_b) backed by different Stellar asset contracts on a single InvoiceEscrow instance. Funds and settles each in sequence. Asserts both reach Settled status and both invoice tokens are unlocked without interfering with each other. 27. test_integration_multi_currency_settlement_token_isolation Settles escrow A (token A) and verifies token B balances are entirely unaffected. Then settles escrow B and checks token A balances remain unchanged. Escrow contract balance is verified to be zero in both currencies after settlement. 28. test_integration_multi_currency_refund_returns_correct_token Settles escrow A normally, then advances time past the due date of escrow B and calls refund. Asserts buyer_b receives the refund in token B (not token A), and that token A is unaffected by the refund. 29. test_integration_multi_currency_fee_collected_in_correct_token Uses a 5% fee. Settles two escrows of different sizes in different tokens and asserts the admin receives exactly 5% in token A from the A-settlement and exactly 5% in token B from the B-settlement. Cross- token fee leakage is verified to be zero. 30. test_integration_multi_currency_state_persists_independently Calls get_escrow on both escrows after creation, after funding only A, and after settling A. Verifies that face_value, purchase_price, status, token address, commitment, due_dt, funded_amt, funder, and paid_amt are stored and read back correctly for each escrow without cross-escrow contamination. How it was implemented ---------------------- All tests use the existing setup() / create_and_fund() helpers where possible and the new register_currency() helper for second-token setup. Real InvoiceToken (invoice_token crate) and Stellar asset contracts are used throughout so cross-contract calls (mint, transfer, set_transfer_locked, decimals) execute as they would on-chain, matching the integration test philosophy of the file. env.mock_all_auths() is used so auth is not the variable under test. ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Acceptance criteria satisfied ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ All implementation tasks completed (7 + 5 new test functions). Error code assertions on every failure path (SignatureExpired, NonceAlreadyUsed, EscrowStatus checks). State storage persistence verified after each operation in both files. Code follows project style (matches existing patterns in test.rs and integration_test.rs). PR targets the dev branch.
|
@Joyyyb Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
Contributor
|
Hi @Joyyyb, your PR merged cleanly with dev, but the CI pipeline failed on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ncy integration tests
Closes #160 — Negative Tests for Expired Off-Chain Signature Submissions Closes #170 — Integration Tests for Multi-Currency Escrow Settlement
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ISSUE #160 — contracts/invoice-escrow/src/test.rs
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Context
The fund_escrow_signed entry-point (PR #183) accepts an off-chain buyer approval consisting of (invoice_id, amount, nonce, expiry). The contract rejects the call when env.ledger().timestamp() > expiry, returning Error::SignatureExpired (code 22). Prior to this commit there were no tests that directly exercised the expiry rejection path, leaving a gap in negative-test coverage for this security-critical gate.
What was done
Seven new unit tests were appended to the existing test.rs test suite under the section heading:
// ========== Issue #160: Negative Tests for Expired Off-Chain
// Signature Submissions ==========
test_fund_escrow_signed_rejects_expired_signature Sets the ledger timestamp to expiry + 1 and asserts Error::SignatureExpired is returned. Confirms escrow remains Created.
test_fund_escrow_signed_rejects_at_exact_expiry_plus_one_boundary Verifies the boundary semantics: ledger_ts == expiry is NOT expired (the guard is current_ts > expiry, not >=), while ledger_ts == expiry+1 is expired. The passing half of this test exercises a successful signed fund at the exact boundary tick.
test_fund_escrow_signed_succeeds_just_before_expiry Sets ledger to expiry - 1 and asserts the call succeeds, the escrow transitions to Funded, and tokens are transferred to the contract.
test_fund_escrow_signed_expired_does_not_change_escrow_state First submits an expired signature (must fail), then submits a valid one. Asserts no state mutation or token movement occurred during the failed attempt, and that the escrow can still be funded afterwards.
test_fund_escrow_signed_nonce_not_consumed_on_expiry Proves that when a submission is rejected for expiry the nonce counter is NOT incremented. The same nonce is reused in the subsequent valid call and must succeed (would return NonceAlreadyUsed if the nonce had been consumed).
test_fund_escrow_signed_zero_expiry_always_expired Passes expiry = 0 with ledger_ts = 1 and verifies Error::SignatureExpired, covering the minimum-value edge-case for the expiry field.
test_fund_escrow_signed_expiry_checked_before_nonce Consumes nonce 1 via a valid signed call, then submits an already-used nonce with an expired timestamp. Asserts SignatureExpired is returned rather than NonceAlreadyUsed, confirming evaluation order: expiry is checked first, nonce second.
How it was implemented
All tests follow the existing pattern in test.rs:
• env.mock_all_auths() so auth is bypassed for fund_escrow_signed.
• Register InvoiceEscrow + MockInvoiceToken + Stellar asset contract.
• Advance the ledger timestamp via env.ledger().with_mut(|li| ...).
• Call try_fund_escrow_signed and assert the returned Err value.
• For state-persistence checks, call get_escrow_status and verify
TokenClient balances are unchanged after a failed attempt.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ISSUE #170 — contracts/invoice-escrow/src/integration_test.rs ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Context
The existing integration test suite (tests 1–25) uses a single payment token per test. There were no tests that ran two escrows with different payment tokens against the same InvoiceEscrow contract instance to verify that multi-currency scenarios are handled correctly. Token balance isolation, fee collection in the right currency, per-escrow state persistence, and correct refund routing all needed explicit coverage.
What was done
Five new integration tests were appended to integration_test.rs under the section heading:
// Issue #170: Multi-Currency Escrow Settlement Integration Tests
A private helper fn register_currency(...) was also added to reduce boilerplate: it registers a new InvoiceToken + Stellar asset contract, initializes the invoice token against the shared escrow, and returns the clients for both.
test_integration_multi_currency_two_escrows_settle_independently Creates two escrows (inv_a / inv_b) backed by different Stellar asset contracts on a single InvoiceEscrow instance. Funds and settles each in sequence. Asserts both reach Settled status and both invoice tokens are unlocked without interfering with each other.
test_integration_multi_currency_settlement_token_isolation Settles escrow A (token A) and verifies token B balances are entirely unaffected. Then settles escrow B and checks token A balances remain unchanged. Escrow contract balance is verified to be zero in both currencies after settlement.
test_integration_multi_currency_refund_returns_correct_token Settles escrow A normally, then advances time past the due date of escrow B and calls refund. Asserts buyer_b receives the refund in token B (not token A), and that token A is unaffected by the refund.
test_integration_multi_currency_fee_collected_in_correct_token Uses a 5% fee. Settles two escrows of different sizes in different tokens and asserts the admin receives exactly 5% in token A from the A-settlement and exactly 5% in token B from the B-settlement. Cross- token fee leakage is verified to be zero.
test_integration_multi_currency_state_persists_independently Calls get_escrow on both escrows after creation, after funding only A, and after settling A. Verifies that face_value, purchase_price, status, token address, commitment, due_dt, funded_amt, funder, and paid_amt are stored and read back correctly for each escrow without cross-escrow contamination.
How it was implemented
All tests use the existing setup() / create_and_fund() helpers where possible and the new register_currency() helper for second-token setup. Real InvoiceToken (invoice_token crate) and Stellar asset contracts are used throughout so cross-contract calls (mint, transfer, set_transfer_locked, decimals) execute as they would on-chain, matching the integration test philosophy of the file. env.mock_all_auths() is used so auth is not the variable under test.
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Acceptance criteria satisfied
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
All implementation tasks completed (7 + 5 new test functions).
Error code assertions on every failure path (SignatureExpired,
NonceAlreadyUsed, EscrowStatus checks).
State storage persistence verified after each operation in both files.
Code follows project style (matches existing patterns in test.rs and
integration_test.rs).
PR targets the dev branch.
Description
Closes #
Type of Change
Checklist
Testing
How to Test
Test Coverage
Screenshots (if applicable)
Additional Notes
For Reviewers