Validate InvoiceOptions ranges at invoice creation (#690–#693) - #715
Merged
Kingsman-99 merged 1 commit intoAug 30, 2026
Conversation
…Stellar-split#691, Stellar-split#692, Stellar-split#693) Add creation-time validation to `_create_invoice_inner`, before any storage is written: * Stellar-split#690 penalty_bps: reject `penalty_bps > 10_000` with `ContractError::InvalidAmount` (via `assert_valid_bps`), so a late-payment penalty can never exceed 100% of the payment. * Stellar-split#691 tranches: when `tranches` is non-empty, its `basis_points` must sum to exactly 10_000, else `ContractError::InvalidRatioSum`. Empty schedule is skipped. * Stellar-split#692 release_stages: when non-empty, entries must sum to exactly 10_000, else `ContractError::InvalidRatioSum`. Empty schedule is skipped. * Stellar-split#693 required_signatures: reject `required_signatures > co_signers.len()` and `required_signatures == 0` with non-empty `co_signers`, both with `ContractError::InvalidAmount`, so the multi-sig release gate is always satisfiable. The tranche / release-stage checks already existed but panicked with a host string via `.expect()`; they now surface the proper `ContractError` so `try_create_invoice` callers get a typed error. Tests: add unit tests for each boundary (10_001 vs 10_000; sum 9_999 vs 10_000; sum 9_500 vs three stages summing 10_000; 2-of-1 and 0-of-2 rejected vs valid 2-of-3). Existing `test_create_invoice_invalid_release_stages_panics` updated to assert the typed error. Also repaired pre-existing compile blockers on `main` that sit directly on the creation/validation path (unrelated to these issues, noted in the PR): duplicate `mod validation;`, missing `validation::assert_valid_bps`, a spliced/duplicated `invoice_expired` with an unclosed delimiter in events.rs, and a duplicate import line in test.rs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GPTytjBS9tuuaywXrcQwRN
|
@gideonpius7-design 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! 🚀 |
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
2 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.
What
Adds creation-time validation of
InvoiceOptionsnumeric ranges in_create_invoice_inner, before any storage is written.Closes #690
Closes #691
Closes #692
Closes #693
penalty_bps > 10_000ContractError::InvalidAmounttranches:sum(basis_points) != 10_000ContractError::InvalidRatioSumrelease_stages:sum != 10_000ContractError::InvalidRatioSumrequired_signatures > co_signers.len(), or== 0with non-emptyco_signersContractError::InvalidAmountEmpty
tranches/release_stagesare skipped (release-all-at-once stays valid).The tranche and release-stage sum checks already existed but panicked with a
host string via
.expect(). They now surface the properContractError, sotry_create_invoicecallers receive a typed error instead of an opaqueInvokeError. A newvalidation::assert_valid_bpshelper backs thepenalty_bpscheck.Tests
New unit tests in
contracts/split/src/test.rs:penalty_bps= 10_001 rejected / = 10_000 acceptedtranchessum 9_999 rejected / 10_000 acceptedrelease_stagessum 9_500 rejected / three stages summing 10_000 acceptedrequired_signatures2-of-1 rejected, 0-of-2 rejected, valid 2-of-3 acceptedtest_create_invoice_invalid_release_stages_panicsupdated to assert thetyped
InvalidRatioSumerror viatry_create_invoice.main(not caused by this PR)cargo test --workspace/cargo test -p splitdoes not compile onmaintoday — CI has been red for a while. This PR fixes the four blockers that
sit directly on the invoice-creation / validation path so the changes here
are coherent:
mod validation;inlib.rsuse validation::assert_valid_bps;referencing a function that did not existinvoice_expiredinevents.rswith an uncloseddelimiter (botched merge)
test.rsRemaining unrelated errors still block a green build (~20+ across
lib.rs,types.rs,storage.rs,events.rs): soroban-sdk API drift(
Persistent::bump,Map::has,Vec::to_vec), a missingcalc_platform_feefn, a missing
ContractError::TooFewRecipientsvariant, non-exhaustivematches on
InvoiceStatus::PayoutInProgress, and.expect()on non-Optiongetters in
test.rs. These are out of scope for #690–#693 and should beaddressed by a dedicated repair PR; until then
cargo testcannot be run togreen and the new tests here are unverified by execution (logic is
straightforward range checks).
🤖 Generated with Claude Code