[high] Cancellation is never recorded: Cancelled status unused, cancelled counter permanently zero, cancellations booked as refunds

[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

Cancellation is a first-class outcome in this contract — there are three ways to reach it — but it is recorded nowhere. EscrowStatus::Cancelled is never assigned, TotalCancelledCount is never incremented, and mutually-agreed cancellations are booked as refunds. Make cancellation observable in state, in analytics, and in events.

Why it matters

EscrowStatus declares six variants, and Cancelled is one of them. It is part of the public contract ABI: any client, indexer or dashboard written against the interface can legitimately expect to observe it. Nothing in the contract ever sets it.

$ grep -n "EscrowStatus::Cancelled" contracts/marketx/src/lib.rs
(no matches)

Trace what the three cancellation paths actually do:

Path	What happens	Cancelled set?	Counter
accept_cancellation (lib.rs:1613)	calls refund_buyer	❌ sets Refunded	TotalRefundedCount +1
propose_cancellation (lib.rs:1566)	records a proposer only	❌	none
cancel_unfunded (lib.rs:1780)	deletes the escrow record	❌	none

refund_buyer (lib.rs:333-345) is the shared tail of the accept path:

escrow.status = EscrowStatus::Refunded;
escrow.cancellation_proposer = None;
Self::add_i128(env, DataKey::TotalRefundedAmount, escrow.amount);
Self::add_u32(env, DataKey::TotalRefundedCount);

Four consequences, in descending order of how much they hurt:

1. A public getter that can only ever return zero. get_total_cancelled_count() (lib.rs:876) reads DataKey::TotalCancelledCount. That key is initialised to 0 in initialize() and never written again — the only add_u32 call sites in the file are for TotalRefundedCount (twice), TotalReleasedCount and TotalDisputedCount. It is an API that reports a number which is always wrong, in the same family as the misleading event fixed in #246 and #259.

2. analytics_summary() is wrong too. GlobalDisputeAnalytics exposes cancelled_count (types.rs:624), so the aggregate report inherits the permanent zero. failure_rate_bps sits in the same struct, computed from counters that are themselves conflated.

3. Refunds and cancellations are conflated. A dispute-driven refund and a mutually-agreed cancellation are different business events with different implications, and both land in TotalRefundedCount. Anyone using these counters to reason about dispute rates is reading an inflated figure, and there is no way to separate the two after the fact.

4. cancel_unfunded leaves almost no trace. It removes the escrow and its hash index and emits EscrowExpiredEvent, which describes expiry rather than cancellation, and increments nothing. Once the record is gone the event is the only evidence it ever existed.

Separately, and worth fixing in the same pass: TotalReleasedAmount is never initialised. initialize() (lib.rs:478-505) sets seven counters and omits it, even though it is written in six places elsewhere. It survives today only because add_i128 falls back to unwrap_or(0), which means the omission is invisible until someone reads the key directly before the first release.

Technical context

• contracts/marketx/src/lib.rs:333-345 — refund_buyer, the shared tail
• contracts/marketx/src/lib.rs:1566, :1613, :1780 — the three cancellation entry points
• contracts/marketx/src/lib.rs:876 — get_total_cancelled_count
• contracts/marketx/src/lib.rs:478-505 — counter initialisation
• contracts/marketx/src/types.rs:234-241 — EscrowStatus
• contracts/marketx/src/types.rs:619-626 — GlobalDisputeAnalytics
• contracts/marketx/src/events.rs — event definitions; a cancellation event may need adding

A judgement call to make explicitly, and state in the PR: accept_cancellation currently reaches a terminal state through refund_buyer, which both moves the money and books the counters. Splitting cancellation out means either giving it its own tail that transfers funds and records Cancelled, or parameterising refund_buyer with the terminal status to apply. Either is fine; decide and say which and why. Do not duplicate the transfer logic.

Do not silently change existing counter semantics. TotalRefundedCount currently includes cancellations. If your change removes them from that count, say so in the PR — anyone reading those numbers today will see a step change.

Acceptance criteria

• accept_cancellation results in EscrowStatus::Cancelled, not Refunded
• TotalCancelledCount is incremented on every path that cancels an escrow
• get_total_cancelled_count() and analytics_summary().cancelled_count return non-zero after a cancellation
• Cancellation-driven amounts are no longer counted in TotalRefundedCount / TotalRefundedAmount, or the PR explains why they should remain
• cancel_unfunded records the cancellation before removing the escrow, and emits an event that names cancellation rather than expiry
• TotalReleasedAmount is initialised in initialize() alongside its siblings
• Tests: a full propose-then-accept cancellation asserting final status, both counters, and the emitted event; cancel_unfunded asserting the counter and event; a dispute refund asserting it still counts as a refund and not as a cancellation; analytics_summary asserting the fields move independently
• grep -n "EscrowStatus::Cancelled" contracts/marketx/src/lib.rs returns at least one assignment
• All four CI jobs pass

Out of scope

• Adding new cancellation entry points or changing who may cancel
• Reworking failure_rate_bps beyond keeping it consistent with corrected counters
• Backfilling analytics for escrows already settled on-chain — call it out in the PR as a migration consideration, but do not attempt it here
• Renaming or removing any existing public function

Getting started

cargo test    # 120 tests currently pass
grep -n "EscrowStatus::Cancelled" contracts/marketx/src/lib.rs   # currently silent
grep -n "add_u32(" contracts/marketx/src/lib.rs                  # four sites, none for Cancelled

Read refund_buyer first. It is small, and the whole issue follows from what it does and does not record.

[PeterOche]

i am an experienced swe, can i work on this ?

[grantfox-oss]

🦊 GrantFox — @PeterOche has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #269)
2. Your PR will be reviewed by the MarketXpress maintainers

Good luck! Track your progress on GrantFox.

[grantfox-oss]

🎉 This issue has been marked as completed on GrantFox as part of the Third Campaign campaign!

@PeterOche's PR #270 was approved and merged by @Cybermaxi7.

🏆 @PeterOche: You earned 35 FoxPoints for this contribution! Your current tier: Explorer (354 total points). Track your full progress on GrantFox.

👏 Great work, @PeterOche! Keep contributing to MarketXpress.