Skip to content

🟡 Fee-split distribution edge cases: zero-share recipients, self-payment, and dust griefing #1024

Description

@Ejirowebfi

Area: Smart contract · lib.rs (distribute_fee, set_fee_split)

Description

distribute_fee iterates split recipients computing amount * bps / 10_000 per recipient. Several
edge cases are unhandled:

  1. Small-fee starvation: for small amount (e.g. a 100-stroop fee with a 50-recipient split),
    every share floors to 0 and the entire fee lands in treasury as "remainder" — silently
    defeating the configured split. Recipients entitled to 99% of fees can receive exactly nothing
    forever if fees are set low.
  2. Recipient set is unbounded and unvalidated: set_fee_split accepts any number of recipients
    (each adding a cross-contract transfer call to every fee-charging user transaction, inflating
    every user's gas and pushing invocations toward resource limits) and does not reject bps == 0
    entries, the treasury/payer themselves, or duplicate-of-treasury entries.
  3. Recipient-induced failure: the split executes inside user transactions; a recipient address
    that cannot receive the fee token (e.g. a contract that traps in transfer, or a frozen
    trustline-style condition) makes every create_token/mint_tokens/set_metadata in the
    factory fail until the admin notices and resets the split — a griefing lever handed to any split
    recipient. Iteration order over a Soroban Map also makes remainder attribution
    implementation-defined.

Tasks

  • Cap the number of split recipients (e.g. ≤ 10) and reject bps == 0 entries in
    set_fee_split; document the cap in the ABI.
  • Implement largest-remainder (or equivalent) share allocation so the configured proportions hold
    for small amounts, with deterministic remainder assignment; document rounding behavior.
  • Evaluate pull-over-push: accrue shares in contract storage and let recipients claim_fees(),
    so a broken recipient can never block user transactions; if push is kept, isolate per-recipient
    transfer failure so one bad recipient doesn't fail the whole call (and document the trade-off).
  • Add events for split configuration changes and (if pull model) claims.
  • Tests: dust amounts across recipient counts (property test: sum of transfers == charged fee,
    each recipient's long-run share converges to its bps), recipient-cap enforcement, zero-bps
    rejection, and a trapping-recipient scenario proving user transactions still succeed.
  • Extend fuzz_fee_arithmetic with multi-recipient split configurations.

Acceptance criteria

  • No configuration exists where a recipient with non-zero bps receives zero over repeated
    representative fee amounts, proven by property tests.
  • A malfunctioning split recipient cannot cause unrelated user transactions to fail.
  • Split size and entry validity are enforced at configuration time with documented limits.


Issue 20 of 20 from the codebase audit tracked in ISSUES.md.

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26auditFrom the ISSUES.md codebase auditreleasedseverity: elevatedCorrectness/robustness gap with user-visible impact

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions