Skip to content

feat(contract): pull-payment escrow — deposit(), per-member claim(), and claimable-balance accounting with TTL management - #138

Merged
Yunusabdul38 merged 4 commits into
Web3Novalabs:mainfrom
ayinde38:feat/contract-pull-payment-escrow
Aug 19, 2026
Merged

feat(contract): pull-payment escrow — deposit(), per-member claim(), and claimable-balance accounting with TTL management#138
Yunusabdul38 merged 4 commits into
Web3Novalabs:mainfrom
ayinde38:feat/contract-pull-payment-escrow

Conversation

@ayinde38

@ayinde38 ayinde38 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Closes #121

What this adds

A pull-payment escrow path alongside the existing distribute, so a payer funds
one transfer instead of N and no single bad member trustline can abort a
payroll run.

Function Behaviour
deposit(id, from, amount) Pulls amount into the contract in a single transfer, then credits each member their basis-point share
claim(id, member) Member withdraws their full accrued balance
claim_to(id, member, to) Same, paid to a different address (only member authorizes)
claimable_balance(id, member) -> i128 View
total_escrowed(id) -> i128 View

distribute is untouched — escrow is additive, not a replacement, and
test_distribute_still_works_alongside_escrow exercises both paths on one group.

Design notes

Snapshot semantics. A deposit credits the member set as of that deposit.
Balances live under DataKey::Claimable(BytesN<32>, Address) — keyed by address,
not by a position in the member list — so update_members can never retroactively
move already-credited funds. Members removed from a group keep what they accrued
and can still claim it. Documented in the module rustdoc and proven in
test_escrow_credits_survive_a_full_member_replacement, which deposits, swaps the
entire member list, and confirms the original members still claim their exact
amounts while the new members start at zero.

Accounting invariant. Per group, sum(claimable) == total_escrowed. deposit
moves both sides up together and settle_claim moves both down together, so they
cannot diverge. base::escrow::sum_claimable and base::escrow::accounting_holds
are the internal helpers the tests assert on (Soroban cannot enumerate storage
keys, so they take the address set to total). Across groups, the sum of
total_escrowed equals the contract's token balance — asserted in
test_escrow_totals_across_groups_never_exceed_contract_balance.

Dust. Credits reuse base::utils::distribute_amounts, which floors every
share except the last and gives the remainder to the final member, so each deposit
sums exactly. test_escrow_no_dust_leaks_across_100_uneven_deposits runs 100
deposits of 1000 across a 7-member group whose split never divides evenly and
asserts sum(claimable) == sum(deposits) to the stroop, then claims every member
out and asserts the contract's token balance returns to zero.

Reentrancy / write ordering. The state change is complete before the token is
touched: claim_to runs settle_claim — which removes the member's entry and
decrements the group total — and only then calls transfer. By the time the token
contract runs there is nothing left to claim. To make that assertable rather than
merely asserted, the effects half is a separate function;
test_escrow_settle_clears_state_before_any_transfer drives it directly and
checks the entry is cleared and the total decremented while not a single stroop
has moved
, then shows a second attempt at that point returns NothingToClaim.
Worth noting for reviewers: Soroban already rejects reentry into a contract
already on the call stack, so a token genuinely calling back would be rejected by
the host — this ordering is a second line of defence, and it is the reason the
test drives settle_claim directly rather than through a reentrant mock token.

TTL policy and rent cost. Named constants in base::escrow:
ESCROW_TTL_THRESHOLD = 30 days (518_400 ledgers) and ESCROW_TTL_EXTEND_TO =
120 days (2_073_600 ledgers), both derived from LEDGERS_PER_DAY = 17_280. Every
escrow write extends the entry it touched, the group record the claim path has to
read, and the contract's own instance entry — an archived instance means the
contract cannot be invoked at all, so live balances behind a dead instance are
still unreachable. The rent is charged to whoever triggers the bump: the depositor
pays one bump per credited member plus the group total, group record and instance;
the claimer pays for the group total, group record and instance. The 30-day
threshold means an active group re-bumps about monthly rather than on every call,
while an idle group still leaves any member a four-month window to come back and
claim without an external RestoreFootprint.

