Fix unbacked value creation and enforcement gaps in vault, treasury, quorum, and compliance - #1925
Open
Idaonoli wants to merge 4 commits into
Open
Conversation
…ree total_amount inflation fund_vault incremented vault.total_amount by the caller-supplied amount without ever debiting the funder, letting any address inflate a vault's claimable value at zero cost. Escrow the funder's tokens into contract custody via a real token transfer before crediting total_amount, mirroring the pattern already used by bridge.rs::lock_tokens and fractionalization.rs::fractionalize, and matching how claim_vault later pays vault.token out of contract custody. Closes Emmyt24#1850
…pient withdraw_fees validated the withdrawal against the daily cap/allowlist, recorded it against the period tracker, and emitted a treasury_withdrawal event, but never moved any value to recipient. Credit the recipient with a real token transfer from the contract's own custody (the same fee_token used by token_creation.rs) after the withdrawal is recorded, so the emitted event matches reality. Closes Emmyt24#1851
…ve quorum recalculation check_and_trigger_reactive_recalculation reacted to a large supply change by calling record_participation with hardcoded votes_cast=50, eligible_voters=100 — a fake 50% participation snapshot completely disconnected from real voting activity. Since compute_effective_quorum blends the base quorum with the average of recent participation snapshots, every large mint/burn silently skewed the quorum used for future proposals. Remove the fabricated snapshot; the trigger now only emits its qrm_trig signal event without touching the participation ring buffer, so compute_effective_quorum still recalculates against only genuine snapshot history. Closes Emmyt24#1852
check_compliance evaluated jurisdiction rules like TransfersSuspended, MaxTransferAmount, MinTransferAmount, and FrozenAddressBlocked, but nothing in the contract ever called it before moving value — every rule an admin registered had zero effect on mint, burn, or admin_burn. Add a per-token compliance jurisdiction (storage-derived, defaulting to a "GLOBAL" jurisdiction, never left to the caller to supply arbitrarily) settable only via the new admin-only set_token_jurisdiction entry point, and gate mint, burn, and admin_burn on check_compliance before they commit any state changes, propagating Error::ComplianceCheckFailed on rejection. Closes Emmyt24#1853
|
@Idaonoli 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.
Summary
fund_vaultincrementedvault.total_amountwithout ever debiting the funder, letting anyone inflate a vault's claimable value for free. It now escrows the funder's tokens into contract custody via a real token transfer (token::Client::transfer) before creditingtotal_amount, matching the pattern inbridge.rs::lock_tokens/fractionalization.rs::fractionalizeand howclaim_vaultalready paysvault.tokenout of contract custody.withdraw_feesvalidated the withdrawal against the cap/allowlist and emitted an event, but never moved any value to the recipient. It now transfersamountof the crate'sfee_tokenfrom the contract's own custody torecipient.check_and_trigger_reactive_recalculationinjected a hardcodedvotes_cast=50, eligible_voters=100fake snapshot into governance participation history on every large supply change, skewingcompute_effective_quorum. It now only emits theqrm_trigsignal event and never touches the participation ring buffer.check_compliancewas never called by any real token operation, so registered rules (includingTransfersSuspended) had zero effect. Added a per-token compliance jurisdiction (storage-derived, defaulting to"GLOBAL", settable only via a new admin-onlyset_token_jurisdiction) and wiredmint,burn, andadmin_burnto callcheck_compliancebefore committing state changes.None of the four public function signatures named in the issues were changed.
Test plan
Each issue's own description required a regression test, so tests were added alongside each fix:
cargo test --lib vault— newtest_fund_vault_insufficient_balance_fails, updated existing funding tests to use real SAC tokenscargo test --lib treasury— newtest_withdraw_fees_credits_recipient_balance,test_withdraw_fees_without_fee_token_configured_failscargo test --lib dynamic_quorum— newtest_reactive_recalculation_does_not_skew_effective_quorum, updatedtest_large_supply_change_triggers_reactive_recalculationcargo test --lib compliance_reporting— newtest_transfers_suspended_blocks_mint,test_transfers_suspended_blocks_burn,test_transfers_suspended_blocks_admin_burn,test_mint_unaffected_by_other_jurisdiction_rulecargo fmt --check/cargo clippy --all-targets --all-features -- -D warningsNote: this sandbox has no local Rust toolchain, and the repo has no committed
Cargo.lock; a fresh dependency resolution here hits ~429 pre-existing compile errors onupstream/mainitself (unrelated to this change — confirmed identical error signature/count against a cleanupstream/maincheckout), so these commands should be run in CI to confirm green.