fix(settlement): enforce MAX_FEE_BPS ceiling independent of governance config (#521) - #589
Merged
therealjhay merged 4 commits intoAug 19, 2026
Merged
Conversation
…e config (Betta-Pay#521) bettapay_common::constants::MAX_FEE_BPS (5000 bps / 50%) was intended for settlement adoption per its doc comment, but settlement only capped each fee at BPS_DENOMINATOR (10000 bps / 100%). The tighter 50% ceiling was enforced solely through validate_fee_against_governance, which is a no-op until a governance FeeConfig exists - so before governance is configured, set_settlement_rule and set_default_rule would accept a per-fee value up to 100%, twice the intended ceiling. Add an explicit MAX_FEE_BPS check to both settlement.rs's public set_settlement_rule/set_default_rule entry points and admin.rs's internal _set_settlement_rule/_set_default_rule (used by the scheduled-operation execution path), so the ceiling holds regardless of whether governance has configured anything yet. Closes Betta-Pay#521 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Kindly fix the merge conflicts in this pull request. |
Merging main left duplicate SettlementError discriminants (InvalidFeeBps defined twice, MerchantMissing colliding with Paused) and admin.rs calling the events:: module/AdminTransferred without importing them, breaking the CI build. Closes Betta-Pay#521
Contributor
Author
|
Ci failure fixed @therealjhay |
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 #521
Problem
bettapay_common::constants::MAX_FEE_BPS = 5_000(50%) was documented as intended for settlement adoption, but settlement's setters only checked againstBPS_DENOMINATOR(10_000 / 100%):The tighter 50% ceiling was only ever enforced by
validate_fee_against_governance, which calls into the governance contract'sget_fee_configand is a no-op when governance hasn't configured aFeeConfigyet:So before governance is configured,
set_settlement_rule/set_default_rulewould accept a per-fee value up to 100% — twice the intended 50% ceiling — and the two contracts enforced inconsistent fee ceilings.Fix
Added an explicit
MAX_FEE_BPScheck alongside the existingMIN_FEE_BPS/BPS_DENOMINATORchecks in:settlement_contract/src/settlement.rs— the publicset_settlement_ruleandset_default_ruleentry points.settlement_contract/src/admin.rs— the internal_set_settlement_rule/_set_default_ruleused by the scheduled-operation execution path (these had the same gap and no governance validation at all).Both now reject any per-fee value above
MAX_FEE_BPS, independent of whether a governanceFeeConfigexists.validate_fee_against_governancestill tightens the ceiling further once governance does configure one — it's a floor for the ceiling, not the source of it.Also updated the
MAX_FEE_BPSandSettlementError::InvalidFeeBpsdoc comments, which previously described the pre-fix behavior.Test plan
settlement_contract/src/tests/admin_tests.rs:set_settlement_rule_rejects_platform_fee_above_max_fee_bpsset_settlement_rule_rejects_network_fee_above_max_fee_bpsset_settlement_rule_accepts_fee_at_max_fee_bps_ceiling(5000 bps still valid)set_default_rule_rejects_fee_above_max_fee_bpscargo test --workspacepasses (33 governance + 20 settlement tests)cargo build --workspace --target wasm32-unknown-unknown --releasesucceeds for both contracts