fix(fee_collector): extend TTL of KEY_TOTAL on every collect_fee write, add get_total_collected_opt, document ambiguous-zero caveat

[DammyAji]

Summary

Closes #39

This PR fixes the silent-data-loss bug described in issue #39: the (KEY_TOTAL, token) persistent storage entry written by collect_fee was never TTL-extended, meaning a token that went dormant long enough would eventually have its lifetime-total entry archived by the network. When that happened, get_total_collected would silently return 0 — indistinguishable from "no fees have ever been collected for this token" — with no error, no warning, and no mechanism for any caller to detect that the value was wrong rather than genuinely zero.

This is the sharpest variant of the TTL class of bugs in this codebase because it is silent: other TTL omissions produce loud traps or explicit "not found" errors that at least surface the problem. A wrong-but-plausible 0 from get_total_collected is the worst possible failure mode for an accounting function, and is especially dangerous for any downstream system (treasury dashboard, per-epoch withdrawal-limit feature, on-chain audit trail) that treats this value as ground truth.

---

Root cause (as diagnosed in issue #39)

// fee_collector/src/lib.rs — BEFORE this PR (lines 104-113)
let total_key = (KEY_TOTAL, token.clone());
let current_total: i128 = env.storage().persistent().get(&total_key).unwrap_or(0i128);
let new_total = current_total.checked_add(amount)…;
env.storage().persistent().set(&total_key, &new_total);
// ← no extend_ttl call here — entry TTL never extended after write

// fee_collector/src/lib.rs — BEFORE this PR (lines 186-192)
pub fn get_total_collected(env: Env, token: Address) -> i128 {
    env.storage().persistent().get(&total_key).unwrap_or(0i128)
    // ↑ unwrap_or(0) makes "entry not present" and "entry = 0" indistinguishable
}

A repo-wide grep -rn "extend_ttl|bump|ttl|TTL" fee_collector/src returned zero matches in non-test source before this PR, confirming no TTL extension existed anywhere in the contract.

---

Changes

fee_collector/src/lib.rs

1. TTL policy constants — new section added immediately after KEY_TOTAL:

/// Threshold: if remaining TTL falls below this, extend to TOTAL_TTL_TARGET.
/// ~30 days at 5 s/ledger (≈518_400 ledgers).
const TOTAL_TTL_THRESHOLD: u32 = 518_400;

/// Target TTL after extension. ~120 days at 5 s/ledger (≈2_073_600 ledgers).
/// Intentionally generous — the lifetime-total counter is the only on-chain
/// audit surface for fee accounting, and the cost of re-extending a healthy
/// entry is negligible compared to the cost of silently losing historical data.
const TOTAL_TTL_TARGET: u32 = 2_073_600;

Both values are documented in full with rationale for their specific sizes.

2. extend_ttl call in collect_fee — added immediately after set():

env.storage().persistent().set(&total_key, &new_total);

// Extend the TTL of the lifetime-total entry on every write so it
// stays live even if this token goes dormant for a long stretch.
// Without this, a token that stops receiving fees could have its
// entry expire, causing `get_total_collected` to silently return 0
// instead of the real historical total — indistinguishable from
// "never collected any fees" and strictly worse than a loud error.
env.storage()
    .persistent()
    .extend_ttl(&total_key, TOTAL_TTL_THRESHOLD, TOTAL_TTL_TARGET);

The extend_ttl call uses the threshold + target pattern, so the Soroban host skips the extension if the entry's current TTL already exceeds TOTAL_TTL_THRESHOLD — there is no wasteful re-bump on every single fee when the entry is already healthy. The extension only fires when the remaining TTL has decayed below ~30 days, at which point it is reset to ~120 days.

3. Updated get_total_collected doc comment — new # Caveat: ambiguous zero section:

The doc comment now explicitly documents both situations in which this function returns 0:

• Case 1 — genuinely zero: no fees have ever been collected for this token.
• Case 2 — stale / archived: fees were collected historically, but the (KEY_TOTAL, token) persistent entry's TTL lapsed (e.g. the token stopped receiving traffic long enough for the entry to be archived by the network), and no one has restored it yet.

It states that the extend_ttl fix makes case 2 unlikely for any actively-used token but cannot be ruled out for tokens dormant longer than TOTAL_TTL_TARGET (~120 days). It directs callers who need to distinguish the two cases to get_total_collected_opt. It also explicitly documents that fee_rcvd events are the authoritative source of truth and that the persistent counter is a reconstructible cache — not the sole record.

4. New get_total_collected_opt function:

/// Return the lifetime total fees collected for `token`, or `None` if
/// the persistent entry is absent.
///
/// Unlike `get_total_collected`, this function surfaces the distinction between:
/// * `Some(n)` — entry is present with total `n` (always > 0 in practice)
/// * `None`    — entry is absent: either never collected, OR entry existed
///               but its TTL lapsed and it has since been archived
pub fn get_total_collected_opt(env: Env, token: Address) -> Option<i128> {
    let total_key = (KEY_TOTAL, token);
    env.storage().persistent().get(&total_key)
}

This gives treasury tooling, future per-epoch withdrawal-limit logic, and any on-chain caller a first-class way to distinguish the two failure modes that get_total_collected collapses into 0. The full doc comment explains all three possible return variants (None, Some(0) edge case, Some(n)) and when each applies.

Naming note: named get_total_collected_opt rather than try_get_total_collected to avoid clashing with the try_* client wrapper methods auto-generated by the #[contractimpl] macro for every exported function. try_get_total_collected would have caused a duplicate-definition compile error (E0592).

---

fee_collector/src/test.rs

Imports added:

use soroban_sdk::{
    testutils::{Address as _, Ledger as _, storage::Persistent as _},
    …
};

• Ledger as _ — required for env.ledger().with_mut(|li| { … }) to set sequence_number, min_persistent_entry_ttl, and max_entry_ttl in the TTL test.
• storage::Persistent as _ — required to call get_ttl(&key) on a Persistent storage instance inside env.as_contract blocks.

New test: test_total_collected_ttl_extended_across_ledgers

This test directly implements the reproduction plan from the issue's "Test/reproduction plan" section: establish a nonzero total via collect_fee, advance the ledger far enough that the entry would have been archived without the fix, and assert the total is still correct.

The test strategy is adapted for soroban-sdk v21 behaviour: in v21, env.as_contract itself requires the contract instance to be live (the host loads the instance entry when creating the test frame). Simply archiving the instance and calling into the contract would fail at the frame-creation level, not at the data-access level — which would not produce a meaningful signal about the data entry's TTL. Instead, the test:

1. Configures min_persistent_entry_ttl = 5_000 and max_entry_ttl = 3_000_000. Without extend_ttl, new KEY_TOTAL entries would start with TTL = 4,999.
2. Calls collect_fee(token, 1_000). The extend_ttl call inside it should set the entry TTL to TOTAL_TTL_TARGET = 2_073_600.
3. Asserts via env.as_contract + get_ttl that the TTL equals exactly TOTAL_TTL_TARGET — not 4,999. This is the direct, unambiguous proof that extend_ttl ran.
4. Separately extends the contract instance TTL using env.as_contract + instance.extend_ttl, so the instance survives the ledger advance and subsequent client calls remain possible.
5. Advances the ledger sequence by 10,000 — past the un-extended TTL of 4,999, but a small fraction of TOTAL_TTL_TARGET.
6. Via env.as_contract: asserts the KEY_TOTAL entry is still Some(1_000) (not archived), its TTL decayed to exactly TOTAL_TTL_TARGET - 10_000 = 2_063_600, and that value is still above TOTAL_TTL_THRESHOLD.
7. Via the public client API: asserts get_total_collected(token) returns 1_000 (not 0) and get_total_collected_opt(token) returns Some(1_000) (not None) — end-to-end correctness through the contract's public interface.
8. Calls collect_fee(token, 500) again and asserts cumulative total = 1,500 on both accessors.

---

fee_collector/test_snapshots/test/

• Updated test_collect_fee_updates_total.1.json and test_withdraw_sends_tokens_to_recipient.1.json: the TTL value for the (KEY_TOTAL, token) ledger entry changed from 4095 (the soroban-sdk test-environment default min TTL when no ledger config is set) to 2073600 (TOTAL_TTL_TARGET). This is the correct and expected reflection of the new extend_ttl call executing inside collect_fee in these tests.
• Added test_total_collected_ttl_extended_across_ledgers.1.json: the ledger snapshot for the new TTL test.

---

Acceptance criteria — line-by-line verification

#	Acceptance criterion from issue #39	Status	Precise evidence
AC1	collect_fee extends the TTL of the (KEY_TOTAL, token) entry on every call	✅	lib.rs: env.storage().persistent().extend_ttl(&total_key, TOTAL_TTL_THRESHOLD, TOTAL_TTL_TARGET) immediately after set()
AC2	get_total_collected's doc comment explicitly states the caveat: 0 is ambiguous between "never collected" and "collected historically, entry now stale"	✅	lib.rs: # Caveat: ambiguous zero section in the doc comment, both cases named and explained
AC3	A test using soroban-sdk TTL testutils demonstrates a token with a nonzero lifetime total continues to report it correctly after enough ledgers pass that the entry would otherwise have gone stale	✅	test.rs: test_total_collected_ttl_extended_across_ledgers — get_ttl proves TTL == TOTAL_TTL_TARGET immediately after collect_fee; ledger advance of 10,000 past un-extended TTL of 4,999; Some(1_000) confirmed in storage and via public API
AC4	Consider adding a distinguishing return type (Option<i128> or dedicated error variant on a try_get_total_collected) so callers who need to distinguish "genuinely zero" from "possibly stale" have a way to do so	✅	lib.rs: get_total_collected_opt(env, token) -> Option<i128> with full doc comment covering all three return variants

---

Test results

running 11 tests
test test::test_collect_fee_invalid_amount                    ... ok
test test::test_collect_fee_updates_total                     ... ok
test test::test_get_total_collected_starts_at_zero            ... ok
test test::test_get_balance_reflects_token_balance            ... ok
test test::test_initialize                                    ... ok
test test::test_initialize_already_initialized                ... ok
test test::test_not_initialized_errors                        ... ok
test test::test_set_treasury                                  ... ok
test test::test_withdraw_invalid_amount                       ... ok
test test::test_total_collected_ttl_extended_across_ledgers   ... ok
test test::test_withdraw_sends_tokens_to_recipient            ... ok

test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

All 10 pre-existing tests continue to pass. 1 new TTL test added. Zero regressions.

---

Why this fix is correct and safe

• extend_ttl uses threshold + target semantics — the Soroban host skips the extension if current_ttl > TOTAL_TTL_THRESHOLD, so there is no wasted ledger-write budget re-bumping a healthy entry on every fee.
• TOTAL_TTL_THRESHOLD (~30 days) and TOTAL_TTL_TARGET (~120 days) are deliberately generous: the lifetime-total counter is the sole on-chain audit surface for fee accounting, and the cost of re-extending it is negligible compared to the cost of silent historical data loss.
• get_total_collected signature and return type are unchanged — zero breaking changes for existing callers or off-chain tooling.
• get_total_collected_opt is purely additive.
• The fee_rcvd events emitted by collect_fee remain the authoritative source of truth and are explicitly documented as such — the persistent counter is a reconstructible cache, not the sole record.
• No changes to any other contract (stellar_send, token_bridge, escrow). Scope is strictly limited to fee_collector.

---

Files changed

File	Change
fee_collector/src/lib.rs	TTL constants, extend_ttl in collect_fee, updated get_total_collected doc, new get_total_collected_opt
fee_collector/src/test.rs	New TTL testutil imports, new test_total_collected_ttl_extended_across_ledgers test
fee_collector/test_snapshots/test/test_collect_fee_updates_total.1.json	TTL field updated 4095 → 2073600
fee_collector/test_snapshots/test/test_withdraw_sends_tokens_to_recipient.1.json	TTL field updated 4095 → 2073600
fee_collector/test_snapshots/test/test_total_collected_ttl_extended_across_ledgers.1.json	New snapshot for TTL test

Closes #39

[DammyAji]

@abayomicornelius Please kindly review. Thank you.

[DammyAji]

@abayomicornelius Please kindly review and merge.