Skip to content

farming-pool: no restore/recovery path exists for archived per-user persistent storage entries after TTL expiry #73

Description

@prodbycorne

Problem

Every per-user persistent storage entry (UserStake, UserPosition, UserBoost, BankedCredits) is only ever TTL-bumped as a side effect of that specific user transacting or being read:

const USER_TTL_THRESHOLD: u32 = 518_400;   // ~30 days at ~5s/ledger
const USER_TTL_EXTEND_TO: u32 = 1_036_800; // ~60 days

fn bump_user(env: &Env, key: &DataKey) {
    env.storage().persistent().extend_ttl(key, USER_TTL_THRESHOLD, USER_TTL_EXTEND_TO);
}

bump_user is only ever invoked from get_user_boost, get_user_stake, set_user_stake, get_position, set_position, set_banked_credits, and get_banked_credits — i.e. only when that specific user's key is read or written. If a user locks/stakes funds and then simply never calls any function again for longer than USER_TTL_EXTEND_TO (~60 days) without anyone (including read-only indexers) querying their specific UserStake/UserPosition, that persistent entry's TTL lapses and Soroban archives it.

Once archived, the entry cannot be read or written again without an explicit RestoreFootprint operation performed by whoever submits the transaction — and this contract exposes no function whatsoever to perform or trigger that restoration. There is no restore_position/restore_stake entry point, and none of the fund-recovery paths (unlock_assets, unstake, emergency_withdraw) can succeed against an archived key, because the very first thing each of them does is read that user's Position/UserStake via get_position/get_user_stake.

This is architecturally different from the factory contract's Pool records, where list_pools/get_pools_by_asset (called by any indexer, for any pool) opportunistically bump every pool's TTL as a side effect of routine registry queries — the farming pool has no equivalent "someone else's routine call touches my TTL" mechanism, because per-user keys are only ever touched by that user's own read/write.

Impact

  • Fund lock: a long-dormant staker/locker (inactive longer than the TTL window, entirely plausible for a "set and forget" LP position over a ~2-month horizon) risks having their UserStake/Position archived with no way to recover it — unstake()/unlock_assets() cannot even read the archived entry to begin the withdrawal.
  • No operational mitigation exists: there is no admin-callable "keep alive" sweep, no public restore_* entry point, and no monitoring hook (event) emitted as an entry's TTL approaches expiry that an off-chain keeper could react to.

Fix

At minimum, add an explicit, permissionless "touch" function any keeper/indexer can call to keep a specific user's entries alive without requiring that user to transact:

pub fn keep_alive(env: Env, user: Address) -> Result<(), PoolError> {
    require_initialized(&env)?;
    bump_instance(&env);
    if get_user_stake(&env, &user).is_some() { /* bump_user already runs inside get_user_stake */ }
    if get_position(&env, &user).is_some() { /* same */ }
    Ok(())
}

And/or document an operational runbook requiring an off-chain keeper to periodically call get_user_position/get_stake (which already bump TTL as a read side effect) for every active user before the ~60-day window elapses, backed by an integration test proving the TTL is actually extended by such a call.

Acceptance Criteria

  • A permissionless function (or documented equivalent workflow) exists to extend a specific user's UserStake/Position/UserBoost TTL without requiring that user to submit a state-mutating transaction.
  • Add a test that advances ledgers past USER_TTL_EXTEND_TO, confirms the entry's TTL has fallen below USER_TTL_THRESHOLD, then calls the new keep-alive path and asserts the TTL is restored to USER_TTL_EXTEND_TO (mirroring the factory's existing test_get_pool_bumps_pool_record_ttl pattern).
  • Document, in a code comment or README, the archival risk and the required off-chain keeper cadence for pools expected to have long-dormant stakers.

Further Investigation

Marked spike: this is an operational/architectural question (what recovery mechanism should exist, who is responsible for triggering it, and on what cadence) rather than a mechanical code fix — the issue's own "Fix" section is explicitly framed as "at minimum" / "and/or," signaling there isn't one obviously-correct answer.