New error. NothingToClaim = 16, appended so no existing discriminant moves
(12–15 went to the upgradeability work that landed on main while this was open).
A current member with no balance (including a second claim after a successful one)
gets NothingToClaim; an address that holds no balance and is not a member gets
MemberNotFound. deposit on a memberless group returns EmptyMembers before
any transfer, so the contract never takes custody of tokens nobody could claim.

Events. escrow_deposited(id, from, amount) and
escrow_claimed(id, member, to, amount).

Tests

20 new tests in contract/src/test.rs, covering every acceptance criterion:
zero-balance and double-claim → NothingToClaim; non-member → MemberNotFound
with nothing moved; memberless deposit → EmptyMembers with zero custody; the
100-deposit dust check; a seeded-LCG fuzz run of 120 random deposit/claim steps
asserting the invariant, the group total and the contract's token balance after
every step; the member-swap snapshot test; the write-ordering test; and a TTL
test that advances the ledger 60 days past the 4_096-ledger default with
env.ledger().with_mut(...) and then claims successfully.

main does not build — fixed here, please read

While this was open, main gained the upgradeability, SEP-10 and pause work, and
main's own Contract CI is red: the crate does not compile there. This branch
cannot go green without fixing that, so the second commit does, and each fix is
something main's own tests already ask for:

  • base::utils::calculate_share was half-converted to a checked form. It
    referenced an undefined product, used ? in a function returning i128,
    and MAX_SAFE_TOTAL — which test.rs and prop_tests.rs both import — was
    never added. Completed as the tests describe.
  • base::auth::validate_percentages still summed with += and trapped on
    overflow, which is exactly what main's own
    test_regression_validate_percentages_overflow_safe exists to catch. It uses
    checked_add now.
  • Two tests passed the token module where token_address was meant.
  • Thirteen inline test setups never called init, so once the crate
    compiled every create in them failed with MigrationRequired — the
    migration gate landed without those setups being updated.
  • test_distribute_large_amount asserted an error while distributing 1000
    against a minted balance of 1e18. That only held because its group was never
    created; with the setup fixed it cannot pass either way. It now distributes
    the large amount it mints and asserts the even split, which is what its name
    and setup describe. If that is not the intent, say so and I will change it.

Left alone deliberately: get_calculated_share still returns i128 and
aborts on overflow, as its trait signature and doc comment say. A test comment
on main argues for a typed error there, but widening the return type is an ABI
change and belongs to whoever owns that refactor.

Verification

