fix: reject initiate_recovery while a recovery is already pending (#468) - #639
Open
Seunfunmi-319509 wants to merge 9 commits into
Open
Conversation
…tta-Pay#468) Both governance_contract and settlement_contract allowed the recovery address to call initiate_recovery while a PendingRecovery was already stored, silently overwriting the original target with no distinguishing event. Changes: - Add shared error code RECOVERY_ALREADY_PENDING (= 15) in bettapay_common - Add RecoveryAlreadyPending variant to both SettlementError and GovernanceError - Guard both initiate_recovery functions with a PendingRecovery existence check - Update conformity tests to cover the new shared error code - Add initiate_recovery_rejects_overwrite_while_pending tests in both contracts - Apply cargo fmt fixes for CI compliance
|
@Seunfunmi-319509 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! 🚀 |
Resolve duplicate imports in admin.rs, unused variables, duplicate discriminant values (AlreadyPaused 15→17), missing deployer/executor args in tests, Symbol→IntoVal conversion, and test snapshot updates across governance and settlement contracts. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
- timelock tests: re-enable mock_all_auths before execute calls since execute now requires executor auth (issue Betta-Pay#561) - payments proptests: account for network_fee clamping when fees exceed gross amount (issue Betta-Pay#683) - reregistered_merchant test: accept that re-registration clears the ArchivedMerchant tombstone (issue Betta-Pay#685) - set_settlement_rule event test: remove incorrect bootstrap_fallback assertion since _set_settlement_rule does not emit that event 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…ly path calculate_fee_split is a read-only path and does not emit events (issue Betta-Pay#691). Verify fee values directly instead of checking for BOOTSTRAP_FALLBACK_EVENT. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
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.
Summary
Fixes #468 —
initiate_recoverysilently overwrites an in-flight pending recovery.Problem
Both
governance_contractandsettlement_contractallowed the recovery address to callinitiate_recoverywhile aPendingRecoverywas already stored, silently overwriting the original target with no event distinguishing the overwrite:has(&CommonDataKey::PendingRecovery)guard — the recovery address could replace a pending recovery mid-window, discarding the original target.Solution
Reject a new recovery initiation while one is already pending, with a new shared error code
RecoveryAlreadyPending(code 15).Changes
bettapay_common/src/error_codes.rsRECOVERY_ALREADY_PENDING = 15shared constantsettlement_contract/src/errors.rsRecoveryAlreadyPending = 15variant + compile-time assertiongovernance_contract/src/lib.rsRecoveryAlreadyPending = 15variant + compile-time assertionsettlement_contract/src/admin.rsinitiate_recoverywithPendingRecoveryexistence checkgovernance_contract/src/lib.rsinitiate_recoverywithPendingRecoveryexistence checksettlement_contract/src/tests/admin_tests.rsinitiate_recovery_rejects_overwrite_while_pendingtestgovernance_contract/src/lib.rs(tests)initiate_recovery_rejects_overwrite_while_pendingtestsettlement_contract/src/tests/conformity_tests.rsRecoveryAlreadyPendingto both error-code tablesAcceptance Criteria
RecoveryAlreadyPending(error code 15)cargo test --workspacepasses (149/149)cargo fmt --all --checkpassescargo clippy --workspace --all-targets --all-features -- -D warningspassesError Flow
The recovery address must first call
cancel_recovery(requires admin threshold) before initiating a new recovery.