Confirmed against current source: bump_user (farming-pool/src/lib.rs:19-23) is called only from get_user_boost (74-81), get_user_stake (83-90), set_user_stake (92-96), get_position (110-117), set_position (119-123), and set_banked_credits/get_banked_credits (104-108, 394-402) — confirmed no other call sites exist. Confirmed remove_user_stake (98-102) and remove_position (125-129) correctly do not bump (deleting a key that's about to be removed is a no-op concern, not a bug). Confirmed there is no restore_*/keep_alive entry point anywhere in farming-pool/src/lib.rs's public #[contractimpl] block. Confirmed the factory contract's asymmetric mitigation: list_pools (factory/src/lib.rs:101-125) and get_pools_by_asset (132-150) both call bump_pool (18-22) for every pool touched during a routine, permissionless, no-auth scan — meaning any indexer's routine read keeps every pool's TTL alive as a side effect, with no equivalent "scan all users" call possible for UserStake/Position since there's no enumerable index of users (unlike the factory's PoolCount-indexed pools).

Edge cases:

  • The four per-user key types don't expire independently in general — a user with both a Position and a UserStake (both are usable simultaneously; emergency_withdraw, lib.rs:351-392, handles both) has two separately-TTL'd entries that could each archive at different times if the user's activity touches one but not the other (e.g. only calling unlock_assets-adjacent functions bumps Position but not UserStake). Any recovery mechanism needs to handle each DataKey variant independently, not assume a user's records are all-or-nothing.
  • UserBoost (per get_user_boost, lines 74-81) is only bumped on read, and only set_boost (459-484) writes it — a user who sets a boost once and then only interacts via plain stake/unstake (which read get_user_boost inside checkpoint, line 149) does incidentally bump UserBoost's TTL as a side effect of unrelated calls, whereas a user with a Position (lock/unlock path) never touches UserBoost at all and it could archive independently — worth explicitly stating this dependency in whatever runbook/keep-alive design is chosen.
  • Interacts with farming-pool: emergency_withdraw transfers funds before clearing UserStake/Position, opening a reentrant double-withdraw window #72 (emergency_withdraw): if a key has already archived before an incident, emergency_withdraw's get_position/get_user_stake calls (lines 365, 372) cannot read it — the fix for farming-pool: no restore/recovery path exists for archived per-user persistent storage entries after TTL expiry #73 (whatever it is) determines whether emergency_withdraw remains the true "last resort," or whether it also needs to attempt a restore-then-read pattern.
  • Any on-chain keep_alive-style function is itself callable during a paused pool or not — worth deciding explicitly (the issue's sketch doesn't gate on pool_is_paused, meaning it'd remain callable even during an incident, which seems correct/desirable for exactly the reason farming-pool: emergency_withdraw transfers funds before clearing UserStake/Position, opening a reentrant double-withdraw window #72 cares about archival at incident time).

Tradeoff approaches:

  1. Permissionless on-chain keep_alive(user) function (per issue's sketch): cheap to implement (just calls the existing getters, which already bump on read), requires an off-chain keeper/cron to actually invoke it before the ~60-day window — pushes the operational burden entirely off-chain with no on-chain enforcement that it actually happens; if the keeper fails/is discontinued, archival still occurs, just later than without the function.
  2. Documented off-chain runbook only, no new function: zero code changes, relies entirely on someone periodically calling existing read-only getters (get_user_position/get_stake, which already bump TTL, per lib.rs:319-323, 588-592) for every known active user — cheapest to build, but has no contract-level acknowledgment of the risk and is easy to silently let lapse since nothing on-chain signals an approaching expiry.
  3. Proactive on-chain restore integrated into the recovery paths themselves: rather than a separate keep_alive, have unlock_assets/unstake/emergency_withdraw attempt an explicit try_restore before reading (Soroban's extend_ttl/restore semantics permit a transaction to restore an archived entry if the submitter includes the right footprint) — this shifts the burden to whoever submits the withdrawal transaction (who has direct incentive to succeed) rather than a third-party keeper, but doesn't prevent archival from happening in the first place, only enables recovery after the fact, and may require RPC/SDK-level footprint changes outside the contract's own control (Soroban's restore is a transaction-level operation, not purely a contract-level one — needs verification against current Soroban Restore-Footprint semantics for whether a contract can trigger this from within a call, or whether it's strictly a client-side XDR footprint concern outside the contract's control entirely).

Test plan (for whichever approach is chosen): at minimum, add a test that advances ledgers past USER_TTL_EXTEND_TO (mirroring the factory's test_get_pool_bumps_pool_record_ttl pattern, confirmed present at factory/src/test.rs:559-572), confirms the TTL has fallen below USER_TTL_THRESHOLD for a UserStake/Position key, then exercises the chosen mechanism (keep_alive call, or a getter call per the runbook) and asserts the TTL is restored to USER_TTL_EXTEND_TO. If approach 3 is pursued, a dedicated integration-test design question arises (how to simulate an actually archived, not just low-TTL, entry within the soroban_sdk test harness) — this sub-question may itself need spike-level investigation into soroban-sdk's testutils support for simulating archived-entry restoration, since advance_ledgers (test.rs:110-113) only manipulates the ledger sequence number, not archival state directly.

Cross-references: #72 (emergency_withdraw's ability to recover funds is gated on this issue's resolution), #74 (test coverage for the TTL-bump mechanism itself, prerequisite groundwork for testing whatever this issue's fix adds).

Metadata

Metadata

Labels

GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignOfficial Campaign | FWC26Campaign: Official Campaign | FWC26farming-poolFarmingPool contractinfrastructureBuild tooling, deployment, CI/CDvery hardExtremely hard — deep expertise, careful design, and significant time required

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions