feat: add per-transaction amount cap policy denominated in XLM - #388
Merged
davedumto merged 1 commit intoAug 31, 2026
Merged
Conversation
Implements the A2 safety rule: reject any single classified transfer whose amount exceeds a cap configured at construction time in stroops. The cap is denominated in native units only. A Soroban contract has no trustless price feed, so a fiat-denominated rule could only be enforced by trusting an oracle, which would make the guarantee only as strong as that oracle. No USD option is provided. This rule is deliberately distinct from the existing cumulative rolling-window allowance in contracts/policy-templates/spending-limit. It holds no spend state and has no time dimension: each transfer is judged individually, so three at-cap transfers in a row all pass. Bounding total outflow remains the cumulative policy's job, and the docs state that the two are complementary rather than substitutes. Enforcement is deny-by-default. Only a well-formed SEP-41 transfer with a positive i128 amount is eligible to pass the cap check; a non-transfer call, a missing or wrongly-typed amount, a non-contract context, a call targeting the wallet's own admin surface, an empty context list, and a context list longer than the evaluation bound are all rejected rather than allowed. Configuration is immutable after construction and each instance is bound to one wallet. The cap check is a pure comparison on range-checked values, so there is no accumulation and no overflow path; a cap of i128::MAX against an amount of i128::MAX is covered by a test, and the release profile keeps overflow-checks enabled as defence in depth. Tests cover the four required cases (under the cap, exactly at the cap, over the cap, and an unclassifiable interaction rejected rather than allowed) plus the per-transfer versus cumulative distinction, boundary amounts, and the deny-by-default and wallet-binding paths. 25 tests pass.
|
@noevidence1017 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! 🚀 |
|
@noevidence1017 is attempting to deploy a commit to the david's projects Team on Vercel. A member of the Team first needs to authorize it. |
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.
Summary
Implements the A2 safety rule: reject any single classified transfer whose amount exceeds a cap configured at construction time in stroops.
New standalone crate at
contrib/contracts/per-transfer-cap. It is not a member of any Cargo or pnpm workspace, so it cannot affect the existing build.closes #14
closes #349
closes #351
Design
Denominated in stroops, never fiat. The cap follows the existing
contracts/policy-templates/spending-limitprecedent and is a native-unit quantity. There is intentionally no USD option: a Soroban contract has no trustless price feed, so a fiat rule could only be enforced by trusting an oracle, which would make the policy's guarantee only as strong as that oracle and would let a stale or manipulated price move the effective limit. As the issue directs, fiat denomination is left as a separate design discussion rather than an oracle bolted into this rule.Per transfer, not a running total. This is deliberately a different rule from the cumulative rolling-window allowance, and the module docs, the README, and a dedicated test all make the distinction explicit:
spending-limit(existing)spenttotalWith a cap of 10 XLM this policy permits 10 XLM three times in a row, because each is individually within the cap. Since
Signature::Policycarries no secret, a per-transfer cap used alone does not bound total loss, so the docs state that this rule complements the cumulative policy rather than replacing it.Deny-by-default. Only a well-formed SEP-41
transferwith a positivei128amount is eligible to pass the cap check. A non-transfer function, a missing or non-i128amount, a zero or negative amount, a non-contract context, a call targeting the wallet's own admin surface, an empty context list, and a context list longer thanMAX_CONTEXT_EVALUATION_LIMITare all rejected. The over-long list is refused outright rather than truncated, so no entry can slip past the cap by sitting beyond the evaluated bound.Arithmetic. The cap check is a pure comparison on values that are range-checked before use, so there is no accumulation and therefore no overflow path. A cap of
i128::MAXagainst an amount ofi128::MAXis exercised in the tests, and the release profile keepsoverflow-checks = trueas defence in depth, matching the audited workspace.Configuration. Written once by the constructor and never mutated (no setter, since a cap the holder can raise in place guarantees nothing), and each instance is bound to a single wallet that both
installandpolicy__enforce.Tests
25 tests, all passing via
cargo test. The four cases the issue requires:allows_transfer_under_capallows_transfer_exactly_at_caprejects_transfer_over_caprejects_unclassifiable_interactionPlus the per-transfer versus cumulative distinction (
cap_applies_per_transfer_not_cumulatively,cap_is_not_affected_by_elapsed_time,allows_batch_whose_sum_exceeds_cap_but_each_transfer_does_not),i128::MAXboundary behaviour, the deny-by-default paths, and wallet-binding and install enforcement.prettier --checkpasses for the files CI checks. Generatedtest_snapshots/are not committed:.prettierignoreonly excludescontracts/**/test_snapshots/, notcontrib/, so committing them would failformat:check. This matchescontrib/contracts/safety-policy, which also commits source only.Note on the two other issues assigned to me
I looked into #349 and #351 and could not find the code either one describes on
drips. Rather than invent a change, I am reporting what I found so a maintainer can confirm before anything is removed or deprecated.#351 (deprecate legacy
/v0verification endpoints). There are no/v0routes in the repository.services/verification-service/src/server.tsexposes only unversioned routes:POST /verification/submitGET /verification/:contractIdGET /verification/:contractId/statusA repository-wide search for
/v0returns no matches outsidepnpm-lock.yaml. There is no endpoint to attach a deprecation header to, and nodocs/decisions.mdexists to record a timeline in.#349 (remove dead legacy account handler in wallet-service).
services/wallet-service/src/server.tsregisters six routes, none of which is a legacy account handler, and the service contains no occurrence of "legacy" or "deprecated":POST /wallet/create,POST /wallet/connect,POST /wallet/submitGET /wallet/session/:id,GET /wallet/sessions,DELETE /wallet/session/:idBoth issues also target
services/, which CONTRIBUTING.md andcontrib/README.mdplace outside the contributor sandbox, so per rule 3 they need a maintainer to make the change or to explicitly widen scope. Happy to pick either up if you can point me at the intended code or widen the scope on the issue.Unrelated pre-existing issue
While running the A1 crate to check my work against it,
contrib/contracts/safety-policydoes not compile undercargo test(22 errors: its test file usesAddress::generatewithout importing thesoroban_sdk::testutils::Addresstrait). This is pre-existing ondripsfrom #386 and untouched by this PR, but flagging it since that crate's tests cannot currently run.