fix(contracts): enforce terminal-state invariants in resolve_market and unlock_tokens, remove fee-admin key landmine - #1244
Open
vitalis200 wants to merge 1 commit into
Open
Conversation
|
@vitalis200 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! 🚀 |
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.
Body:
Summary
Fixes three related contract-safety issues in
contracts/predict-iq:closes #1186 —
disputes::resolve_market(the admin override, called fromlib.rs::resolve_market) had no guard against re-resolving an already-ResolvedorCancelledmarket. PerINVARIANTS.md§3, those states are terminal "at all times"; without the guard, an admin could call this path twice with different outcomes after bettors had already claimed under the first outcome, allowing overpayment against an unchangedtotal_staked/outcome_stakes.closes #1192 —
voting::unlock_tokenshard-requiredMarketStatus::Resolved, but aDisputedmarket can also terminate via community-votedcancel_market_vote→Cancelled. Voters who locked governance tokens throughcast_vote's fallback path (used when the governance token doesn't supportbalance_at) had no way to recover them if the market was later cancelled instead of resolved.closes #1195** —
admin::set_fee_admin/get_fee_adminread and wroteConfigKey::GuardianAccount— the same keyset_guardian/get_guardianuse — instead of the dedicatedConfigKey::FeeAdminthat the real, wired-up flow (fees::set_fee_admin/get_fee_admin) correctly uses. The functions were dead code with no callers, but calling them would have silently overwritten the guardian address that gates pause/unpause/emergency governance. Removed rather than repointed, sincefees::set_fee_adminalready implements this correctly and is the one exposed vialib.rs.closes #1193
Changes
modules/disputes.rs:resolve_marketnow rejects withErrorCode::CannotChangeOutcomeifmarket.statusisResolvedorCancelled, matching the identical guard already used incancellation::cancel_market_adminandresolution::finalize_resolution.modules/voting.rs:unlock_tokensnow acceptsResolvedorCancelledas valid terminal states.modules/admin.rs: removed the dead, miskeyedset_fee_admin/get_fee_adminfunctions.test.rs/modules/voting.rs/modules/admin.rs: added regression tests for all three fixes.Test plan
cd contracts/predict-iq && cargo test disputescd contracts/predict-iq && cargo test votingcd contracts/predict-iq && cargo test admincd contracts/predict-iq && cargo clippy --all-targets --all-features -- -D warningscd contracts/predict-iq && cargo build --releaseNote: these commands could not be run to completion in the dev environment used to prepare this PR —
maincurrently has ~27 pre-existing compile errors in unrelated production code (e.g. a missing `CANCEL_OUTCOME_INDEX