Fix/invoice treasury hardening - #530
Open
rdj-savyy wants to merge 4 commits into
Open
Conversation
mark_paids collapsed every non-Pending status into the generic InvoiceAlreadyPaid error, so calling it on an invoice that was already RefundRequested, Released, Cancelled, or Expired was rejected but with a misleading error and no explicit acknowledgement that a payer's refund request could otherwise be clobbered by a stale payment confirmation. Add an explicit guard at the top of the loop that returns the new ContractError::InvalidStateTransition for those four states, keeping InvoiceAlreadyPaid for the genuinely-already-paid case. Also add the Overflow and AddressBlocked variants that the file already referenced but never declared on ContractError, which left the crate unable to compile. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Individual InvoiceStatus values were documented in isolation, but no single diagram showed which transitions between them are legal, making it hard to tell which InvalidStateTransition/NotPending cases in docs/error-codes.md are expected versus a real bug. Add a Mermaid state diagram to ARCHITECTURE.md covering every state and the function that triggers each transition, with notes on the deliberately-absent edges (e.g. Paid is never re-entered). Cross-link it from docs/error-codes.md in both directions, and fill in the previously-undocumented ContractError (invoice) codes 15-18 (NotAParty, Overflow, AddressBlocked, InvalidStateTransition) that had drifted out of sync with the enum in lib.rs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
rotate_signer did not exist, and the total_signer_weight helper it needs (used by update_threshold's cap check) was only ever called, not defined — a botched merge of two prior feature branches had dropped its body along with the SignerList DataKey variant and the ThresholdExceedsWeight/InvalidPagination error variants those functions rely on, and left a second, duplicate `mod tests` block plus two dangling helper functions referencing an on-chain Dispute type that was never actually merged in. None of this could compile. Restore total_signer_weight/get_total_signer_weight (recomputed live from SignerList on every call, so it can never drift from the individual Signer(address) entries), remove the dead duplicate test module and orphaned dispute-voting helpers, and add rotate_signer: deregisters the old signer and registers the new one atomically so SignerList - and therefore total_signer_weight - stays exactly in sync whether the new weight is higher or lower than the old one. Add tests for both directions: rotating to a higher weight and asserting total_signer_weight updates correctly, and rotating to a lower weight after a signer has already approved a settlement, confirming that in-flight approval_weight is a snapshot and the settlement remains executable even though the signer's live weight (and total_signer_weight) has since dropped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
withdraw had no rate limiting beyond the admin/pause checks, so a compromised signer set that reached quorum could drain the treasury of a given token in a single settlement cycle with no configurable ceiling on worst-case exposure. Add an admin-only set_daily_withdraw_limit(token, limit), enforced by withdraw (now token-scoped) via a per-token WithdrawWindow record tracking cumulative withdrawals against a rolling 24h ledger-time window: the window resets once ledger.timestamp() has advanced a full 86400s past its start, and any withdrawal that would push the current window's cumulative total above the configured limit is rejected with the new TreasuryError::DailyLimitExceeded. A token with no configured limit remains unrestricted, preserving existing behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@rdj-savyy 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
1 participant
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 #400
Closes #402
Closes #404
Closes #405
SUMMARY
mark_paidsnow rejects calls on invoices already inRefundRequested,Released,Cancelled, orExpiredstatus with a newContractError::InvalidStateTransition, instead of silently overriding arefund already in progress.
ARCHITECTURE.md, cross-linked fromdocs/error-codes.md, so it's clearwhich state transitions are legal.
rotate_signer(it didn't previously exist) andrestored the
total_signer_weighthelper, which had been left uncompilableby a bad merge. Added tests covering both a weight increase and a weight
decrease, including a case where a settlement's already-accumulated quorum
is unaffected by a later rotation.
(
set_daily_withdraw_limit) enforced bywithdrawover a rolling 24hledger-time window, returning
TreasuryError::DailyLimitExceededwhenexceeded.