feat(contracts): protocol fee engine with multi-party payout splits & treasury accounting

[balisdev]

Summary

Closes #39.

Adds a configurable, governance-bounded fee engine to escrow that splits every worker-paying settlement across worker, protocol treasury, and an optional referrer in exact integer arithmetic, with per-token treasury accounting and provable no-dust/no-loss invariants.

• FeeConfig { protocol_bps, referrer_bps }, governance-bounded by a hard-coded MAX_TOTAL_FEE_BPS (1,500 = 15%) that set_fee_config enforces unconditionally — no governance signer can push the combined rate above it, so the worker always keeps at least 85% of a settled appointment. No FeeConfig entry is written at initialize; get_fee_config treats an absent entry as {0, 0} (no fees), so a contract that never calls set_fee_config has byte-for-byte unchanged instance storage.
• Applied to confirm_completion and the worker-favoring branch of resolve_dispute. cancel_appointment and the refund-to-client branch of resolve_dispute are deliberately unchanged — they still return the full amount with no fee, since the client is being refunded for work that was never delivered. This asymmetry is documented in the module docs, README, and CHANGELOG rather than left incidental, per the issue's acceptance criteria.
• Deterministic, overflow-safe rounding: each of the protocol and referrer shares floor-rounds independently via a split-multiply identity (q*bps + floor(r*bps/D)) that never lets amount * bps overflow i128, even for i128::MAX-adjacent amounts. The worker absorbs the rounding remainder, guaranteeing worker_share + protocol_share + referrer_share == amount exactly for every input — no path can pay out more than was escrowed, and no dust is ever stranded.
• Per-token treasury accounting: the protocol share is credited to a new DataKey::Treasury(token) balance and stays in the contract's own token balance until a governance signer calls the new withdraw_treasury.
• Optional referrer: a new referrer: Option<Address> field on Appointment and final parameter on create_appointment. Paid directly when set; contributes nothing to the common case of no referrer. settlement-router's mirrored escrow::Appointment type gained the same field to keep cross-contract decoding in lockstep (its doc comment already calls out that every field must match).
• New entrypoints: set_fee_config, get_fee_config, get_treasury_balance, withdraw_treasury — all gated by governance::require_signer (any single current signer), the same authority migrate answers to.
• New errors FeeExceedsMaximum, ArithmeticOverflow, InsufficientTreasuryBalance (codes 42-44), appended so no existing error code moved.
• CI cargo-dependency caching was already in place from a prior PR (Swatinem/rust-cache with cache-on-failure in soroban-ci.yml) — no change needed there.

Testing

• 24 new tests covering: fee-config get/set/validation (including exact-cap and above-cap rejection, and non-signer/admin-arbiter rejection), the three-way split with and without a referrer, cancellation and refund-to-client fee-freedom, dispute-resolved-to-worker fee application, treasury withdrawal (success, insufficient balance, non-signer), a 1-unit amount, an i128::MAX-adjacent amount, zero-fee and max-fee configs, and a property/invariant test asserting sum(shares) == amount across a spread of adversarial amounts (including i128::MAX) crossed with fee configs at the edges of what's allowed.
• cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace (all 7 crates — 310 tests total, 0 failures), and cargo build --workspace --release --target wasm32v1-none all pass locally, matching soroban-ci.yml exactly.

Test plan

• cargo fmt --check
• cargo clippy --workspace --all-targets -- -D warnings
• cargo test --workspace (escrow: 80 passed; settlement-router: 14 passed; full workspace: 310 passed, 0 failed)
• cargo build --workspace --release --target wasm32v1-none (optimized WASM build)

[meshackyaro]

Solid PR — well-scoped and the invariant work is the standout part here.

Strengths

