Skip to content

fix(repl): persist wallet after each command - #376

Open
emma31-dev wants to merge 2 commits into
bitcoindevkit:masterfrom
emma31-dev:fix/repl-wallet-persist
Open

emma31-dev wants to merge 2 commits into
bitcoindevkit:masterfrom
emma31-dev:fix/repl-wallet-persist

Conversation

@emma31-dev

Copy link
Copy Markdown

Fixes #375

Description

REPL mode loaded a persisted wallet, but the REPL loop never called wallet.persist(). The non-REPL path persists after every wallet command, so changes made inside the REPL were lost on exit.

This adds wallet.persist()? after every successful REPL command. Consequences of the bug this fixes:

  • wallet new_address rewound the address index each session, handing out already-used addresses
  • create_tx / bump_fee results disappeared from the tx graph
  • broadcast transactions were not recorded
  • lock_utxo / unlock_utxo state was lost
  • full_scan / sync results were discarded, so funds appeared missing until re-syncing

Notes to the reviewers

  • I persist after each command, not only on exit, so a crash or Ctrl-C doesn't discard the session either. Persisting an empty changeset is a no-op in BDK, so the extra calls are cheap.
  • Failed commands do not persist, matching the non-REPL path (cmd.execute(...)? skips the persist on error). Happy to change this if reviewers prefer persisting regardless.
  • The new test is gated on repl + sqlite (not repl + electrum like the existing repl_tests) so it runs under the default feature set. It spawns two separate REPL processes against the same datadir and asserts the address index advances 0 -> 1.
  • I confirmed the test fails on current master (0 vs 1) and passes with the fix.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

The REPL loaded a persisted wallet but never called `wallet.persist()`, so
every change made in a session was lost on exit: the address index rewound
(reusing already handed-out addresses), and created or broadcast transactions,
UTXO locks and scan results disappeared.

Persist after each successful command, matching the non-REPL path, so the
state also survives a crash or Ctrl-C instead of only being flushed on exit.

Add a regression test that runs `wallet new_address` in two consecutive REPL
processes and asserts the address index advances.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

REPL mode never persists wallet changes in the REPL loop

1 participant