Fix fee waivers and tooling verification - #640
Open
Busiii-adetiba wants to merge 473 commits into
Open
Conversation
…ontract feat(treasury): scaffold Treasury contract for protocol fee collection
…treasury-contract feat(treasury): implement TreasuryContract and wire 50 bps fee into Market withdrawals
…139-oracle-adapter-interface feat: resolve centralization risk
…cle-canonical-format feat: align oracle signature format with on-chain keccak message
…ich-market-events Enrich market lifecycle events with actors
…-version-marker feat: add StorageVersion marker to guard storage layout upgrades
…-market-price-bps feat: persist market_price_bps on Market, update on every trade
Title: feat: add proptest fuzz suite for share and collateral math
…fee-deduction Done with the validation
Add public query endpoints get_market and get_position
closes: Vatix-Protocol#263 Add an admin-only `cancel_market` flow that lets the protocol halt a market before it is resolved, plus the collateral-reclaim path that the cancel policy requires. What changed - `cancel_market(admin, market_id)`: admin-authorized transition of an Active market to `MarketStatus::Canceled`. Rejects non-admin callers (NotAdmin), unknown markets (MarketNotFound), already-resolved markets (MarketAlreadyResolved), and redundant cancels (MarketNotActive). - `withdraw_canceled_collateral(user, market_id)`: refunds a user's full deposited collateral from a canceled market, zeroing their position and transferring the SAC tokens back. This satisfies the "allow collateral withdrawal per policy" requirement without altering the standard withdraw path (which is reserved for active markets). - `validation::validate_cancelable(status)`: single home for the cancel policy (Active only), keeping the entry point declarative. - `events::MarketCanceledEvent` + `emit_market_canceled`: indexed by market_id so off-chain consumers can detect cancellations. - Tests: admin/non-admin, not-found, already-resolved, already-canceled, deposit-rejected-after-cancel, trade-rejected-after-cancel, reclaim success, reclaim on active/ no-position rejection, plus unit tests for the new validation helper and event. Why `MarketStatus::Canceled` existed in `types` but there was no way to halt a market without resolution, and no path for users to recover collateral from a halted market. Assumptions - Cancellation is only valid before resolution; a resolved market has a final outcome and is not cancelable. - On cancellation users are made whole by refunding their full deposit, since there is no winning outcome to settle. - Deposit and update_position already gate on `MarketStatus::Active`, so canceled markets reject new deposits/trades with no change required; tests assert this behavior. Note: this branch contains only the Vatix-Protocol#263 feature. The base `dev` branch currently fails to compile due to pre-existing errors in storage.rs (duplicate get_treasury/set_treasury, a stray paren, missing StorageKey variants); those are intentionally left untouched. The feature was verified green (all 15 new tests pass) against a locally patched base. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
closes: Vatix-Protocol#270 Add a workspace-level integration test that drives the whole protocol loop through the public MarketContract client and asserts both SAC token balances and emitted events at each stage. What changed - tests/helpers/mod.rs: add reusable harness helpers — register_contract (bootstraps admin + storage version), oracle_keypair, and sign_outcome (builds a valid Ed25519 resolution signature via oracle::construct_oracle_message), plus a shared STROOPS_PER_USDC constant. - tests/market_test.rs: add full_protocol_loop_deposit_trade_resolve_settle covering initialize → create market → deposit → update_position → resolve → settle → payout, asserting collateral moves user→contract on deposit and contract→user on settle, and that each step emits its event. Why Root tests/*.rs lacked a full protocol-loop test at the workspace level and the existing helpers were unused. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
closes: Vatix-Protocol#271 Replace the hardcoded zero-fee placeholder in the withdraw path with a real basis-points fee that is computed, charged, and forwarded to the treasury. What changed - contracts/market/src/withdraw.rs: - Read the configured fee rate via storage::get_fee_rate_bps and validate it. - Compute the fee on the requested amount with validation::calculate_fee (zero rate -> zero fee). Previously fee_amount was hardcoded to 0 and the code referenced an undefined `total_deposited_after_fee`. - Derive available collateral from total_deposited minus the fee minus the locked amount; require amount + fee <= available. - Emit FeeCalculated with the now non-zero fee, and when a treasury is registered transfer the fee and record it via the treasury's `collect_fee` entry point. - contracts/market/src/storage.rs: - Add the missing StorageKey::FeeRateBps and StorageKey::Treasury variants that the fee-config and treasury accessors require. - Remove the duplicate instance-based get/set/has_treasury functions left by an earlier merge (the persistent Option-based accessors are the ones the withdraw path uses), and fix get_next_market_id so the module parses. - contracts/market/src/validation.rs: add validate_fee_rate_bps (0–10_000 bps) used by the withdraw path, with unit tests. The treasury contract (contracts/treasury) already exposes a matching `collect_fee(caller, token, market_id, fee_amount)`; the withdraw path invokes it with exactly those args. Why emit_fee_calculated emitted zeros and the treasury was never actually called, so no protocol fees were charged or collected. Assumptions - The fee rate is held in contract storage (StorageKey::FeeRateBps, default 0) and the treasury address in StorageKey::Treasury; both are configured by the admin out of band (set_treasury already exists; fee-rate configuration is read here). - The fee is charged on the withdrawn amount so the user nets exactly `amount` while `amount + fee` leaves their deposited balance. Note: the wider `dev` base still has unrelated pre-existing compile errors (e.g. admin handling in lib.rs) that are outside this issue's scope and left untouched; CI will not go green until those are repaired separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
closes: Vatix-Protocol#252 Replace the echo-guard deploy tooling with a real build/deploy/invoke flow and make the Makefile the single canonical build approach. What changed - scripts/deploy-testnet.sh: builds via `stellar contract build`, locates the WASM artefact, deploys to testnet with a funded account secret (TESTNET_SECRET_KEY), prints the contract ID, and exports it to $GITHUB_OUTPUT (`contract_id`) for a downstream smoke-invoke step. - scripts/invoke-example.sh: real `stellar contract invoke` smoke test against a read-only function (get_treasury) on the just-deployed contract, consuming CONTRACT_ID from the deploy step. Non-zero exit on failure. - contracts/market/Makefile: removed stale "echo guard" TODO comments; `build` (`stellar contract build`) is documented as the single canonical build used by CI and the deploy scripts (artefact: target/wasm32v1-none/release/*.wasm). Why Deploy scripts only echoed their intent and the Makefile (`stellar contract build`) and CI (`cargo build --target wasm32-unknown-unknown`) disagreed on how to build, producing artefacts at different paths. Assumptions - Testnet credentials are provided via the `TESTNET_SECRET_KEY` repository secret (a funded testnet account secret key). The scripts skip/fail loudly when it is absent. Note: the corresponding `.github/workflows/ci.yml` change (install Stellar CLI, build via `make build`, and a gated `testnet-deploy` job that runs the two scripts and passes the contract ID through) is intentionally NOT included in this commit because the push token lacks the GitHub `workflow` OAuth scope. The workflow wiring is ready and should be applied in a follow-up by a token with `workflow` scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…e-token contract storage
…lved event - Add resolver: Option<Address> and resolved_at: Option<u64> fields to Market struct - Update MarketResolvedEvent to include both oracle_pubkey and resolver Address - Modify resolve_market to accept resolver parameter and require_auth() - Store resolver and resolved_at in market upon resolution - Update all tests and integration tests to pass resolver address
…rify oracle signature in resolution propose Closes Vatix-Protocol#322 Closes Vatix-Protocol#323 Closes Vatix-Protocol#324 Closes Vatix-Protocol#328
…cel-market-admin-flow feat(market): implement cancel_market admin flow (Vatix-Protocol#263)
…-workspace-test test(e2e): full deposit→trade→resolve→settle→payout workspace test (Vatix-Protocol#270)
…l-fee-bps-treasury feat(fees): real withdrawal fee bps wired to treasury collect_fee (Vatix-Protocol#271)
…l-testnet-deploy-ci ci(deploy): real testnet deploy + smoke invoke scripts, aligned build (Vatix-Protocol#252)
…e-token-update-position Wire market update_position to outcome-token mint/burn and add outcome-token contract storage
…ally gates deposits and withdrawals The Paused storage flag and every require_not_paused() gate (including on deposit_collateral and withdraw_unused_collateral) already existed, but no contract entrypoint could ever set the flag - pause() and unpause() were missing entirely, so the emergency halt was permanently unreachable. Adds admin-gated pause/unpause/is_paused entrypoints, wires them to the existing emit_emergency_pause_toggled event, and adds tests asserting a paused market rejects deposit_collateral and withdraw_unused_collateral, that unpause restores both paths, and that a non-admin cannot toggle the flag.
…olved markets settle_position and batch_settle_positions already reject any market whose status isn't Resolved (via validate_settlement_eligibility), and that was covered for Active markets, but not for Canceled - and settle_positions_page (the paginated settlement path) had no rejection coverage at all. Adds Canceled-market negative tests for validate_settlement_eligibility and batch_settle_positions, Active/Canceled rejection tests for settle_positions_page, and a page-settle happy-path test confirming a Resolved market still pays out correctly.
…cument auth audit Audited every admin mutator in the market, treasury, and resolution contracts for require_auth() + caller-equals-stored-admin checks. Treasury and resolution were already fully covered. Market was missing set_resolution_contract entirely: storage::set_resolution_contract/ get_resolution_contract and tests referencing client.set_resolution_contract already existed, but the contract never exposed the entrypoint that calls them, so the resolution-contract gate could never actually be configured. Adds the admin-gated setter and a getter, a consolidated non-admin-rejection test across the admin mutator set, and AUTH_TABLE.md documenting the full audit as required by the acceptance criteria.
set_fee_rate already rejected proposals above the fee cap, but two gaps left the cap ineffective in practice: (1) execute_fee_rate_change applied a previously-approved pending rate without re-checking it against the *current* cap, so lowering the cap after a proposal but before its timelock elapsed did not stop the stale, now-excessive rate from landing; and (2) there was no set_fee_cap entrypoint at all, so FeeCap storage could never actually be changed from its default (MAX_FEE_RATE_BPS), making the cap check at propose time a no-op in production. Adds an admin-gated set_fee_cap entrypoint plus its storage helper, re-validates the cap inside execute_fee_rate_change, and adds tests for over-cap rejection and at-cap acceptance on both the set and execute paths.
When a withdrawal fee is configured but no treasury address has been registered, the fee was silently retained in the contract's own token balance with only a one-line code comment noting it. Document the chosen behavior (skip the transfer, never revert the withdrawal), emit a new FeeRetainedNoTreasury event whenever this happens so it is observable on-chain, and add unit + integration tests covering both the treasury-set and treasury-unset configurations, including the transition from unset to set.
…ited Document the locked_collateral <= total_deposited invariant directly on the Position type (which functions enforce it and how), and add a dedicated table-driven test file covering net-YES, net-NO, hedged, and exact-boundary scenarios plus a documented failure case: an over-leveraged trade is rejected with InsufficientCollateral and leaves the stored position completely unchanged, rather than clamped. This complements the existing randomized/proptest coverage already in tests/collateral_invariant_test.rs and tests/proptest_locked_invariant.rs.
update_position, deposit_collateral, and withdraw_unused_collateral already gate on market.status != Active, so a Canceled market was already rejected via ContractError::MarketNotActive — but this path was only ever exercised against Resolved markets in existing tests. Add explicit Canceled-market coverage (buy, sell, deposit, withdraw) plus a before/after regression test showing the exact same trade succeeds while Active and fails immediately once canceled. Clarify the behavior in update_position's doc comment.
…views get_position and get_net_position read straight from position storage, which is keyed by (market_id, user) and returns Ok(None) whether the market doesn't exist or the market exists but the user has no position — the two cases were indistinguishable to callers. Both views now check market existence first and return ContractError::MarketNotFound for a missing market_id, leaving the Ok(None)/Ok(0) happy path for a real market with no position untouched. Added tests and a short doc note on the scope decision to leave the plain-return cross-contract views (get_market_status, get_collateral_token) untouched.
Complete the previously stubbed set_resolution_contract entrypoint and add the resolution gate itself: when a resolution contract is registered, resolve_market now cross-calls into it and requires a Finalized candidate whose outcome/signature match the call, so the only path to a resolved market is ResolutionContract::finalize's callback. Unset resolution keeps prior direct-oracle-signature behavior unchanged. Reuses the existing ResolutionNotFinalized error and get/set_resolution_contract storage helpers, which were already present but unwired.
…age for collect_fee collect_fee's authorized-market gate was already fully implemented (storage::is_authorized_market + CallerNotMarket, wired up since the v2 registry). Add two edge-case regression tests (re-add after removal; never-registered caller) and document the audit findings, since no production code change was required.
…gate mint/burn already required config.market_contract.require_auth() in production code, but every existing test ran under env.mock_all_auths(), which bypasses require_auth entirely and so never actually exercised that gate. Add a second harness (setup_unmocked) using scoped env.mock_auths instead, with positive tests proving the market-contract path succeeds and should_panic tests proving an unauthorized caller cannot mint/burn.
…d max Add a fee_rounding_invariants proptest module covering validation::calculate_fee across the full fee_rate_bps range (0-10_000) and amount edges near zero and near the i128::MAX/2 reasonable-amount ceiling. Surfaces and asserts the correct behavior for a real overflow edge case: amount * fee_rate_bps can exceed i128::MAX at near-max amounts even though amount itself is within the validated range, and calculate_fee must fail closed with ArithmeticOverflow rather than panic or wrap. Documents the floor-division dust rule (fee is additive on top of amount, not carved out of it; up to 9_999 stroops of amount*bps rounds down and is never collected).
Prepend a fixed ASCII domain separator (VATIX_ORACLE_V1) to the keccak256 preimage the oracle signs, so a resolve signature can never be replayed against a different message scheme that happens to hash the same raw market_id/outcome bytes. Document the exact preimage layout and width in the module docs. Add validate_oracle_preimage_len / hash_oracle_preimage_checked as an explicit, tested bound on the preimage width so any future entrypoint that accepts a raw preimage rejects truncated/oversized input with InvalidSignature instead of hashing malformed data or trapping. Update the exported oracle-message.json test vector generator to the new preimage layout. Add tests covering: exact-width construction, truncated/oversized/empty rejection, and that a legacy preimage without the domain separator no longer verifies.
settle_position already guarded against double payout via the persisted Position.is_settled flag (checked before any payout math or transfer, across the single/batch/paginated settlement paths), but there was no dedicated test asserting the *effect* of a repeat call. Add test_second_settle_position_cannot_double_pay, which settles once then calls settle_position three more times and asserts each repeat is rejected with PositionAlreadySettled while user/contract token balances and the stored position stay byte-for-byte unchanged. Add SETTLEMENT_IDEMPOTENCY.md documenting the existing guarantees and what the new test covers, since the production-code diff here is small (the idempotency logic was already correct).
FeeCollected (market_id + amount, on fee-to-treasury transfer) already existed and was already only emitted for a strictly positive fee_amount (collect_fee rejects <= 0; withdraw_unused_collateral skips the cross-contract call entirely when fee_amount is 0), so zero-fee withdraws never emitted a misleading amount. That behavior only had unit coverage from calling TreasuryContract::collect_fee directly. Add withdraw_emits_fee_collected_event_with_market_id_and_amount, which drives a real withdraw_unused_collateral call, locates the fee_collected_event among the events fired around it, and asserts its topics (market_id, token) and data (fee_amount, new_token_balance, new_cumulative_fees) match what was actually charged. Extend the existing zero-fee-rate test to assert no such event fires when there is nothing to report. Add FEE_COLLECTED_EVENT.md documenting the existing guarantees and the added coverage, since production code was unchanged.
STORAGE_VERSION had already been bumped to 4 in storage.rs (v4 adds StorageKey::AdapterEnabled for Vatix-Protocol#488, documented inline there), but STORAGE_MIGRATION_GUIDE.md's Version History section was never updated — it still showed "Version 3 (Current)" with no v4 entry. Add the missing Version 4 section to the guide, demote the old Version 3 (Current) heading, and fix the stale STORAGE_VERSION = 3 code snippet in the Overview. Add test_storage_version_documented_in_migration_guide, which reads the guide via include_str! and fails if there is no "### Version {STORAGE_VERSION} (Current)" heading, or if more than one section is marked (Current). This turns a future version bump without a guide update into a failing test instead of a review-time catch. Add STORAGE_VERSION_CI_CHECK.md summarizing the drift found and what the new test covers, since the code diff here is small.
Extend apps/web/lib/errors.ts with a full code->copy table for the market contract's ContractError enum, so deposit, withdraw, and admin auth failures surface clear, specific messages instead of a generic "error #N" string. Add unit tests covering deposit/withdraw/auth mappings, the unknown-code fallback, and existing simulation/tx/wallet rejection branches. Closes Issue 1: Surface ContractError codes in web error mapper.
Add a build-contracts CI job that runs `cargo build -p <crate>` for market, treasury, resolution, and outcome-token independently of the existing longer ci/frontend pipelines, so a compile error in any contract crate fails the PR check red on its own. Closes Issue 2: Add CI job that builds all contract crates.
Add a "Reviewer Checklist: StorageKey Table Drift" section to STORAGE_MIGRATION_GUIDE.md explaining how to verify the src/storage.rs StorageKey enum stays in sync with the storage layout doc table in src/lib.rs, and link it from README.md. Fix the drift the checklist caught: Paused, AdapterEnabled, and DepositLock were missing from the lib.rs table. Closes Issue 3: Document StorageKey table drift check.
Add cancel_admin_transfer to MarketContract so the current admin can clear an outstanding propose_admin nomination before it's accepted, reusing the existing storage::clear_pending_admin helper. Emit a new AdminTransferCanceled event and add tests covering: clearing pending state, event emission, non-admin rejection, no-pending rejection, and the key acceptance criterion that a canceled nomination cannot later be accepted. Closes Issue 4: Test admin two-step transfer cancel path.
feat: docs/ci: bindings drift check, testnet registry, smoke test, event reference
feat: fix: issues 502, 503, 504, 513 — audit event, workspace CI, math corpus, contributing guide
…-pause-settlement-auth-feecap Fix/market hardening pause settlement auth feecap
…ariant-canceled-market-views Fix/treasury invariant canceled market views
…easury-outcometoken-fee-fuzz-hardening Fix/resolution treasury outcometoken fee fuzz hardening
…ment-events-storage-hardening Fix/oracle settlement events storage hardening
…-ci-storage-docs-admin-cancel Fix/error mapper ci storage docs admin cancel
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
19 participants
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:
Closes #566
Closes #567
Closes #568
Closes #569