Fix/four correctness issues#1223
Open
Mac-5 wants to merge 4 commits into
Open
Conversation
added 4 commits
July 25, 2026 22:56
backup_storage_state previously wrote only a timestamp under a version-keyed marker, and restore_storage_state just checked the marker existed and removed it -- no storage was ever actually reverted, so a failed/invalidated migration left any mutations from migration_fn in place despite the "rollback" claim. Snapshot and restore the actual values of ConfigKey::Admin and ConfigKey::GuardianSet (the keys verify_migration_integrity checks) so a validation failure or migration_fn error genuinely reverts them. Documented the scope of what is/isn't covered, and wired reverse_migration through the same restore path for consistency. Added a test that tampers with both keys mid-migration and asserts they are restored to their original values, not just that a marker was cleared.
voting::prune_market_voting_state existed to clear Vote, VoteTally, LockedTokens, LockedBalance, and the DisputeVoters registry for a market, but markets::prune_market never called it. Every disputed-and-resolved market was leaving its full voting-state footprint in persistent storage forever after being pruned, growing unbounded rent/storage costs. Call voting::prune_market_voting_state from prune_market before removing the market record. Added a test that seeds a resolved market's voting-state footprint directly, prunes it, and asserts every voting-state entry is gone.
emit_fee_collected's address parameter meant "the actual recipient" at the market-creation call site (treasury) but "the contract's own address" at the per-bet fee call site, and never identified which token the fee was collected in even though collect_fee tracks revenue per-token via DataKey::FeeRevenue(token). Indexers couldn't reliably interpret the event across call sites. Added an explicit token parameter to emit_fee_collected and pass the real token address from both collect_fee and create_market_with_dispute_window, so the event now consistently carries (token, recipient) at every call site. Added a test asserting the event emitted by collect_fee includes the correct token address.
The module was documented as tripping CircuitBreakerState::Open after 10 tracked errors, but nothing in the codebase ever called track_error on a real failure -- there was no functioning automatic protection against a burst of failing operations, only manual admin/guardian pause. Call monitoring::track_error from sac::safe_transfer's failure paths and oracles::validate_oracle_staleness's StalePrice/OracleFailure paths. Added a test that drives repeated transfer failures through safe_transfer and asserts the circuit breaker opens after the documented threshold.
|
@Mac-5 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
1 participant
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.
Description
176b05e — migration rollback (modules/migration.rs)
backup_storage_state used to store just a timestamp; restore_storage_state only checked/cleared that marker — no state was ever actually reverted. Now both snapshot and restore the real values of ConfigKey::Admin and ConfigKey::GuardianSet (the two keys verify_migration_integrity checks), via a new MigrationBackup struct. reverse_migration now routes through the same restore path instead of duplicating the old fake-check. Doc comments spell out that rollback scope is limited to those two keys — a migration_fn touching other storage must be self-reversible. Rewrote the existing test to tamper with both keys and assert they come back to their original values, not just "the marker was cleared."
b1ad505 — voting-state pruning (modules/markets.rs)
prune_market deleted the market record but never called voting::prune_market_voting_state, so every disputed-and-resolved market left its Vote/VoteTally/LockedTokens/LockedBalance/DisputeVoters entries in storage forever. Added the call before the record is removed. New test seeds a resolved market plus a full voting-state footprint directly, prunes it, and asserts every voting key is gone.
a7c6178 — fee event standardization (modules/events.rs, modules/fees.rs, modules/markets.rs)
emit_fee_collected's address param meant "recipient" at one call site and "the contract itself" at the other, and never carried the token. Added an explicit token parameter; both call sites (collect_fee, create_market_with_dispute_window) now pass the real token address alongside the recipient. New test asserts collect_fee's emitted event contains the token address.
76bd4e8 — circuit breaker wiring (modules/monitoring.rs, modules/sac.rs, modules/oracles.rs)
track_error was documented as tripping the breaker after 10 errors but nothing ever called it. Wired it into sac::safe_transfer's two failure paths and oracles::validate_oracle_staleness's StalePrice/OracleFailure paths. New test drives 11 failed transfers through safe_transfer and asserts CircuitBreakerState::Open.
Type of Change
Testing Done
Bundle Size
Checklist
docs/architecture.mdRelated Issues
Closes #1196
Closes #1197
Closes #1198
Closes #1199