Skip to content

Scale extend_deadline's TTL bump to the deadline; add permissionless keep_alive - #72

Merged
chonilius merged 6 commits into
MergeFi:mainfrom
Smoothjane:fix/extend-deadline-ttl-mismatch-56
Aug 19, 2026
Merged

Scale extend_deadline's TTL bump to the deadline; add permissionless keep_alive#72
chonilius merged 6 commits into
MergeFi:mainfrom
Smoothjane:fix/extend-deadline-ttl-mismatch-56

Conversation

@Smoothjane

@Smoothjane Smoothjane commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

extend_ttl bumps a persistent record's TTL by a fixed ~29 days (500_000 ledgers at ~5s/ledger) regardless of context. escrow::extend_deadline calls it after setting escrow.deadline to whatever far-future value a contributor requests — but the TTL bump never scaled with that deadline, so a contributor calling extend_deadline(issue_id, now + 1 year) and reasonably doing nothing else would still see the record's actual on-chain survivability lapse in ~29 days, not the year they thought they'd secured. Past that, the record moves toward archival and becomes fully inaccessible (get_escrow, release, refund all fail) until someone submits a RestoreFootprint operation — a step this repo's scripts don't automate.

Implemented both directions the issue asks for, not just one:

  • mergefi_common::extend_ttl_for_target(env, key, target_timestamp): converts target_timestamp - now into an approximate ledger count (~5s/ledger) and extends to that many ledgers — capped at env.ledger().max_live_until_ledger(), Soroban's own real, runtime-queryable ceiling on a persistent entry's TTL (~1 year on a typically-configured network — confirmed via the SDK's own testutils default, not assumed). Never extends less than the existing flat 500_000-ledger baseline, so near-future deadlines don't regress.
  • extend_deadline now calls this (targeting new_deadline + GRACE_PERIOD, so the permissionless-refund window itself stays reachable) instead of the flat bump.
  • Added escrow::keep_alive(issue_id) — permissionless, touches only TTL, never deadline or status — because even the capped scaling can't cover a deadline set beyond the network's own ceiling in one call; a concerned sponsor/contributor or an automated mergefi-backend job can call this periodically to keep a far-future escrow alive toward it.
  • Updated extend_deadline's doc comment and the README's new "Storage & TTL" section to be explicit about what the scaling does and does not guarantee (the residual gap beyond the network ceiling, and keep_alive as the mitigation for it) — closing the documentation/expectation mismatch the issue is fundamentally about, not just the numbers.

Also fixed two pre-existing, unrelated CI blockers (both confirmed present on a clean, unmodified upstream/main clone before this PR touched anything, and both blocking this PR's own CI from running at all):

  • contracts/milestones/src/lib.rs: a missing closing brace (sort_remainders_desc's body had a doc comment for the next function concatenated directly onto it, almost certainly a lost } from a bad merge) broke cargo build --workspace and the wasm-build CI job entirely, for any PR against this repo, not just this one.
  • contracts/escrow/src/test.rs: test_multi_sponsor_refund_returns_exact_contributions_to_each_sponsor advanced the ledger timestamp to a stale hardcoded 300 instead of 200 + GRACE_PERIOD (GRACE_PERIOD is 14 days in seconds = 1,209,600) — so refund() correctly took its admin-only branch instead of the intended permissionless one, and failed on the missing admin auth. Test-only fix, matching the exact pattern the neighboring (already-correct) test_refund_after_deadline_is_permissionless uses.

Test plan

  • test_extend_deadline_scales_ttl_proportionally_for_a_moderately_far_future_deadline — a 90-day deadline gets proportional TTL, exact-value assertion
  • test_extend_deadline_caps_ttl_at_the_network_max_for_a_very_far_future_deadline — a 3-year deadline gets exactly the network's max, not a naive (impossible) larger value
  • test_extend_deadline_never_extends_less_than_the_existing_flat_baseline — a near-future deadline still gets at least the old 500_000-ledger floor
  • test_keep_alive_refreshes_ttl_without_changing_deadline_or_status — asserts the escrow record is byte-identical before/after, only TTL moves
  • test_keep_alive_rejects_nonexistent_escrow
  • cargo test --workspace — all 4 crates, fully green (48/48 including maintenance-pool/milestones)
  • cargo build --target wasm32v1-none --release — succeeds
  • cargo fmt --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean

Closes #56

@vercel

vercel Bot commented Aug 19, 2026

Copy link
Copy Markdown

@Smoothjane is attempting to deploy a commit to the chonilius' projects Team on Vercel.

A member of the Team first needs to authorize it.

sort_remainders_desc was missing its closing brace — a doc comment for
the next function (refund_remaining_budget) had been concatenated
directly onto its body, almost certainly a lost brace from a bad
merge. This broke cargo build --workspace and cargo build --target
wasm32v1-none --release entirely (not just this PR's changes), which
is why this PR's CI wasm-build job failed.

Pre-existing on upstream/main itself — confirmed via a clean,
unmodified clone reproducing the identical error before this change.
Purely a missing `}`; no logic changed.
This test advanced the ledger timestamp to a hardcoded 300 and then
called permissionless refund() with all auths turned off, expecting
the deadline+grace-period path. But GRACE_PERIOD is 14 days in
*seconds* (1,209,600) — 300 is nowhere near deadline(200) +
GRACE_PERIOD, so refund() correctly took its admin-only branch
instead, and failed on the missing admin auth. The neighboring
test_refund_after_deadline_is_permissionless already gets this right
(`set_timestamp(200 + crate::GRACE_PERIOD)`); this test just used a
stale/wrong literal instead of the same pattern.

Test-only fix — no contract logic changed. Pre-existing on
upstream/main itself — confirmed via a clean, unmodified clone
reproducing the identical failure before this change.
@chonilius

Copy link
Copy Markdown
Contributor

checks passed

@chonilius
chonilius merged commit 16622a4 into MergeFi:main Aug 19, 2026
2 of 3 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.

escrow::extend_deadline: a far-future new_deadline creates false security — persistent TTL is still bumped by a fixed ~29 days, not proportionally

2 participants