Skip to content

[high] batch_collect_fees reports collected fees but transfers no fundsΒ #259

Description

@Cybermaxi7

🚨 ALL CI CHECKS MUST PASS

Your PR will not be reviewed or merged until every CI job is green. No exceptions.

Run all four locally before you push:

cargo fmt --all -- --check                   # Formatting
cargo clippy --all-targets -- -D warnings    # Clippy β€” warnings are errors
cargo test                                   # Tests
./scripts/build_wasm.sh                      # Build Optimized WASM

A red build is the single most common reason work stalls on this repo. If CI fails and you are stuck, say so in the PR β€” do not push a failing build and go quiet.

Also required: put Closes #<this issue number> in your PR description. Without it, GrantFox cannot link your PR to this issue.


What needs to be done

batch_collect_fees calculates fees across a list of escrows, publishes a BatchFeesCollectedEvent stating the total collected, and returns that total β€” but it never transfers tokens and never updates any stored state. Make it actually move the funds it claims to, and cap how many escrows one call may process.

Why it matters

Any off-chain consumer β€” indexer, dashboard, accounting β€” reads that event and records revenue that does not exist. The contract's own return value confirms the false number. On-chain balances and reported earnings drift apart permanently, with no way to reconcile after the fact. The function is also the only fee path that operates in bulk, so the discrepancy compounds.

Technical context

  • contracts/marketx/src/lib.rs:3656-3721
  • The loop computes fee per escrow with min/max clamping, accumulates total_fees and count, publishes the event, returns the total. There is no transfer and no storage write anywhere in the function.
  • Compare with the working single-escrow fee path for the transfer pattern and the pending-fee bookkeeping this should mirror.
  • escrow_ids: Vec<u64> is unbounded. Existing cap to follow as precedent: MAX_ITEMS_PER_ESCROW (50) in types.rs:145.

Acceptance criteria

  • Fees are transferred to the fee collector; balances change by exactly the reported total
  • Collected escrows are marked so the same fee cannot be collected twice
  • A MAX_ESCROWS_PER_BATCH constant is defined in types.rs and enforced, returning a typed error when exceeded
  • BatchFeesCollectedEvent fires only after a successful transfer, and never on a zero-value call
  • Tests: successful batch transfer, double-collection rejected, over-limit rejected, mixed eligible/ineligible escrows, wrong-token escrows skipped
  • All four CI jobs pass

Out of scope

  • Changing the fee formula or min/max clamping logic
  • The single-escrow fee path
  • Fee collector permissions

Getting started

cargo test              # 120 tests currently pass
cargo clippy --all-targets -- -D warnings
./scripts/build_wasm.sh

Existing fee tests live in src/test.rs β€” search for test_withdrawal_pattern_for_fees.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third CampaignbugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions