Fix/treasury allowlist recovery docs legacy coverage - #531
Open
Valreb001 wants to merge 4 commits into
Open
Conversation
…tlement Move the token allowlist check to the earliest point in propose_settlement so a disallowed token is rejected before check_not_paused and signer.require_auth() run, instead of after paying for that work (WHEELBACK#31). Add a regression test proving TokenNotAllowed now takes precedence over ContractPaused, and a benchmark asserting the rejected-token path costs less CPU than a full accepted validation pass (relates to WHEELBACK#182). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
MAINNET_DEPLOYMENT.md covered the signing ceremony (WHEELBACK#276) but had no documented procedure for what remaining signers should do if a signer's key is lost or compromised and the treasury threshold can no longer be met. Add a Signer Loss or Compromise Recovery section under Multi-Sig Governance Model covering the set_signer / update_threshold recovery path (both admin-gated on-chain, not signer-quorum-gated), the correct call ordering to avoid a self-inflicted DoS, and the minimum number of remaining signers needed to clear the ceremony quorum under the default 3-of-5 threshold model. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
contracts/api-integration-tests only covered the legacy invoice flow; the legacy contracts/settlement contract had no integration coverage in this tree, unlike its canonical counterpart in COMEBACKHERE-contracts. Add contracts/settlement/src/integration_settlement_multisig.rs, mirroring the canonical treasury's integration_settlement_multisig.rs scenarios (2-of-3 quorum, insufficient single signer, weighted signers, independent settlements). This legacy contract has no execute_settlement entry point, so "quorum reached" via approve_settlement's ApproveResult is treated as the execute signal, as documented in the new file. Also fixes a pre-existing missing closing brace in contracts/settlement/src/lib.rs's test module (the proptest quorum test was accidentally nested inside test_cancel_not_pending_fails), which otherwise prevents this crate from compiling at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
New contributors browsing the repo root can land in the legacy contracts/ tree before noticing COMEBACKHERE-contracts/ is canonical for CI. Add a short contracts/README.md that states this plainly and links to CONTRIBUTING.md and ARCHITECTURE.md for the full rationale, so the pointer is visible from inside the tree itself and not just in top-level docs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@Valreb001 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! 🚀 |
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
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.
Closes #406
Closes #407
Closes #408
Closes #409
SUMMARY
Treasury allowlist check order (COMEBACKHERE-contracts/contracts/treasury/src/lib.rs, benchmark.rs)
Moved the token allowlist check in propose_settlement to run before check_not_paused and signer.require_auth(), so a disallowed token is now
rejected before the contract pays for pause/auth work. Added a correctness test proving TokenNotAllowed now wins over ContractPaused when both
conditions hold, and a benchmark asserting the rejected-token path costs less CPU than a full accepted proposal.
Signer recovery documentation (docs/MAINNET_DEPLOYMENT.md)
Added a "Signer Loss or Compromise Recovery" section documenting the set_signer/update_threshold procedure. Notably, I found both are gated only
by the admin key on-chain (check_admin), not a live signer-quorum vote — so I documented the correct call ordering (lower threshold before
revoking a signer, to avoid a self-inflicted deadlock) and spelled out that under the default 3-of-5 model, a single signer loss is always
recoverable via ceremony quorum, but losing 2+ signers at once drops below ceremony quorum and leaves recovery gated solely by the admin key —
flagged as a residual single point of failure.
Legacy settlement integration tests (contracts/settlement/src/integration_settlement_multisig.rs)
Added propose/approve multisig coverage mirroring the canonical treasury's test suite (2-of-3 quorum, insufficient single signer, weighted
signers, independent settlements). Note: this legacy contract has no execute_settlement call — "quorum reached" via ApproveResult is treated as
the execute signal, documented inline. Also had to fix a pre-existing bug: a missing closing brace in lib.rs's test module had the entire
proptest quorum test nested inside another test function, which meant the crate didn't compile at all.
Legacy tree README (contracts/README.md)
New file pointing contributors landing in contracts/ to COMEBACKHERE-contracts/ as canonical for CI, linking to CONTRIBUTING.md and
ARCHITECTURE.md.