test(settlement): pin admin-set guard on empty and duplicate recovery targets - #594
Merged
therealjhay merged 2 commits intoAug 21, 2026
Conversation
… targets execute_recovery writes DataKey::Admin directly rather than going through validate_admins_and_threshold, and the existing suite only covered the recovery happy path. Nothing asserted that the post-recovery admin set is non-empty and duplicate-free, or that the guard rejects those shapes. Add settlement_contract/src/tests/recovery_admin_set_tests.rs covering: - validate_admins_and_threshold directly: empty sets (threshold 0 and 1) are rejected with InvalidThreshold, duplicate entries (adjacent and non-adjacent, at threshold 1 and 2) with InvalidAdmin, and a zero-address member with ZeroAddress; a distinct set is accepted. - The set execute_recovery installs: exactly one admin at threshold 1, and it passes validate_admins_and_threshold. Recovering to an address that is already an admin collapses to a single entry instead of duplicating it. - initiate_recovery rejects a zero-address target, so an invalid admin can never reach the pending recovery record. - transfer_admin from the recovered lone admin rejects both an empty target set and a duplicate target set, and a rejected transfer leaves the stored admin set and threshold untouched.
Contributor
|
Kindly resolve conflict |
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 #582
Problem
execute_recoverywritesDataKey::Admindirectly rather than going throughvalidate_admins_and_threshold, and the existing suite insettlement_contract/src/tests/admin_tests.rsonly exercised the recovery happy path (recovery_executes_after_delay). Nothing asserted:execute_recoveryinstalls is non-empty and duplicate-free,validate_admins_and_thresholdactually rejects an empty or duplicate admin set, andChanges
Added
settlement_contract/src/tests/recovery_admin_set_tests.rsand wired it intosettlement_contract/src/tests/mod.rs.validate_admins_and_threshold— the guard itselfvalidate_rejects_empty_admin_set_with_threshold_one/..._with_zero_threshold— empty admin vector is rejected withInvalidThreshold(Fix Settlement Rule Validation (Sum Limit) #26).validate_rejects_duplicate_admins,validate_rejects_duplicate_admins_at_threshold_one,validate_rejects_non_adjacent_duplicate_admins— the duplicate-target test called out in the issue, covering adjacent and non-adjacent duplicates at threshold 1 and 2, all rejected withInvalidAdmin(Custom Error for Admin Authorization Failure #14).validate_rejects_zero_address_in_admin_set— a zero-address entry anywhere in the set is rejected withZeroAddress(Audit Event Publishing Payload inremove_anchor#21).validate_accepts_distinct_admin_set— a legitimate distinct set is accepted (negative control).Post-recovery admin set
recovery_settles_on_non_empty_single_admin_set— afterinitiate_recovery+execute_recovery,get_admin()holds exactly one address,get_threshold()is 1, and the resulting set passesvalidate_admins_and_threshold— pinning that recovery can never settle on an empty set.recovery_to_existing_admin_does_not_duplicate_admin_set— recovering to an address that is already in the admin set collapses to a single entry rather than storing it twice.initiate_recovery_rejects_zero_address_target— an invalid recovery target is rejected before it can ever reach the pending-recovery record.transfer_adminfrom the recovered adminrecovered_admin_cannot_transfer_to_empty_admin_set— the freshly recovered lone admin cannot transfer to an empty set (InvalidThreshold, Fix Settlement Rule Validation (Sum Limit) #26).recovered_admin_cannot_transfer_to_duplicate_admin_set— nor to a set with a duplicated target (InvalidAdmin, Custom Error for Admin Authorization Failure #14).rejected_duplicate_transfer_leaves_recovered_admin_set_intact— usestry_transfer_adminto confirm both rejections leaveget_admin()/get_threshold()untouched.Testing
cargo test --workspace— all 62 tests pass (33 governance + 29 settlement, including the 12 new tests here).cargo fmt --all -- --check— clean.cargo clippy --all-targets -- -D warnings— clean.This is test-only; no production code changes.