Skip to content

bridge: decouple the idempotency store reset from --rescan #1111

Description

@sameh-farouk

Summary

The crash-safe idempotency store added in #1096 has its reset tied to --rescan. Those two things are needed for unrelated reasons and at very different frequencies, so any deployment that keeps --rescan on for its Stellar-side purpose silently loses the crash-safety the store exists to provide.

Suggesting we decouple them.

What --rescan does today

pkg/bridge/bridge.go does three things behind the one flag:

blockPersistency.SaveStellarCursor("0")   // re-read Stellar history from the start
blockPersistency.SaveHeight(0)            // re-read TFChain history from the start
...
if cfg.RescanBridgeAccount {
    idempotency.Reset()                   // wipe withdraw/refund processing state
}

Its documented purpose is the first of those — the help text is "if true is provided, we rescan the bridge stellar account and mint all transactions again".

Why the coupling is wrong

The two key spaces in the store have different provenance:

key source can it collide after a reset?
withdraw withdrawReady.ID (uint64) TFChain burn tx id — see subClient.GetBurnTransaction(types.U64(txID)) Yes, but only if TFChain is reset — ids restart from low numbers
refund RefundTransactionHash Stellar deposit tx hash — matched via FindRefundByReturnHashInPage on a MemoReturn No — content-derived, never repeats

So the store only ever needs clearing when TFChain is reset, which is the reason given in the code comment. Nothing about a Stellar reset can cause a key collision: refund keys are hashes, and withdraw keys aren't Stellar-derived at all.

But --rescan is also the only tool for the Stellar-side need — recovering a stale Stellar cursor and re-reading deposits — and that need arises far more often than a TFChain reset. Which means an operator who sets --rescan for a Stellar reason unavoidably discards TFChain-scoped state they had no reason to touch.

This is already happening

The devnet bridges run rescan: true (chart value). The idempotency store is therefore wiped on every start, so:

The flag is presumably on for cursor-recovery reasons rather than because anyone wants the store cleared. Turning it off would fix the store but changes deposit-scanning behaviour — which is the coupling this issue is about, not a real choice between the two.

Suggested improvement

Preferred: scope withdraw keys by TFChain genesis hash. Key withdraws as (genesis_hash, txID) instead of txID. A chain reset changes the genesis, old keys become unreachable, and collisions are impossible by construction. No operator action, no flag to remember, and the store can then safely persist across every --rescan.

Refund keys need no change — Stellar tx hashes are already collision-free.

The reset call can then be dropped from the --rescan path entirely, leaving that flag meaning only what its help text says.

Alternative if genesis scoping is more than we want: split the behaviour out into its own flag, e.g. --reset-idempotency, used only when TFChain is actually reset. Less good, because it keeps a footgun that has to be documented and remembered, but it decouples the two.

Note on growth

Keys are deliberately never pruned — the comment reasons ~50 bytes per entry, so ~18 MB/year at 1000 tx/day, omitted "for simplicity and auditability". Genesis scoping would leave entries from a superseded chain as dead weight, which at those volumes is immaterial; worth mentioning only so it's a conscious choice rather than a surprise.

Not urgent

Nothing is broken in production by this. It is an improvement to make the guarantee hold regardless of how --rescan is used, and to stop devnet from silently running without the protection it is supposed to be validating.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions