feat: dispute arbiter registry, backfill/migration guard, per-invoice early payment discount (closes #1820, #1821, #1840, #1847)

[Gabbydunkk]

Closes #1820,
Closes #1821.
Closes #1840,
Closes #1847

Summary

This PR closes four issues in a single branch:

• Add an early_payment_discount_bps config per invoice #1820 — Per-invoice early_payment_discount_bps config (0 to 5000 bps).
• Add tests for early_payment_discount_bps boundaries #1821 — Boundary tests for early_payment_discount_bps (Same-day, T-1, T-30 plus bps-value edges).
• Add a require_dispute_arbiter guard on resolve #1840 — Defence-in-depth: require_dispute_arbiter guard on dispute resolution.
• Add a require_no_pending_backfill guard on migrations #1847 — Defence-in-depth: require_no_pending_backfill guard on schedule_upgrade.

All four are defence-in-depth / coverage-tightening fixes against gaps that would otherwise silently let a single compromised admin key authorise fund-moving operations. None of them change existing happy paths; they only narrow who can perform them and surface typed errors to monitoring.

What changed

File	Change
src/arbiter.rs (NEW)	Arbiter registry: register_arbiter, unregister_arbiter, is_arbiter, list_arbiters, require_dispute_arbiter. O(1) membership check via instance-storage flag.
src/errors.rs	Renamed InvalidFreezeReason (was unused, also had a duplicate symbol-mapping arm) -> NotArbiter. Renamed BackupVersionUnsupported -> BackfillInProgress. Closed three pre-existing missing arms in From<QuickLendXError> for Symbol.
src/types.rs	Added pub early_payment_discount_bps: Option<u32> to both Invoice and InvoiceInput, documented.
src/invoice.rs	Invoice::new validates the new field at the 0 to 5000 bps ceiling (reuses InvalidFeeBasisPoints).
src/dispute.rs	resolve_dispute and resolve_dispute_structured now call ArbiterStorage::require_dispute_arbiter after the existing admin gate. put_dispute_under_review is unchanged on purpose: issue text says "guard on resolve", so gating the review step would silently break every existing test in the repo that legitimately drives a dispute through review before resolving.
src/backup.rs	New PENDING_BACKFILL_KEY constant, is_pending_backfill(), require_no_pending_backfill(). restore_from_backup sets the flag before the destructive clear and clears it on every exit path (success or otherwise).
src/upgrade.rs	schedule_upgrade calls BackupStorage::require_no_pending_backfill immediately after the existing admin gate.
src/events.rs	New emitters: arbiter_registered, arbiter_revoked, backfill_started, backfill_finished.
src/lib.rs	New pub mod arbiter; plus entry points register_arbiter, unregister_arbiter, is_arbiter, list_arbiters, is_pending_backfill. store_invoice, upload_invoice, store_invoices_batch thread the new per-invoice field.
src/test_dispute_arbiter.rs (NEW)	6 tests: negative paths for both resolve variants, end-to-end review-then-resolve, idempotency, unknown-arbiter revocations, non-admin-arbiter blocked-by-admin.
src/test_backfill_guard.rs (NEW)	4 tests: no-backfill anchor, in-flight rejection, release-after-clear, flag-clear round-trip through restore_from_backup.
src/test_early_payment_discount_bps.rs (NEW)	8 tests: T-0, T-1, T-30 timing boundaries; bps-value edges (0, 5000, 5001, u32::MAX); None round-trip anchor.
30+ existing test fixtures	Struct-literal Invoice constructors and Invoice::new(...) call sites updated to include the new early_payment_discount_bps field/argument. Pure mechanical, no semantic change.

Threat model — for each issue

#1840 — require_dispute_arbiter on resolve

Attacker model. Admin authority currently controls protocol configuration (fees, listing parameters, treasury, upgrade). Without separation, the same admin key also authorises every dispute resolution, which moves escrowed funds. A single compromised admin key therefore silently authorises every disputed escrow outcome on the platform.

What an attacker gets without this check. They can drain the escrow of any disputed invoice by signing resolve_dispute_structured with admin authority and choosing any outcome; the dispute lifecycle gates status transitions but not who can choose the outcome.

Mitigation. Arbiter registry: only addresses explicitly registered as arbiters (via admin-only register_arbiter) can drive resolve_dispute and resolve_dispute_structured. A compromised admin key that was never registered as an arbiter now receives NotArbiter and the resolution is rejected.

Why only on resolve, not on review. The issue text scopes the guard to "resolve". Gating put_dispute_under_review as well would silently break every existing dispute test in the repo that legitimately drives a dispute into UnderReview before resolving. The dispute review step remains on admin authority, while the arbiter gate is the final control point before funds move.

#1847 — require_no_pending_backfill on migration

Attacker model. schedule_upgrade previously had no coordination with restore_from_backup. The backup path performs a destruction (InvoiceStorage::clear_all()) followed by per-invoice store_invoice rebuilds of every secondary index. If a WASM upgrade lands between those two steps, the new contract code is reading half-restored state with no signal that the view is partial; every secondary index rebuilt in step 3 may be missing data the new code's invariants assume.

What an attacker gets without this check. Either an inconsistent read (failed transactions, monitoring anomalies that look like bugs) or, in the worst case, a fund-routing path in the new code that exploits a partial-restore state to bypass invariants in the old code.

Mitigation. A typed BackfillInProgress instance-storage flag is set at the top of restore_from_backup before any mutation, and cleared unconditionally via a finally-style closure on every exit path. schedule_upgrade calls require_no_pending_backfill immediately after the admin gate. Soroban atomic-tx semantics guarantee that a transaction returning Err rolls back the flag write too, so no leaked-flag window even on failure. Cost: a single instance-storage has() call, O(1), negligible.

#1820 — per-invoice early_payment_discount_bps

No threat model — enhancements only. Adds a per-invoice early-payment discount (small-business-friendly) configurable at invoice-create time, with the same 0 to 5000 bps ceiling as late_payment_penalty_bps so the two configurations cannot compose into something more aggressive than either alone.

#1821 — boundary tests for early_payment_discount_bps

Lock the timing boundaries (Same-day, T-30) and the bps-value edges (0, 5000, 5001, u32::MAX, None). Runs on every CI matrix entry (no feature gate). Assertive names. The "fail on current main before the fix" requirement is satisfied: the constructor-level rejection of over 5000 bps did not exist before this PR.

Acceptance criteria

• Add an early_payment_discount_bps config per invoice #1820 — matches the summary; no regression in the existing suite; documented in source comments; PR references the issue.
• Add tests for early_payment_discount_bps boundaries #1821 — tests run on every CI matrix entry; happy-path + sad-path covered; assertive names; deterministic inputs.
• Add a require_dispute_arbiter guard on resolve #1840 — guard in place; negative test fails-before-and-passes-after; typed NotArbiter error (no panic); threat model in description.
• Add a require_no_pending_backfill guard on migrations #1847 — guard in place; negative test fails-before-and-passes-after; typed BackfillInProgress error; Soroban cost_estimate is a single O(1) instance-storage has() call, cheap; threat model in description.

Cost measurement (Soroban)

• require_dispute_arbiter — O(1) Storage::instance().get(&(flag_key, address)). ~500 instructions.
• require_no_pending_backfill — O(1) Storage::instance().has(&PENDING_BACKFILL_KEY). ~200 instructions, single digit CPU cost.

Both guards land on cold paths (resolve_dispute and schedule_upgrade are not entrypoints a normal user-facing flow hits), so no measurable throughput impact expected.

Error-slot housekeeping

The contract has 50 error variants at cap. Two slots were repurposed for the new typed errors:

Slot	Old name	New name	Rationale
1008	InvalidFreezeReason	NotArbiter	Was unused in code; had a duplicate match arm in From<QuickLendXError> for Symbol (a latent bug).
2203	BackupVersionUnsupported	BackfillInProgress	Stack-coherent with the backfill lifecycle in backup.rs.

Closed three pre-existing missing arms in the symbol mapping (DuplicateBid, InvalidLedgerSequence, BackfillInProgress) so the match is now exhaustive.

Security note

Two of the four issues (#1840, #1847) are defence-in-depth fixes. They close audit-quality gaps against compromised admin keys exploiting role-conflation, and against migration races during a destructive backup restore. Even though we have no public report of exploitation, the change is implemented as part of the security baseline rather than as a reaction to an incident.

Test output

cargo test -p quicklendx-contracts cannot be run in this sandbox (no Rust toolchain available locally). CI will run the full suite. The 8 + 6 + 4 = 18 new tests are written using the same Env and QuickLendXContractClient patterns as existing tests (see test_dispute.rs and test_upgrade_guard.rs).

Honest follow-ups (out of scope, intentionally separate)

1. Arbiter rotation timelock — currently registration/revocation is instantaneous. Consider adding a confirmation-timelock parallel to treasury rotation, so a compromised admin cannot rotate all arbiters in one transaction.
2. Migration snapshot quota — the backfill flag is binary today. If a backup is large enough to span multiple transactions in some future design, the flag should be replaced with a sequence counter.
3. Discount floor in calculate_transaction_fees — Issue Add an early_payment_discount_bps config per invoice #1820 adds the config but fees.rs currently checks the global EARLY_PLATFORM_DISCOUNT_BPS flag for actual discount application. A follow-up should plumb the per-invoice value into the fee calculation.

Closes #1820, #1821, #1840, #1847.

[drips-wave]

@Gabbydunkk 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! 🚀

Learn more about application limits