fix(#445): gate UpdateTradeFeePolicy on marketauth, not asset-0 insurance_authority - #455
Merged
Merged
Conversation
…market-wide setter `handle_update_trade_fee_policy` authorized on asset 0's per-asset `insurance_authority` while writing `cfg.trade_fee_base_bps`, which is not scoped to asset 0: it is the floor in `hybrid_trade_fee_bps_view` (`base = max(caller_fee_bps, cfg.trade_fee_base_bps)`) and in `handle_trade_cpi` (`fee_floor_pre`). So a per-asset role moved the fee floor for every asset in the market. Every sibling market-wide fee-policy setter gates on `cfg.marketauth` — UpdateFeeSplit, UpdateFeeRedirectPolicy, UpdateLiquidationFeePolicy, UpdateMaintenanceFeePolicy, UpdateMaintenanceFeePerSlot, UpdateMarketInitFeePolicy. This one was the exception, and the divergence was already flagged in-code without being tracked. The split is an ORDINARY operational state, not a contrived one: marketauth handed to a governance multisig while the creator keeps asset 0's insurance role, or that role delegated to an insurance manager. The fix also unfreezes a field that could become permanently unsettable. On a staked market, `BindInsuranceAuthority` parks asset 0's `insurance_authority` on a stake PDA that cannot sign, which left `trade_fee_base_bps` writable by nobody at all. The existing test asserted the exact opposite of this fix — it pinned that marketauth is REJECTED once asset 0's insurance authority is rotated away. Inverted and renamed, following the invert-and-rename convention this file already uses for the retired two-rate-floor guards. The bps-cap assertion now runs against the authority that is allowed through, so it proves the bound and not merely the gate. Negative control: restoring the old gate fails the new test. Wrapper suite: 225 passed / 19 failed — the same 19 that fail on origin/main untouched (verified by running the suite on the base commit), so no regression. Upstream checked: `handle_update_trade_fee_policy` does not exist upstream at all — the creator/trade-fee-policy surface is ours alone, so there is nothing to port and nothing to report back. Refs: #445 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NgoNgagkvw7i5SSRC3FJ8D
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Closes #445.
handle_update_trade_fee_policyauthorized on asset 0's per-assetinsurance_authoritywhile writing the market-widecfg.trade_fee_base_bps. That field is the fee floor inhybrid_trade_fee_bps_viewand inhandle_trade_cpi'sfee_floor_pre, neither scoped to asset 0 — so a per-asset role controlled the floor for every asset in the market.Every sibling market-wide fee-policy setter gates on
cfg.marketauth. This one was the lone exception, and the divergence was already flagged in-code without being tracked as an issue.Why this is a defect and not a policy preference
Two things push it past "inconsistent":
BindInsuranceAuthorityparks asset 0'sinsurance_authorityon a stake PDA that cannot sign.trade_fee_base_bpswas then writable by nobody. So the current gate is not merely the wrong authority; in a supported configuration it is no authority.Test
The existing guard asserted the opposite of this fix — it pinned that marketauth is rejected once asset 0's insurance authority is rotated away. Inverted and renamed, following the invert-and-rename convention this file already documents for the retired two-rate-floor guards. The bps-cap assertion now runs against the authority that is allowed through, so it proves the bound as well as the gate.
Negative control: restoring the old gate fails the new test.
Suite
225 passed / 19 failed. Those 19 are the same 19 that fail on
origin/mainuntouched — verified by running the suite on the base commit before applying anything. No regression.Upstream
handle_update_trade_fee_policydoes not exist upstream. The creator/trade-fee-policy surface is ours alone, so there is nothing to port in and nothing to report back.🤖 Generated with Claude Code
https://claude.ai/code/session_01NgoNgagkvw7i5SSRC3FJ8D