Skip to content

Fix/used from tx storage leak#188

Open
dripsmvcp wants to merge 1 commit intoentrius:testfrom
dripsmvcp:fix/used-from-tx-storage-leak
Open

Fix/used from tx storage leak#188
dripsmvcp wants to merge 1 commit intoentrius:testfrom
dripsmvcp:fix/used-from-tx-storage-leak

Conversation

@dripsmvcp
Copy link
Copy Markdown

@dripsmvcp dripsmvcp commented Apr 24, 2026

Summary

Fixes #174used_from_tx storage leak.

  • Mapping<String, bool>Mapping<String, ()>. The value was always true; unit reflects the set semantics and drops one byte per entry.
  • Replace get(&k).unwrap_or(false) with .contains(&k) at the guard site for readability.
  • Add a comment documenting why entries must persist across swap completion.

Why not delete entries on confirm / timeout

The issue report suggests cleaning entries in confirm_swap / timeout_swap, analogous to the request_votes cleanup in #149. That approach is unsafe here:

  • swaps.remove(swap_id) already runs on both confirm (lib.rs:804) and timeout (lib.rs:861).
  • After that removal, used_from_tx is the only on-chain record that a given source tx hash has already been consumed.
  • The guard at lib.rs:648 is what prevents a second vote_initiate from replaying the same user payment to pay a miner twice.

Removing entries on completion would reopen that replay vector, so this PR keeps the dedup permanent and takes the modest win of shrinking each entry's storage footprint.

The request_votes case in #149 is different — request IDs are monotonic, so cleaned-up entries can never collide with a future round.

Caveats

  • Storage-layout change — requires fresh deploy. Same constraint as Plug request_votes storage leak #149; an in-flight upgrade would lose any existing used_from_tx entries. Halt first if upgrading a live instance.
  • Entry-count growth is inherent to replay protection, not fixed by this PR. If the team later decides to bound growth, it needs an explicit policy (e.g., a reorg-horizon window per source chain, or moving dedup off-chain to validator-level state). That's a larger design discussion; this PR is the narrow, safe improvement.

Test plan

  • cargo check --release passes
  • cargo contract build --release produces optimized wasm
  • E2E suite against a redeployed contract confirms initiate → confirm → replay-attempt flow still rejects DuplicateSourceTx

Mapping<String, bool> -> Mapping<String, ()>. The value was always true;
unit reflects the set semantics and drops one byte per entry. Switch the
guard to .contains() for readability.

Entries cannot be removed on confirm/timeout: swaps.remove(swap_id) wipes
the only other on-chain record of a consumed source tx, so used_from_tx
is the sole replay guard at initiate. Comment documents this.

Storage-layout change; requires fresh deploy.

Refs: entrius#174

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dripsmvcp dripsmvcp force-pushed the fix/used-from-tx-storage-leak branch from f3e1a47 to 06a6e00 Compare April 24, 2026 15:49
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.

Storage leak: used_from_tx never cleaned up

1 participant