code/cl8y-dex-terraclassic

Fork 0

Security: no test verifies commission treasury is unchanged after a non-hook swap failure [SEC-C03] #402

New issue

Closed

opened 2026-06-19 10:59:08 +00:00 by totdking · 6 comments

totdking commented

2026-06-19 10:59:08 +00:00

(Migrated from gitlab.com)

No description provided.

totdking commented

2026-06-19 10:59:53 +00:00

(Migrated from gitlab.com)

Checklist Item

SEC-C03: Verify commission/treasury transfer is atomic and pair does not retain fees unexpectedly. Verify: contract tests compare balances before/after successful and reverted swaps.

Summary

Treasury invariant tests exist for the wrap/unwrap treasury and hook-revert scenarios. However, no test snapshots the swap commission treasury before a swap that fails for non-hook reasons (max_spread exceeded, deadline expired, insufficient liquidity) and asserts the balance is unchanged after the revert.

What Was Checked

smartcontracts/tests/src/lib.rs line 11917: test_fees_accumulate_across_multiple_swaps -- accumulation on success
smartcontracts/tests/src/lib.rs line 15153: test_treasury_balance_audit -- wrap treasury invariant
smartcontracts/tests/src/lib.rs line 12923: test_state_consistent_after_hook_revert -- full state snapshot on hook failure
smartcontracts/tests/src/lib.rs line 8655: swap_fails_atomically_when_allowlisted_hook_reverts -- hook-caused revert tested
Searched for commission treasury balance before/after deadline-exceeded, max-spread-exceeded, insufficient-liquidity swap failures: no results

Expected (per checklist)

Tests must compare the commission (fee) treasury balance before and after both successful swaps and reverted swaps. A reverted swap must leave the treasury balance exactly as it was before the attempt.

Actual

The hook-revert path is well tested and asserts all state is unchanged. The wrap/unwrap treasury invariant is also tested. No test covers commission treasury state after a swap fails for reasons unrelated to hooks (spread limit, deadline, or liquidity).

Evidence

smartcontracts/tests/src/lib.rs line 12923: test_state_consistent_after_hook_revert snapshots state before a failed swap and verifies all state after -- hook failures only
No test found that records commission treasury balance before a spread-rejected or deadline-rejected swap and asserts no change

Suggested Fix

Add a test that:

Records the commission treasury token balance before a swap attempt
Submits a swap that will fail due to max_spread being exceeded
Asserts the commission treasury balance is unchanged after the failure
Repeats with a deadline-exceeded failure

This confirms the commission transfer is part of the atomic swap execution and is rolled back with it.

Verification Checklist

Test records commission treasury balance before a max_spread-rejected swap
Test asserts commission treasury balance is unchanged after the revert
Same pattern verified for deadline-exceeded failure
All new tests pass in CI

Labels

security, pre-launch

Cc: @PlasticDigits

### Checklist Item SEC-C03: Verify commission/treasury transfer is atomic and pair does not retain fees unexpectedly. Verify: contract tests compare balances before/after successful and reverted swaps. --- ### Summary Treasury invariant tests exist for the wrap/unwrap treasury and hook-revert scenarios. However, no test snapshots the swap commission treasury before a swap that fails for non-hook reasons (max_spread exceeded, deadline expired, insufficient liquidity) and asserts the balance is unchanged after the revert. --- ### What Was Checked - `smartcontracts/tests/src/lib.rs` line 11917: `test_fees_accumulate_across_multiple_swaps` -- accumulation on success - `smartcontracts/tests/src/lib.rs` line 15153: `test_treasury_balance_audit` -- wrap treasury invariant - `smartcontracts/tests/src/lib.rs` line 12923: `test_state_consistent_after_hook_revert` -- full state snapshot on hook failure - `smartcontracts/tests/src/lib.rs` line 8655: `swap_fails_atomically_when_allowlisted_hook_reverts` -- hook-caused revert tested - Searched for commission treasury balance before/after deadline-exceeded, max-spread-exceeded, insufficient-liquidity swap failures: no results --- ### Expected (per checklist) Tests must compare the commission (fee) treasury balance before and after both successful swaps and reverted swaps. A reverted swap must leave the treasury balance exactly as it was before the attempt. --- ### Actual The hook-revert path is well tested and asserts all state is unchanged. The wrap/unwrap treasury invariant is also tested. No test covers commission treasury state after a swap fails for reasons unrelated to hooks (spread limit, deadline, or liquidity). --- ### Evidence - `smartcontracts/tests/src/lib.rs` line 12923: `test_state_consistent_after_hook_revert` snapshots state before a failed swap and verifies all state after -- hook failures only - No test found that records commission treasury balance before a spread-rejected or deadline-rejected swap and asserts no change --- ### Suggested Fix Add a test that: 1. Records the commission treasury token balance before a swap attempt 2. Submits a swap that will fail due to max_spread being exceeded 3. Asserts the commission treasury balance is unchanged after the failure 4. Repeats with a deadline-exceeded failure This confirms the commission transfer is part of the atomic swap execution and is rolled back with it. --- ### Verification Checklist - [ ] Test records commission treasury balance before a max_spread-rejected swap - [ ] Test asserts commission treasury balance is unchanged after the revert - [ ] Same pattern verified for deadline-exceeded failure - [ ] All new tests pass in CI --- ### Labels `security`, `pre-launch` Cc: @PlasticDigits

