Fix #57: emit fee provenance in withdraw events - #187
Conversation
Track which markets contribute to each fee withdrawal so indexers can detect cross-market contamination. The fees_withdrawn event now includes a Vec<FeeProvenance> with (market_id, amount) pairs in drain order (legacy first, then newest-to-oldest). Changes: - Add FeeProvenance struct (market_id, amount) - debit_proven_fees now returns the provenance it debits - Add preview_fee_provenance helper for non-mutating reads - Add get_withdrawal_provenance view function for integrators - Update withdraw_fees and execute_withdraw_fees events to include provenance - Update event schema comment to document new format Existing safeguards (20% cap, 24h timelock, per-market ledger, admin cancellation) remain unchanged.
Muyideen-js
left a comment
There was a problem hiding this comment.
@praizeD10 The PR adds fee provenance tracking but does not address the critical security requirements in the issue. The issue explicitly requires a per-transaction cap, per-market segregation, and a timelock. The diff only modifies event emission and adds a preview function; the withdrawal logic still allows a recipient to drain the entire accumulator in one call, and there is no restriction on which markets' fees can be withdrawn. Please implement the missing safeguards: add a cap (e.g., MAX_WITHDRAWAL_BPS already exists but is not enforced as a per-call cap? Actually it is used, but the issue asks for a percentage cap with timelock; ensure the cap is applied and add a timelock between request and execute). Also, enforce that a recipient can only withdraw fees from markets they are authorized for. Add tests for the new provenance and the security fixes. CI is missing, so please run tests.
closes #172
Track which markets contribute to each fee withdrawal so indexers can detect cross-market contamination. The fees_withdrawn event now includes a Vec with (market_id, amount) pairs in drain order (legacy first, then newest-to-oldest).
Changes:
Existing safeguards (20% cap, 24h timelock, per-market ledger, admin cancellation) remain unchanged.