fee_collector: persistent per-token totals are never TTL-extended, and get_total_collected silently masks archival as zero

[abayomicornelius]

Overview

fee_collector's only persistent storage is a lifetime-total accounting counter, keyed per token:

// fee_collector/src/lib.rs:32-33
/// Persistent key prefix for lifetime-total-collected per token.
const KEY_TOTAL: Symbol = symbol_short!("TOTAL");

Nothing in the contract ever calls extend_ttl on (KEY_TOTAL, token) entries (confirmed via grep -rn "extend_ttl\|bump\|ttl\|TTL" fee_collector/src, zero matches). For an actively-used token this may never matter in practice, since collect_fee touches the entry on every fee forwarded. But get_total_collected reads it with:

// fee_collector/src/lib.rs:186-192
pub fn get_total_collected(env: Env, token: Address) -> i128 {
    let total_key = (KEY_TOTAL, token);
    env.storage()
        .persistent()
        .get(&total_key)
        .unwrap_or(0i128)
}

unwrap_or(0i128) treats "entry not present" identically to "this token has never had any fees collected" — which is exactly correct for a genuinely new token, but is the wrong interpretation if the entry used to exist with a real, nonzero total and has since gone stale from TTL expiry. This is meaningfully worse than the equivalent risk in the other three contracts in this batch's TTL findings: those either fail loudly (a get() used with .ok_or(SomeError) on a required record) or the consequence is "temporary inaccessibility until restored." Here, a lapsed entry doesn't produce an error at all — get_total_collected just silently reports 0, indistinguishable from "no fees were ever collected for this token," for a token that may have accumulated a large lifetime total before going dormant (e.g. a token the protocol stopped actively routing payments through for a stretch, or one that's naturally low-frequency).

Since get_total_collected is this contract's only on-chain accounting/audit surface for lifetime fee totals (there's no separate ledger, no event-replay requirement documented anywhere), any off-chain tooling, dashboard, or on-chain caller that trusts this value for reconciliation, reporting, or a future withdrawal-limit feature (see the stubs.rs backlog item "add per-epoch withdrawal limit") would silently and permanently lose historical accounting data with zero indication anything went wrong.

Requirements

• Extend the TTL of (KEY_TOTAL, token) on every write in collect_fee (fee_collector/src/lib.rs:104-113), sized generously since this contract has no way to predict how often a given token will see traffic again.
• Since get_total_collected is a read-only query and can't itself force a TTL extension without a state-changing transaction, consider whether the accounting model should be event-sourced as documented ground truth (i.e. explicitly say the persistent counter is a cache that can, in principle, be reconstructed by replaying fee_rcvd events, and that get_total_collected returning 0 for a token with historical activity is a known caveat) rather than implying the counter is authoritative.
• At minimum, make the risk visible: this function silently returning a wrong-but-plausible value (0) is strictly worse than an error, and is worth flagging loudly in the doc comment even if the TTL-extension fix isn't shipped immediately.

Acceptance Criteria

• collect_fee extends the TTL of the (KEY_TOTAL, token) entry on every call.
• get_total_collected's doc comment explicitly states the caveat: without the TTL fix (or as defense in depth even with it, for entries whose extended TTL is itself exceeded), 0 is ambiguous between "never collected" and "collected historically, entry now stale."
• A test using soroban-sdk's TTL testutils demonstrates that a token with a real, nonzero lifetime total continues to report it correctly after enough ledgers pass that the entry would otherwise have gone stale, once the TTL-extension fix lands.
• Consider adding a distinguishing return type (e.g. Option<i128> or a 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, even after the extension fix reduces how often this actually triggers.

Additional Notes

Precise references: fee_collector/src/lib.rs:104-113 (the write path in collect_fee, no TTL call), :186-192 (get_total_collected, unwrap_or(0i128) masking the archived-vs-never-collected ambiguity), confirmed via repo-wide grep that no TTL API is used anywhere in this workspace's non-test source.

Why this is a sharper variant of the other TTL findings in this batch: the escrow and stellar_send (subscription) TTL issues in this batch describe a loud failure — a transaction traps or requires an explicit restore operation, which is bad UX but at least visible and fixable per-key. This one is silent: no error, no trap, just a plausible-looking wrong number. That makes it strictly harder to notice in production and a worse fit for any downstream system (dashboards, treasury reporting, a future withdrawal-limit feature) that treats get_total_collected as ground truth.

Test/reproduction plan: in fee_collector/src/test.rs, extend the existing test_collect_fee_updates_total pattern (fee_collector/src/test.rs:68-82): call collect_fee to establish a nonzero total for a token, then use the ledger/TTL testutils to advance far enough that the persistent entry's TTL would lapse without an explicit extension (mirroring the approach in the other TTL tests in this batch), and assert get_total_collected still returns the correct nonzero value post-fix rather than silently reporting 0.

[DammyAji]

Hi Maintainer, I’d like to take on this issue because it addresses a subtle accounting problem where an expired persistent entry can make historical fee data appear as zero. I’m interested in contributing to contract reliability and learning more about Soroban storage and TTL behavior through a real issue where data correctness is important.
From the issue description, I understand that the lifetime total for each token is stored persistently but its TTL is not extended when "collect_fee" updates it. Because "get_total_collected" falls back to "0" when the entry is no longer available, a token with previous fee activity could be incorrectly presented as having no collected fees. My approach would be to first examine the existing "fee_collector" implementation and its test patterns, then make the TTL handling and accounting behavior consistent with the contract's existing design.
I’d pay particular attention to the persistent storage lifecycle and the Soroban TTL test utilities when implementing and testing the fix. I’d also make sure the documentation clearly communicates the ambiguity around a zero value and consider the proposed way of distinguishing a genuinely new token from an entry that may have become stale. I’ll follow the acceptance criteria closely and use the existing tests as a guide for reproducing the issue before validating the fix.
I’d first go through the relevant parts of the codebase carefully to understand how the "fee_collector" contract currently handles storage, fee collection, lifetime totals, and testing. This would help me make the changes in a way that follows the project's existing patterns rather than introducing an approach that doesn't fit the contract.
I’d really appreciate the opportunity to take on this challenge. Thank you for considering my application.

[grantfox-oss]

🦊 GrantFox — @DammyAji 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 #39)
2. Your PR will be reviewed by the StellarSend maintainers

Good luck! Track your progress on GrantFox.