Skip to content

fix: cap maximum guardians to 9 in add_guardian - #75

Open
Ash739-ux wants to merge 1 commit into
Orbit-Wal:mainfrom
Ash739-ux:fix-guardian-cap
Open

fix: cap maximum guardians to 9 in add_guardian#75
Ash739-ux wants to merge 1 commit into
Orbit-Wal:mainfrom
Ash739-ux:fix-guardian-cap

Conversation

@Ash739-ux

Copy link
Copy Markdown

Closes #27

pr description

Context & Problem

Currently, the asset registry implements an explicit, documented cap (MAX_ASSETS = 50) to keep the contract well within Soroban's ~100 KB instance storage ceiling. However, the add_guardian function lacked an equivalent bound.

The Guardians vector lives in instance storage, meaning it shares a very tight size budget with every other piece of contract instance state (Admin, PendingUpgrade, RecoveryConfig, etc.). Without a cap:

  1. Storage Risk: An admin (or compromised admin key) could continuously add guardians until instance storage is exhausted or prohibitively expensive to write, as every mutation rewrites the entire Vec<Address>.
  2. Compute/DoS Risk: Validations like require_guardian, add_guardian duplicate scans, and remove_guardian rebuilds require O(n) linear scans. An inflated guardian list makes the social recovery path progressively more expensive, potentially bricking the exact recovery mechanism the guardians are meant to protect.

Solution

This PR introduces a strict upper bound to the guardian list, mirroring the protective design of the asset registry:

  • MAX_GUARDIANS Constant: Added and set to 9. This value is consistent with realistic social-recovery configurations (which rarely exceed 5-9 members) while keeping instance storage consumption nominal and bounding O(n) scan costs.
  • Dedicated Error Variant: Added WalletError::GuardianLimitExceeded (code 1031).
  • Enforcement: add_guardian now explicitly checks the list length before allowing a push_back operation.
  • Test Coverage: Added test_guardian_list_has_upper_bound to ensure that adding guardians up to the cap succeeds, but any subsequent addition properly triggers the GuardianLimitExceeded error.

Impact

  • Security: Eliminates a potential DoS vector where the recovery mechanism could be rendered too expensive to execute.
  • Performance: Ensures predictable fee costs for all guardian-related operations by putting a strict mathematical ceiling on the linear scans.
  • Storage: Safeguards the shared instance storage space from being bloated by an unbounded vector.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No upper bound on the guardian set — unlike MAX_ASSETS, add_guardian allows unbounded growth

2 participants