Run locally in contract/ on the merged tree, all green:

  • cargo fmt --all -- --check
  • cargo clippy --all --all-targets -- -D warnings
  • cargo build
  • cargo test81 passed, 0 failed (60 from main + 21 escrow tests)
  • cargo test --doc
  • cargo build --target wasm32v1-none --release (the Makefile's build target)

Not covered

  • The TTL test has to keep the payment token's own entries alive across the
    ledger jump, because this contract can only extend entries it owns. It does that
    with a small SacDataKey mirror of the Stellar asset contract's Balance(Address)
    key layout, which relies on Soroban encoding #[contracttype] enum keys by
    variant name. That is test scaffolding standing in for the traffic that keeps a
    real asset contract live, not production code — but it is a coupling to SAC
    internals that a reviewer should be aware of.
  • Nothing was deployed or exercised against testnet; all verification is the local
    suite above.
  • test_snapshots/ is gitignored in this repo, so the regenerated snapshots are
    deliberately left out of the diff.

ayinde38 and others added 2 commits August 18, 2026 19:00
…agement

`distribute` pushes tokens to every member inside one invocation, so the payer
funds N cross-contract transfers, must be online with the full balance at that
exact moment, and a single unusable member trustline aborts the whole payroll
run. This adds the pull-payment counterpart alongside it.

- `deposit(id, from, amount)` takes custody in a single transfer and credits
  each member using the same basis-point floor-plus-dust math as `distribute`.
- `claim(id, member)` / `claim_to(id, member, to)` pay one member out and clear
  their entry; `claimable_balance` and `total_escrowed` expose the books.
- Credits are a snapshot of the member set at deposit time: balances are keyed
  by `(group id, member address)`, so replacing a group's members never moves
  funds that were already credited.
- Every write extends the entry it touched, the group record and the contract
  instance under named TTL constants, so a member claiming months later does
  not find an archived entry.
- `settle_claim` is split out of `claim_to` so the effects-before-interaction
  ordering is directly assertable in a test.

`distribute` is untouched — escrow is additive.
@Yunusabdul38

Copy link
Copy Markdown
Contributor

@ayinde38 ci check is failling kindly resolve it

The merge of main into this branch was taken automatically and left the crate
uncompilable. Resolving it properly turned up that `main` itself does not build
— its own Contract CI run is red for the same reasons — so the escrow work
cannot go green without fixing them.

Merge resolution:

- errors.rs — main took discriminants 12 through 15 for the upgradeability
  work, so `NothingToClaim` moves to 16. Discriminants are ABI; nothing already
  deployed may be renumbered.
- lib.rs — rebuilt on main's version with the five escrow methods appended, so
  `upgrade`, `migrate`, `pause` and the escrow entrypoints all survive.
- types.rs, events.rs, interfaces/autoshare.rs, test.rs — both sides kept.

Pre-existing breakage on main, fixed here because it blocks CI:

- `base::utils::calculate_share` was half-converted to a checked form: it
  referenced an undefined `product`, used `?` in a function returning `i128`,
  and `MAX_SAFE_TOTAL` — which main's tests and prop tests both import — was
  never added. Completed the conversion the tests describe.
- `base::auth::validate_percentages` still summed with `+=` and trapped on
  overflow, which is exactly what main's own
  `test_regression_validate_percentages_overflow_safe` was written to catch.
  It uses `checked_add` now.
- Two tests passed the `token` module where `token_address` was meant.
- Thirteen inline test setups never called `init`, so every `create` in them
  failed with `MigrationRequired` once the crate compiled — the migration gate
  landed without them being updated.
- `test_distribute_large_amount` asserted an error while distributing 1000
  against a minted balance of 1e18. That only held because its group was never
  created; with the setup fixed it cannot pass either way, so it now
  distributes the large amount it mints and asserts the even split, which is
  what its name and setup describe.

`get_calculated_share` is left returning `i128` and aborting on overflow, as
its trait signature and doc comment still say. A test comment on main argues
for a typed error there, but widening it is an ABI change and belongs to
whoever owns that refactor.

Verified locally: cargo fmt --check, clippy -D warnings, build, 81 tests
passing, doc tests, and the wasm32v1-none release build.
The build job runs `stellar contract build` with a pinned stellar-cli v21.4.0,
which emits to target/wasm32-unknown-unknown/release, and then verifies
target/wasm32v1-none/release/*.wasm — a path only newer CLIs produce. The step
could never find the artifact, so the job failed on `ls`.

Building through cargo with the same target the Makefile's `build` target
already uses makes the output path correct by construction and removes the
dependency on the pinned CLI version.

This only became visible once the crate compiled again; before that the job
failed earlier, at clippy.

@Yunusabdul38 Yunusabdul38 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK
Thanks for ur continuous contribution on paymesh

@Yunusabdul38
Yunusabdul38 merged commit 7be5e40 into Web3Novalabs:main Aug 19, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(contract): pull-payment escrow — deposit(), per-member claim(), and claimable-balance accounting with TTL management

2 participants