Review: factory pair-creation BankMsg::Send to treasury (#313) #789

2026-06-05T10:07:21Z

PlasticDigits commented

2026-06-05 10:07:21 +00:00

(Migrated from gitlab.com)

"## Summary\n\nCloses the security review in GitLab #313 for factory pair-creation uluna fees sent to treasury via BankMsg::Send (introduced in #276).\n\nNo factory contract logic changes \u2014 review found the implementation sound. This MR adds:\n\n- Written audit checklist: docs/audits/factory-treasury-bank-send.md\n- Adversarial integration test create_pair_fee_bank_send_adversarial_paths (stray denom rejection, overpay refund, treasury rotation via UpdateConfig)\n- make verify-issue-313 QA script\n- Cross-link on invariant F2 in docs/contracts-security-audit.md\n\n## Acceptance checklist\n\n| Criterion | Verification | Status |\n|-----------|--------------|--------|\n| Written review checklist with sign-off | docs/audits/factory-treasury-bank-send.md | PASS |\n| Issues found filed or fixed | None required (no contract bugs) | PASS |\n| Adversarial tests for gaps | create_pair_fee_bank_send_adversarial_paths | PASS |\n| No regression on happy path | make test-contracts; existing #276 tests | PASS |\n\n## Verification (third parties)\n\nbash\nmake verify-issue-313\n\n\n## Test plan\n\n| Path | Expected | Result |\n|------|----------|--------|\n| Exact fee | Treasury + pair | PASS |\n| Underpay | InsufficientPairCreationFee | PASS |\n| Overpay | Fee to treasury; refund to user | PASS |\n| Stray denom | Reject; no pair | PASS |\n| Zero fee | Free create; refund mistaken uluna | PASS |\n| Gov updates treasury | New addr receives fee | PASS |\n\nRelated: #276\n"

Note

Low Risk
Documentation, integration tests, and QA automation only; no changes to factory fee or bank-send logic.

Overview
Closes the GitLab #313 security review of factory uluna pair-creation fees sent to treasury via BankMsg::Send (#276). No factory contract logic changes — the review signed off the existing implementation.

Adds docs/audits/factory-treasury-bank-send.md (checklist, attack vectors, test mapping, sign-off) and extends invariant F2 in docs/contracts-security-audit.md with links to that audit and the new tests.

Adds integration test create_pair_fee_bank_send_adversarial_paths covering stray-denom rejection, overpay refund, and fee crediting the treasury after governance UpdateConfig rotation.

Adds make verify-issue-313 / scripts/qa/verify-issue-313.sh to run the adversarial and #276 fee regressions, assert the audit doc exists, and run make test-contracts.

^{Reviewed by Cursor Bugbot for commit c2b975b79f. Bugbot is set up for automated code reviews on this repo. Configure here.}

"## Summary\n\nCloses the security review in [GitLab #313](https://gitlab.com/PlasticDigits/cl8y-dex-terraclassic/-/work_items/313) for factory pair-creation **uluna** fees sent to treasury via `BankMsg::Send` (introduced in #276).\n\n**No factory contract logic changes** \u2014 review found the implementation sound. This MR adds:\n\n- Written audit checklist: `docs/audits/factory-treasury-bank-send.md`\n- Adversarial integration test `create_pair_fee_bank_send_adversarial_paths` (stray denom rejection, overpay refund, treasury rotation via `UpdateConfig`)\n- `make verify-issue-313` QA script\n- Cross-link on invariant **F2** in `docs/contracts-security-audit.md`\n\n## Acceptance checklist\n\n| Criterion | Verification | Status |\n|-----------|--------------|--------|\n| Written review checklist with sign-off | `docs/audits/factory-treasury-bank-send.md` | PASS |\n| Issues found filed or fixed | None required (no contract bugs) | PASS |\n| Adversarial tests for gaps | `create_pair_fee_bank_send_adversarial_paths` | PASS |\n| No regression on happy path | `make test-contracts`; existing #276 tests | PASS |\n\n## Verification (third parties)\n\n```bash\nmake verify-issue-313\n```\n\n## Test plan\n\n| Path | Expected | Result |\n|------|----------|--------|\n| Exact fee | Treasury + pair | PASS |\n| Underpay | `InsufficientPairCreationFee` | PASS |\n| Overpay | Fee to treasury; refund to user | PASS |\n| Stray denom | Reject; no pair | PASS |\n| Zero fee | Free create; refund mistaken uluna | PASS |\n| Gov updates treasury | New addr receives fee | PASS |\n\nRelated: #276\n"  --- > [!NOTE] > **Low Risk** > Documentation, integration tests, and QA automation only; no changes to factory fee or bank-send logic. > > **Overview** > Closes the **GitLab #313** security review of factory **uluna** pair-creation fees sent to treasury via `BankMsg::Send` (#276). **No factory contract logic changes** — the review signed off the existing implementation. > > Adds **`docs/audits/factory-treasury-bank-send.md`** (checklist, attack vectors, test mapping, sign-off) and extends invariant **F2** in **`docs/contracts-security-audit.md`** with links to that audit and the new tests. > > Adds integration test **`create_pair_fee_bank_send_adversarial_paths`** covering stray-denom rejection, overpay refund, and fee crediting the treasury after governance **`UpdateConfig`** rotation. > > Adds **`make verify-issue-313`** / **`scripts/qa/verify-issue-313.sh`** to run the adversarial and #276 fee regressions, assert the audit doc exists, and run **`make test-contracts`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c2b975b79f73c6fcab1e4b967a278971ef025640. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>

PlasticDigits commented

2026-06-05 10:07:27 +00:00

(Migrated from gitlab.com)

mentioned in issue #313

ghost1 commented

2026-06-05 10:07:31 +00:00

(Migrated from gitlab.com)

changed the description

ghost1 commented

2026-06-05 10:07:36 +00:00

(Migrated from gitlab.com)

changed the description

PlasticDigits commented

2026-06-05 10:08:33 +00:00

(Migrated from gitlab.com)

Security review — MR !789

Commit reviewed: c2b975b79f73c6fcab1e4b967a278971ef025640
Scope: Added/modified diff only (docs/audits/factory-treasury-bank-send.md, docs/contracts-security-audit.md F2 cross-link, smartcontracts/tests/src/lib.rs adversarial test create_pair_fee_bank_send_adversarial_paths, scripts/qa/verify-issue-313.sh, Makefile verify-issue-313 target). No factory contract logic changes in this MR.

Method: Traced attacker-controlled inputs through changed paths; confirmed existing on-chain controls on the referenced execute_create_pair fee/BankMsg::Send flow (unchanged) — stray-denom rejection, underpay guard, governance-only treasury/fee mutation, refund/overpay handling, OnePairCreationPerBlock gate before Response.

Outcome: FINDINGS: 0 medium+

Security review: no medium+ findings on this diff.

Changed artifacts are documentation, integration tests, and a fixed-path QA script (set -euo pipefail, no external/user-controlled input, no command injection surface). No inline threads opened.

Automated security review (Cursor Cloud Agent)

## Security review — MR !789 **Commit reviewed:** `c2b975b79f73c6fcab1e4b967a278971ef025640` **Scope:** Added/modified diff only (`docs/audits/factory-treasury-bank-send.md`, `docs/contracts-security-audit.md` F2 cross-link, `smartcontracts/tests/src/lib.rs` adversarial test `create_pair_fee_bank_send_adversarial_paths`, `scripts/qa/verify-issue-313.sh`, `Makefile` `verify-issue-313` target). No factory contract logic changes in this MR. **Method:** Traced attacker-controlled inputs through changed paths; confirmed existing on-chain controls on the referenced `execute_create_pair` fee/`BankMsg::Send` flow (unchanged) — stray-denom rejection, underpay guard, governance-only treasury/fee mutation, refund/overpay handling, `OnePairCreationPerBlock` gate before `Response`. **Outcome:** `FINDINGS: 0` medium+ Security review: **no medium+ findings on this diff.** Changed artifacts are documentation, integration tests, and a fixed-path QA script (`set -euo pipefail`, no external/user-controlled input, no command injection surface). No inline threads opened. --- *Automated security review (Cursor Cloud Agent)*

PlasticDigits commented

2026-06-05 10:37:34 +00:00

(Migrated from gitlab.com)

mentioned in commit 0d3c4eeacf

mentioned in commit 0d3c4eeacfd39ef666e34ba458bb56a6247c8221