totdking commented

2026-06-19 11:16:16 +00:00

(Migrated from gitlab.com)

mentioned in issue #381

PlasticDigits commented

2026-06-19 13:15:32 +00:00

(Migrated from gitlab.com)

mentioned in commit ccab0ae342

mentioned in commit ccab0ae342239f007fe2254c4b088e6d59a787e3

PlasticDigits commented

2026-06-19 13:15:41 +00:00

(Migrated from gitlab.com)

mentioned in merge request !931

PlasticDigits commented

2026-06-19 13:47:21 +00:00

(Migrated from gitlab.com)

mentioned in commit 6d893483d3

mentioned in commit 6d893483d305fe74d283e76015ec5be069da0786

PlasticDigits commented

2026-06-25 11:35:33 +00:00

(Migrated from gitlab.com)

Verification complete — SEC-C03 (#402)

Verified on main (no repo changes required).

Acceptance checklist

Criterion	Result	How verified
Test records commission treasury balance before a max_spread-rejected swap	PASS	`audit_invariant_tests::commission_treasury_unchanged_after_max_spread_rejected_swap` snapshots `treasury` token_a/token_b balances after a successful swap, then before the failing swap (`smartcontracts/tests/src/lib.rs` ~L9029–9030)
Test asserts commission treasury balance is unchanged after the revert	PASS	Same test asserts `treasury_b_before` unchanged after `Max spread assertion` error (~L9073–9076); also checks token_a
Same pattern verified for deadline-exceeded failure	PASS	`audit_invariant_tests::commission_treasury_unchanged_after_deadline_rejected_swap` (~L9083–9146): advances block past deadline, snapshots treasury, asserts unchanged after deadline rejection
All new tests pass in CI	PASS	`cargo test audit_invariant_tests::commission_treasury_unchanged` → 2 passed; `make test-contracts` → exit 0

Hook-revert full state snapshot: new_feature_tests::test_state_consistent_after_hook_revert
Successful fee accumulation: test_fees_accumulate_across_multiple_swaps
Docs: docs/security-model.md § Treasury Management; invariant P10 in docs/contracts-security-audit.md

Commands run

glab issue view 402 --comments
cargo test audit_invariant_tests::commission_treasury_unchanged -- --nocapture
make test-contracts

Closing — SEC-C03 commission treasury atomicity on non-hook swap failures is covered by contract regression tests and documentation.

## Verification complete — SEC-C03 ([#402](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/issues/402)) Verified on `main` (no repo changes required). ### Acceptance checklist | Criterion | Result | How verified | |-----------|--------|--------------| | Test records commission treasury balance before a max_spread-rejected swap | **PASS** | `audit_invariant_tests::commission_treasury_unchanged_after_max_spread_rejected_swap` snapshots `treasury` token_a/token_b balances after a successful swap, then before the failing swap (`smartcontracts/tests/src/lib.rs` ~L9029–9030) | | Test asserts commission treasury balance is unchanged after the revert | **PASS** | Same test asserts `treasury_b_before` unchanged after `Max spread assertion` error (~L9073–9076); also checks token_a | | Same pattern verified for deadline-exceeded failure | **PASS** | `audit_invariant_tests::commission_treasury_unchanged_after_deadline_rejected_swap` (~L9083–9146): advances block past deadline, snapshots treasury, asserts unchanged after deadline rejection | | All new tests pass in CI | **PASS** | `cargo test audit_invariant_tests::commission_treasury_unchanged` → 2 passed; `make test-contracts` → exit 0 | ### Related coverage (unchanged, already present) - Hook-revert full state snapshot: `new_feature_tests::test_state_consistent_after_hook_revert` - Successful fee accumulation: `test_fees_accumulate_across_multiple_swaps` - Docs: `docs/security-model.md` § Treasury Management; invariant **P10** in `docs/contracts-security-audit.md` ### Commands run ```bash glab issue view 402 --comments cargo test audit_invariant_tests::commission_treasury_unchanged -- --nocapture make test-contracts ``` Closing — SEC-C03 commission treasury atomicity on non-hook swap failures is covered by contract regression tests and documentation.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-06-25 11:35:34 +00:00