Skip to content

Fix/issue 19 admin guardian separation#322

Merged
hman38705 merged 4 commits intosolutions-plug:mainfrom
Mystery-CLI:fix/issue-19-admin-guardian-separation
Mar 27, 2026
Merged

Fix/issue 19 admin guardian separation#322
hman38705 merged 4 commits intosolutions-plug:mainfrom
Mystery-CLI:fix/issue-19-admin-guardian-separation

Conversation

@Mystery-CLI
Copy link
Copy Markdown
Contributor

Title: fix: enforce strict separation between Admin and Guardian identities (#19)

Body:

Summary

Prevents the Admin address from being added to the Guardian set, closing the governance bypass where a single entity
could control both upgrade initiation and voting majority.

Changes

governance.rs

  • add_guardian: added identity check — returns NotAuthorized if guardian.address == admin
  • initialize_guardians: same check applied at init time, preventing the admin from being embedded in the initial
    Guardian set before the restriction could be enforced

Tests

  • test_add_admin_as_guardian_rejected — confirms add_guardian with the admin address returns NotAuthorized
  • test_initialize_guardians_with_admin_rejected — confirms initialize_guardians with the admin address in the list
    returns NotAuthorized

closes #127

- Replace client.transfer() (host panic) with client.try_transfer()
  in safe_transfer() so failures are caught as Result instead of
  crashing the host environment
- Map both invocation-level and contract-level errors to
  ErrorCode::TransferFailed (= 52)
- Emit xfer_fail event (topics: [xfer_fail, from, to],
  data: (token_address, amount)) on failure for indexer observability
- Add TransferFailed = 52 to ErrorCode enum
- Add test_transfer_failure_returns_error_not_panic to verify a
  zero-balance bettor returns Err instead of a host panic

Closes solutions-plug#11
- Set COOLDOWN_SECONDS to 6 hours (was 1 hour)
- Add HALF_OPEN_MAX_OPS=5 to limit probe transactions in HalfOpen
- Add DataKey::HalfOpenOps to track ops during HalfOpen
- _set_state_internal records OpenedAt on Open, resets HalfOpenOps on HalfOpen
- require_closed auto-triggers Open->HalfOpen via maybe_recover on each call
- HalfOpen trips back to Open after max ops exceeded
- Add tests: auto-recovery after cooldown, re-trip after max ops

Closes solutions-plug#12
- Remove duplicate TIMELOCK_DURATION / MAJORITY_THRESHOLD_PERCENT constants
- Add TIMELOCK_MIN_SECONDS (6h) and TIMELOCK_MAX_SECONDS (7d) bounds
- Add ConfigKey::TimelockDuration for persistent storage override
- Add get_timelock_duration() — returns stored value or 48h default
- Add set_timelock_duration() — admin-gated, enforces [6h, 7d] range
- Update is_timelock_satisfied() to use get_timelock_duration()
- Fix duplicate use imports in governance.rs
- Tests: early execution after reducing to 6h, out-of-range rejection

Closes solutions-plug#13
- add_guardian: reject if guardian.address == admin (Issue solutions-plug#19)
- initialize_guardians: reject any guardian whose address matches admin
- Tests: add_admin_as_guardian_rejected, initialize_guardians_with_admin_rejected

Closes solutions-plug#19
@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Mar 27, 2026

@Mystery-CLI 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! 🚀

Learn more about application limits

@hman38705 hman38705 merged commit 82b5577 into solutions-plug:main Mar 27, 2026
2 of 13 checks passed
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.

Prevent Admin from Compromising Guardian Integrity

2 participants