• The split-multiply rounding identity (q*bps + floor(r*bps/D)) avoiding i128 overflow while guaranteeing worker_share + protocol_share + referrer_share == amount exactly is the right way to handle this, and having a property test sweep adversarial amounts (including i128::MAX) against edge-case fee configs rather than just spot-checking a few numbers gives real confidence in that guarantee.
• MAX_TOTAL_FEE_BPS enforced unconditionally in set_fee_config, with no governance path around it, is the correct place to put that ceiling — worker protection shouldn't depend on well-behaved governance.
• Treating an absent FeeConfig as {0, 0} so contracts that never call set_fee_config see byte-for-byte unchanged storage is a nice backward-compatibility detail, and documenting the fee/no-fee asymmetry between completion and refund paths (rather than leaving it implicit) matches the issue's intent well.
• Keeping settlement-router's mirrored Appointment type in lockstep with the new referrer field, and appending new error codes without moving existing ones, shows care for the cross-contract and ABI surface.
• Test breadth is good — cap boundary tests (exact-cap accepted, above-cap rejected), non-signer rejection, 1-unit and near-i128::MAX amounts, and treasury withdrawal failure paths are exactly the cases I'd want covered.

Questions / possible follow-ups

• withdraw_treasury — does it withdraw the full treasury balance for a token in one call, or a specified amount? Worth confirming partial withdrawals (if supported) update Treasury(token) correctly and can't be raced against a concurrent fee credit in the same ledger.
• Any current signer can call set_fee_config and withdraw_treasury via require_signer — same as migrate. Given fee changes and treasury withdrawals are more routine/frequent operations than migration, is single-signer authority intentional here, or would these eventually want a higher bar (multisig threshold, timelock)?
• For the referrer path, is there any validation that referrer != worker and referrer != client, or is that left to the caller? Doesn't need to block this PR, but worth a comment either way.
• Is get_treasury_balance per-token only, or is there a way to enumerate all tokens with a nonzero treasury balance? Might matter for tooling/ops around withdraw_treasury.

Nice work — this is a well-tested, carefully-bounded change. Pending answers above, looks mergeable.

[balisdev]

Thanks for the review! Answering in order:

withdraw_treasury — full or partial? It takes a caller-specified amount, so partial withdrawals are supported. Treasury(token) is decremented by exactly that amount (checks the result stays >= 0, erroring InsufficientTreasuryBalance otherwise) before the token transfer — checks-effects-interactions, matching the rest of the contract. On the race concern: Soroban contract invocations are sequential per-ledger (no interleaved storage access within a single contract the way concurrent threads could race), and effects-before-interactions here means even a malicious to re-entering during its own transfer callback would see the already-decremented balance, so a double-spend isn't reachable.

Single-signer authority for set_fee_config/withdraw_treasury — intentional? Yes. It mirrors migrate's precedent in this contract (single-signer for operational actions vs. the M-of-N threshold gate on propose_upgrade/approve_upgrade, which changes contract code). The blast radius is bounded on both ends: set_fee_config can't be pushed past MAX_TOTAL_FEE_BPS (15%) no matter who calls it, and withdraw_treasury only ever moves already-earned protocol fees, never escrowed client/worker funds. A single compromised signer could misdirect treasury revenue, which is bad, but can't touch anyone's escrow. Agreed it's a reasonable candidate for a threshold-gated variant later if treasury size grows large enough to justify it — happy to track that as a follow-up issue rather than block this PR on it.

Referrer == worker/client validation — good catch, added in the latest commit (86be9a3). Wasn't exploitable (the sum == amount invariant holds regardless of who referrer is), but it's a meaningless self-referral state that's cheap to reject outright in create_appointment rather than let onto the chain. New InvalidReferrer error (code 45), two new tests, and README/CHANGELOG updated.

Treasury enumeration across tokens — get_treasury_balance is per-token by design; there's no on-chain list of tokens with a nonzero balance. Deliberate, not an oversight: an on-chain Vec<Address> of every token ever used would grow unbounded and its read footprint would eventually tax every entrypoint that touches it. The intended path for ops tooling is off-chain — index create_appointment's token argument (or the eventual protocol-share-credit path) from transaction history to get the distinct set of tokens, then poll get_treasury_balance per token. Can add that as a note in the README if useful.

Pushed the referrer-validation fix; CI is green. Let me know if any of the above needs code changes rather than just an answer.

[meshackyaro]

This PR reflects care that goes beyond just making tests pass — the split-multiply rounding identity, the checks-effects-interactions ordering on withdraw_treasury, and the deliberate choice to keep treasury enumeration off-chain all show someone thinking about failure modes before they're asked to. The follow-up answers were as rigorous as the PR itself — especially walking through why the reentrancy concern doesn't apply given Soroban's execution model, rather than just asserting it's fine. Approving.