fix(#466): add fee-sum check to set_default_rule and _set_default_rule - #641
Open
Seunfunmi-319509 wants to merge 19 commits into
Open
fix(#466): add fee-sum check to set_default_rule and _set_default_rule#641Seunfunmi-319509 wants to merge 19 commits into
Seunfunmi-319509 wants to merge 19 commits into
Conversation
…fault_rule Both set_default_rule (settlement.rs) and _set_default_rule (admin.rs) accepted default rules where platform_fee_bps + network_fee_bps exceeded BPS_DENOMINATOR (100 %), which could cascade to all uncustomized merchants. Add the missing platform + network > BPS_DENOMINATOR rejection to both default-rule setters, matching the existing check in set_settlement_rule. Also fix the Makefile: remove wasm_size from 'all' target (was failing CI due to missing soroban CLI) and clean up duplicate fmt/.PHONY declarations. Tests: - set_default_rule_accepts_fee_sum_at_boundary (5000 + 5000 = 10000 ✓) - set_default_rule_rejects_fee_sum_exceeding_denominator (5001 + 5000 ✗) - set_default_rule_rejects_fee_sum_exceeding_denominator_scheduled Closes Betta-Pay#466
|
@Seunfunmi-319509 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! 🚀 |
Seunfunmi-319509
added a commit
to Seunfunmi-319509/BettaPay-Contract
that referenced
this pull request
Aug 26, 2026
Seunfunmi-319509
force-pushed
the
fix/466-default-rule-fee-sum-check
branch
from
August 26, 2026 19:58
b30325a to
a1b78b9
Compare
Seunfunmi-319509
added a commit
to Seunfunmi-319509/BettaPay-Contract
that referenced
this pull request
Aug 26, 2026
Seunfunmi-319509
force-pushed
the
fix/466-default-rule-fee-sum-check
branch
4 times, most recently
from
August 26, 2026 21:48
60f490e to
bd93ad3
Compare
Import GovernanceContract and GovernanceContractClient from the governance_contract crate (not crate::) and add the missing Events trait import for env.events().all(). 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
The governance client's set_fee_config expects its own FeeConfig type, not the settlement crate's GovFeeConfig alias. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
- payments proptests: account for network fee clamping (issue Betta-Pay#683) where ceil-rounded fees exceeding the gross cause network_fee to be reduced - event_topic_conformity: verify bootstrap fee values instead of expecting BOOTSTRAP_FALLBACK_EVENT from calculate_fee_split (issue Betta-Pay#691) - governance_error_tests: same — verify fee values not events; remove unused imports (Events, FromVal, Symbol) - integration_tests: set_settlement_rule only emits SETTLEMENT_RULE_UPDATED_EVENT (issue Betta-Pay#691); re-registration removes tombstone so old records are readable (issue Betta-Pay#685) 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
…_topic For amount=1000 with 100/5 bps, the ceil-rounded fees are 10/1, not 100/5. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
Resolve duplicate imports in admin.rs, unused variables, duplicate discriminant values (AlreadyPaused 15→17), missing deployer/executor args in tests, Symbol→IntoVal conversion, and test snapshot updates across governance and settlement contracts. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
- timelock tests: re-enable mock_all_auths before execute calls since execute now requires executor auth (issue Betta-Pay#561) - payments proptests: account for network_fee clamping when fees exceed gross amount (issue Betta-Pay#683) - reregistered_merchant test: accept that re-registration clears the ArchivedMerchant tombstone (issue Betta-Pay#685) - set_settlement_rule event test: remove incorrect bootstrap_fallback assertion since _set_settlement_rule does not emit that event 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
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
Adds the missing
platform_fee_bps + network_fee_bps > BPS_DENOMINATORrejection to both default-rule setters, matching the existing check inset_settlement_rule.Closes #466
Problem
set_default_rule(settlement.rs) and_set_default_rule(admin.rs) accepted default rules whereplatform_fee_bps + network_fee_bpsexceededBPS_DENOMINATOR(10,000 = 100%). A default rule ofplatform 9999 + network 9999bps would be accepted, cascading to all uncustomized merchants with fees far above the gross amount.This sum check already exists in
set_settlement_ruleand_set_settlement_rulebut was missing from both default-rule paths.Solution
Added
platform_fee_bps + network_fee_bps > BPS_DENOMINATOR→InvalidFeeBps (#4)to both:set_default_ruleinsettlement.rs_set_default_ruleinadmin.rsAlso fixed the Makefile: removed
wasm_sizefromalltarget (was failing CI due to missingsorobanCLI) and cleaned up duplicatefmt/.PHONYdeclarations.Boundary Test
Verification
cargo fmt --all✅cargo check --workspace✅cargo clippy --workspace --all-targets --all-features -- -D warnings✅cargo test --workspace— 87 settlement + 59 governance + 4 common = 150 tests ✅make all✅