Skip to content

feat: implement M-of-N multisig admin key management (closes #103)#2

Open
maztah1 wants to merge 70 commits intomainfrom
feat/issue-103-multisig-admin-security
Open

feat: implement M-of-N multisig admin key management (closes #103)#2
maztah1 wants to merge 70 commits intomainfrom
feat/issue-103-multisig-admin-security

Conversation

@maztah1
Copy link
Copy Markdown
Owner

@maztah1 maztah1 commented Mar 25, 2026

Summary

Closes QuorumCredit#103

A single admin key controlling slash, config, and treasury is a catastrophic single point of failure. This PR adds dedicated admin key management functions so the admin set can be safely expanded, shrunk, and rotated all requiring the current quorum to sign.

What was already there

The multisig infrastructure was already in place:

  • Config.admins: Vec<Address> and Config.admin_threshold: u32
  • require_admin_approval() enforcing M-of-N on every privileged call
  • validate_admin_config() rejecting empty sets, duplicates, and impossible thresholds
  • set_config() allowing a full admin set replacement

What was missing was the ability to make surgical changes to the admin set without replacing the entire config.

New functions (src/lib.rs)

Function Description
add_admin(signers, new_admin) Append a new admin; threshold unchanged; rejects duplicates
remove_admin(signers, admin) Remove an admin; blocked if removal makes threshold unsatisfiable
rotate_admin(signers, old, new) Atomic key swap primary mitigation for a compromised key
set_admin_threshold(signers, n) Update the quorum threshold independently

All four emit on-chain events for auditability and require the current quorum to authorise.

Tests (22 new tests)

  • Threshold enforcement: zero signers, below threshold, exact threshold, above threshold
  • Non-admin signer rejection
  • add_admin: success, duplicate rejection, quorum requirement
  • remove_admin: success, unsatisfiable-threshold guard, unknown address, quorum requirement
  • rotate_admin: success, same-address rejection, existing-admin-as-new rejection, unknown old admin, quorum requirement
  • set_admin_threshold: success, zero rejection, exceeds-count rejection, quorum requirement
  • End-to-end: rotated-in key can execute operations; rotated-out key is rejected
  • validate_admin_config: empty admins, duplicate admins, threshold > count

Documentation (CONTRIBUTING.md)

Added a new Admin Key Management section covering:

  • How the M-of-N model works
  • Recommended configurations (2-of-3 for small teams, 3-of-5 for production)
  • Hardware wallet and geographic separation guidance
  • Key rotation procedures
  • Function reference table

Nexha-dev and others added 30 commits March 24, 2026 15:59
- Add DataKey::BorrowerList to track all borrowers who have taken loans
- Append borrower to BorrowerList on each request_loan call
- Add get_all_loans(page, page_size) admin-only paginated view
- Add tests: pagination correctness and empty state
- Fix pre-existing bug: duplicate initialize() calls in test setup
- Add grace_period: u64 to Config struct (default 3 days)
- Add DEFAULT_GRACE_PERIOD constant (3 * 24 * 60 * 60 seconds)
- auto_slash now enforces timestamp > deadline + grace_period using
  saturating_add to prevent u64 overflow
- grace_period = 0 is valid and restores original deadline-only behaviour
- set_config accepts any grace_period value (0 is a valid no-grace config)
- Add 8 unit tests: blocked during grace period, allowed after, boundary
  at exact threshold, one second past threshold, zero grace period,
  blocked on repaid loan, default value check, set_config update

Closes QuorumCredit#64
- Add DEFAULT_GRACE_PERIOD constant (3 days in seconds)
- Add grace_period: u64 to Config struct with doc comment
- Config::default() initialises grace_period to DEFAULT_GRACE_PERIOD
- auto_slash enforces timestamp > deadline.saturating_add(grace_period)
  using saturating_add to prevent u64 overflow
- grace_period = 0 is valid; restores immediate post-deadline slashing
- set_config comment clarifies 0 is an accepted value
- 9 unit tests: blocked during grace period, allowed after, exact
  boundary rejection, one-second-past-threshold, zero grace period,
  blocked on repaid loan, blocked on already-defaulted loan, default
  value assertion, set_config round-trip

Closes QuorumCredit#65
)

- Add extend_loan(admin_signers, borrower, new_deadline) admin function
- Add consent_extension(voucher, borrower) for voucher consent tracking
- Add get_extension_consents(borrower) view function
- Add ExtensionConsents storage key and LoanNotActive/DeadlineNotExtended errors
- Add 6 tests covering happy path, auth rejection, and edge cases
- Extract do_vouch private helper; vouch delegates to it
- Add batch_vouch(voucher, borrowers, stakes) — validates lengths match,
  rejects empty batches, aborts atomically on any per-entry error
- Add 4 tests: happy path, length mismatch, empty batch, duplicate abort
…balance before payout loop

Fixes QuorumCredit#10

- Compute total_payout (stake + proportional yield) for all vouchers
  before entering the transfer loop
- Return ContractError::InsufficientFunds early if contract balance
  cannot cover the full payout, preventing a mid-loop panic
- Removed duplicate/malformed 'if fully_repaid' block that referenced
  an undefined variable
Fixes QuorumCredit#13

- Added ContractError::BorrowerHasActiveLoan = 14
- Check has_active_loan before accepting stake transfer in vouch
- Prevents voucher stake being locked with no effect on existing loan
Fixes QuorumCredit#14

- Replace raw .is_none() storage check with has_active_loan() so repaid
  or defaulted loan records no longer permanently block vouch withdrawal
- Return ContractError::PoolBorrowerActiveLoan instead of panicking
- Return ContractError::UnauthorizedCaller when voucher not found
- Add require_not_paused guard
- Emit vouch/withdrawn event on success
- Change return type to Result<(), ContractError>
… balance

Fixes QuorumCredit#15

- Add total_yield field to LoanRecord, locked in at disbursement
- Borrower now repays loan.amount + loan.total_yield total
- outstanding = (amount + total_yield) - amount_repaid
- Vouchers receive stake + proportional yield funded entirely by borrower
- Removes pre-funded balance dependency and InsufficientFunds balance check
- Apply same total_yield calculation in create_loan_pool disbursement
Add a 60-second minimum age check on all vouches before a loan can be
requested. Vouches used in the same transaction (or block) as the loan
request share an identical ledger timestamp, so the check

  now < vouch_timestamp + MIN_VOUCH_AGE  →  VouchTooRecent

reliably blocks the vouch → request_loan → repay flash-loan pattern.

Changes:
- Add MIN_VOUCH_AGE = 60 constant
- Add ContractError::VouchTooRecent (variant 14)
- Enforce age check in request_loan after min_vouchers guard
- Add advance_past_vouch_age() test helper
- Update all existing tests to advance the clock between vouch and loan
- Add test_request_loan_rejects_vouch_too_recent
- Add test_request_loan_succeeds_after_vouch_age_elapsed
The upgrade() function already existed but had no tests and no
documented upgrade process. This commit completes the feature.

Contract:
- upgrade(admin_signers, new_wasm_hash) calls
  env.deployer().update_current_contract_wasm() after verifying
  admin_threshold signatures — same multisig quorum as all other
  admin operations
- Emits an 'upgrade' event with the new WASM hash for auditability
- A single compromised admin key cannot unilaterally upgrade

Tests added:
- test_upgrade_rejected_without_admin_approval
- test_upgrade_multisig_rejects_single_signer

Docs:
- README: full upgrade process documented (pause → install → upgrade → unpause)
- Explains why admin_threshold is required and what the WASM hash represents
Admin functions previously had no on-chain audit trail. This adds
events to every state-mutating admin function:

- pause        → ("admin", "pause",    admin, timestamp)
- unpause      → ("admin", "unpause",  admin, timestamp)
- set_min_stake     → ("admin", "minstake", admin, amount, timestamp)
- set_max_loan_amount → ("admin", "maxloan",  admin, amount, timestamp)
- set_min_vouchers  → ("admin", "minvchrs", admin, count, timestamp)
- set_config        → ("admin", "config",   admin, timestamp)
- set_protocol_fee  → ("admin", "fee",      admin, fee_bps, timestamp)
- set_reputation_nft → ("admin", "repnft",  admin, nft_contract, timestamp)
- slash_treasury    → ("admin", "treasury", admin, recipient, amount, timestamp)

upgrade and slash already emitted events — unchanged.

Tests added for every new event.
Admin actions (slash, config changes) previously took effect instantly,
giving users no time to react. This adds a two-phase timelock:

  propose_action(admin_signers, action) -> proposal_id
  execute_action(admin_signers, proposal_id)  [after TIMELOCK_DELAY]
  cancel_action(admin_signers, proposal_id)

Constants:
  TIMELOCK_DELAY  = 24 hours
  TIMELOCK_EXPIRY = 72 hours

Supported actions (TimelockAction enum):
  Slash(borrower)   - timelocked slash path
  SetConfig(config) - timelocked config update (covers admin transfer)

Events:
  (tl, proposed)  -> (id, proposer, eta)
  (tl, executed)  -> (id, admin, timestamp)
  (tl, cancelled) -> (id, admin)
 QuorumCredit#102)

- Add set_max_loan_to_stake_ratio() admin function to update the ratio
  without requiring a full set_config() call
- Add get_max_loan_to_stake_ratio() view function
- Add 7 dedicated tests covering:
  - Default ratio value (150%)
  - Admin setter updates config
  - Zero ratio rejected
  - Loan at exact ratio limit succeeds
  - Loan exceeding ratio is rejected
  - Loan below ratio limit succeeds
  - Increasing ratio allows larger loans
  - Ratio enforced correctly with multiple vouchers
…edit#103)

Security: a single compromised admin key can no longer unilaterally
control slash, config, or treasury operations.

Changes:
- Add add_admin(): quorum-gated admin set expansion
- Add remove_admin(): quorum-gated removal; blocked when removal would
  make the threshold unsatisfiable
- Add rotate_admin(): atomic key rotation preserving count and threshold;
  the primary mitigation for a compromised key
- Add set_admin_threshold(): quorum-gated threshold update
- All four functions emit on-chain events for auditability
- Add 22 dedicated multisig security tests covering:
  - Threshold enforcement (zero signers, below threshold, exact, above)
  - Non-admin signer rejection
  - add_admin: success, duplicate rejection, quorum requirement
  - remove_admin: success, unsatisfiable threshold guard, unknown address,
    quorum requirement
  - rotate_admin: success, same-address rejection, existing-admin-as-new
    rejection, unknown old admin, quorum requirement
  - set_admin_threshold: success, zero rejection, exceeds-count rejection,
    quorum requirement
  - End-to-end: rotated-in key can operate; rotated-out key is rejected
  - validate_admin_config: empty admins, duplicate admins, threshold > count
- Document key management recommendations in CONTRIBUTING.md:
  recommended M-of-N configurations, hardware wallet guidance, rotation
  procedures, and a function reference table
…all-loans-paginated

feat: implement get_all_loans paginated admin view (QuorumCredit#84)
…take-vouch

feat: fix(issue-4): reject zero-stake vouches to prevent threshold inflation
…an-extension

Feat/issue 53 loan extension
…tch-vouch

feat: implement batch_vouch function (issue QuorumCredit#55)
…ment-remove-voucher

fix-implement-remove-voucher
…/issue-10-repay-balance-check

fix(repay): pre-calculate total yield and assert sufficient contract …
…/issue-13-vouch-active-loan-check

Fix/issue 13 vouch active loan check
…/issue-14-withdraw-vouch

Fix/issue 14 withdraw vouch
…e-lockout

security: enforce MIN_VOUCH_AGE to block same-tx vouch→loan attack
…-events

feat: emit events for all admin actions with admin address and timestamp
…protocol-fee-on-repayments

feat: implement protocol fee on repayments (QuorumCredit#66)
…-max_vouchers_per_loan-cap

Implement max vouchers per loan cap
…implement-loan_count-view-function

Issue 68 implement loan count view function
…implement-default_count-view-function

Issue 69 implement default count view function
…x-loan-to-stake-ratio

feat: add set/get_max_loan_to_stake_ratio admin setter and tests (clo…
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.

Issue 103 Prevent Admin Key Single Point of Failure — Recommend Multisig