Skip to content

fix(dispute-resolution): extend instance TTL from every entry point - #78

Merged
JamesVictor-O merged 2 commits into
Ads-Bazaar:mainfrom
olathedev:fix/59-dispute-instance-ttl
Aug 29, 2026
Merged

fix(dispute-resolution): extend instance TTL from every entry point#78
JamesVictor-O merged 2 commits into
Ads-Bazaar:mainfrom
olathedev:fix/59-dispute-instance-ttl

Conversation

@olathedev

@olathedev olathedev commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Closes #59.

dispute-resolution has no instance-TTL bump anywhere. The Admin, EscrowContract, Version and NextDisputeId keys are written once by initialize via plain env.storage().instance().set(...) and never touched again — storage.rs extends TTL for its two persistent keys (Dispute, OpenDispute) but has no extend_instance_ttl helper at all, and no function in lib.rs ever called env.storage().instance().extend_ttl(...).

This turned out to be sharper than the issue estimates. Instrumenting it in a test, the instance entry's TTL sits at 4095 ledgers immediately after initialize — roughly 5.7 hours at 5s/ledger — and only decays from there. It isn't a slow leak that bites an under-used deployment eventually; a freshly initialized contract is inside a day of archival on day one.

Once archived, get_escrow_contract fails with Error::NotInitialized, which breaks the cross-contract reads (get_campaign_business, freeze_for_dispute) that every raise_dispute call depends on — bricking the contract's only fully implemented mutating entry point until someone submits a RestoreFootprint operation. get_admin fails the same way for assign_arbiter and upgrade.

Fix

  • storage.rs — added extend_instance_ttl, mirroring campaign-escrow's helper of the same name, with a matching INSTANCE_BUMP_LEDGERS / INSTANCE_LIFETIME_THRESHOLD pair (518_400 / 500_000). Kept as separate constants from the persistent pair for the same reason escrow keeps them separate: instance and persistent TTL are tracked independently by the ledger even when the numbers happen to coincide.
  • lib.rs — called from every public #[contractimpl] function: initialize, raise_dispute, assign_arbiter, resolve_dispute, close_dispute, get_dispute, version, upgrade. Also from close_dispute's early-return branch (the no-open-record no-op), since that path is still a real invocation that read instance storage to authorize its caller.

Why the read-only paths bump too

get_dispute and version mutate nothing, so bumping TTL there may look out of place. It's deliberate, and it's the difference between fixing this and half-fixing it: nothing writes to instance storage after initialize. If only the write paths bumped, a deployment being actively read — an indexer polling get_dispute, a client checking version — would still let its config expire, which is exactly the failure mode this issue describes. This follows the precedent already set by campaign-escrow::get_protocol_config, which bumps on a pure read for the same reason. Reads bump after the underlying read succeeds, so version still returns NotInitialized before initialization rather than bumping a nonexistent entry.

Scope notes

Test coverage added

New test_instance_ttl module in dispute-resolution/src/test.rs:

  • initialize_extends_instance_ttl — TTL is the full 518_400 window after initialize, not the 4095 default.
  • raise_dispute_re_extends_decayed_instance_ttl — advances the ledger sequence past the extend threshold, asserts the TTL actually decayed under it (so the bump is genuinely exercised rather than being a no-op), then asserts raise_dispute restores the full window.
  • read_only_version_call_extends_instance_ttl — same decay-then-restore, through a pure read, covering the case argued above.

All three were verified to fail against the pre-fix code with an observed TTL of 4095 vs. an expected 518400 — they're regression guards, not assertions that pass either way.

Test plan

  • cargo test --workspace — 147 pass (98 escrow + 18 integration + 31 dispute-resolution; dispute-resolution up from 28)
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo build --workspace --target wasm32v1-none --release — builds
  • Confirmed the three new tests fail without the fix

Second commit: cargo fmt on campaign-escrow (unrelated to the fix)

cargo fmt --all -- --check has been failing on main since 43ba011, 7c41d08 and 748ba2f landed on 2026-08-25 — four hunks, all in campaign-escrow (src/lib.rs:268, src/lib.rs:1033, src/test.rs:1800, src/test.rs:1826). None of them are in the three files this fix touches; dispute-resolution is fmt-clean on its own. The effect is that the CI fmt job is red on every open PR against this repo, not on anything those PRs changed.

I originally left it alone to keep this diff focused, but since it blocks this PR's CI I've applied it as a separate second commit (3bc11bc) rather than folding it into the fix. It's pure rustfmt output — two call-site rewrappings and two assert_eq! expansions, no semantic change — so it can be dropped or cherry-picked independently if you'd rather land it as its own PR. Happy to split it out if you prefer; just say so and I'll rebase.

With it applied, all four CI gates pass locally.

The Admin, EscrowContract, Version and NextDisputeId keys are written
once at initialize and never touched again, and nothing in the contract
ever called extend_ttl on instance storage. Measured in a test, the
instance entry's TTL sat at 4095 ledgers (~5.7h at 5s/ledger) after
initialize and only decayed from there.

Once archived, get_escrow_contract and get_admin fail with
NotInitialized, which bricks raise_dispute — the contract's only fully
implemented mutating entry point — until someone submits a
RestoreFootprint operation.

Add extend_instance_ttl to storage.rs, mirroring campaign-escrow's
helper and reusing the same ~30-day bump/threshold pair, and call it
from every public function in lib.rs. The read-only paths (get_dispute,
version) bump too: nothing writes to instance storage after initialize,
so leaving reads out would let the config expire on a contract that is
being actively read.

Tests assert the TTL is bumped to the full window after initialize, and
re-extended after raise_dispute and version once it has decayed under
the threshold. All three fail against the previous behaviour with an
observed TTL of 4095.

Closes Ads-Bazaar#59
`cargo fmt --all -- --check` has been failing on main since 43ba011,
7c41d08 and 748ba2f landed on 2026-08-25, so the CI fmt job is red on
every open PR rather than on anything those PRs changed.

Pure rustfmt output on the four reported hunks — two call-site
rewrappings in lib.rs and two assert_eq! expansions in test.rs. No
semantic change.

Unrelated to the dispute-resolution TTL fix in this branch; kept as its
own commit so it can be dropped or cherry-picked independently if the
maintainers would rather land it separately.
@olathedev

Copy link
Copy Markdown
Contributor Author

Pushed 3bc11bc to fix the red fmt job.

To be clear about attribution, since it looked like this PR's fault: the four failing hunks are all in campaign-escrow, which this PR doesn't touch. git blame puts them in 43ba011, 7c41d08 and 748ba2f (2026-08-25) — so cargo fmt --all -- --check is currently failing on main itself, and every open PR inherits it. dispute-resolution was already clean.

The commit is pure rustfmt output, kept separate from the TTL fix so it can be dropped or cherry-picked on its own. Also worth noting rust-toolchain.toml sets channel = "stable" while CI uses dtolnay/rust-toolchain@stable, so both float — a future rustfmt release can re-break this without anyone changing code. Pinning a concrete version would make the gate deterministic, if that's of interest.

All four gates green locally: fmt clean, clippy -D warnings clean, 147 tests pass, wasm release builds.

@JamesVictor-O
JamesVictor-O merged commit 33937c6 into Ads-Bazaar:main Aug 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: dispute-resolution never extends its instance storage TTL — Admin/EscrowContract keys can expire off-ledger

2 participants