mirror of
https://github.com/PR0M3TH3AN/bitvid.git
synced 2026-10-27 07:52:24 +00:00
fix(sync): empty wallet save must not wipe the synced copy on relays
Data-loss bug: on a fresh device the user opened the Wallet pane (field empty,
wallet not yet restored), clicked Save, and the "removed" branch called
walletSync.disable() — publishing a {cleared:true} note that destroyed the good
encrypted copy another device relied on. (Confirmed in logs: a kind-30078
"bitvid:nwc" {cleared:true} event signed right after login.)
An empty save happens routinely (saving before hydration/restore), so treating it
as a deliberate disconnect-that-wipes-the-remote is wrong. Removed the auto-clear
from the empty-save branch: the remote synced note is now changed ONLY via the
explicit sync toggle (enable/disable) or a successful save (push). An empty save
clears just the LOCAL wallet.
tests/wallet-sync-no-clobber.test.mjs: empty save does not call disable/push; a
real save still pushes. Mutation-verified (re-adding the disable fails the test).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
1 parent
234f57e009
commit
c26ef89a93
2 files changed
+93
-12
No files matched your search
@@ -631,18 +631,13 @@ export class ProfileWalletController {
|
||||
finalVariant = "info";
|
||||
this.mainController.showStatus("Wallet connection removed.");
|
||||
context.reason = "cleared";
|
||||
// Don't leave a now-removed spending URI lingering (encrypted) on relays.
|
||||
const sync = this.getWalletSyncService();
|
||||
if (sync.isEnabled(normalizedActive)) {
|
||||
try {
|
||||
await sync.disable(normalizedActive);
|
||||
} catch (syncError) {
|
||||
devLogger.warn(
|
||||
"[ProfileModal] Wallet sync clear after disconnect failed:",
|
||||
syncError
|
||||
);
|
||||
}
|
||||
}
|
||||
// NOTE: deliberately do NOT clear the remote synced note here. An empty
|
||||
// save happens routinely (e.g. saving before the wallet has hydrated /
|
||||
// been restored on a fresh device), and wiping the encrypted note on
|
||||
// relays would destroy a good copy that another device relies on. The
|
||||
// remote note is only ever changed via the explicit sync toggle
|
||||
// (enable/disable) or a successful save (push). Toggling sync off is the
|
||||
// single, deliberate way to clear the remote.
|
||||
}
|
||||
context.success = true;
|
||||
} catch (error) {
|
||||
|
||||
@@ -0,0 +1,86 @@
|
||||
// Regression (data-loss): saving the Wallet pane with an EMPTY URI must NOT wipe
|
||||
// the encrypted synced copy on relays. This happened on a fresh device — the user
|
||||
// opened the pane (field empty, wallet not yet restored), clicked Save, and the
|
||||
// "removed" branch called walletSync.disable() which published a {cleared:true}
|
||||
// note, destroying the good remote copy another device relied on.
|
||||
//
|
||||
// Correct behavior: an empty save only clears the LOCAL wallet; the remote synced
|
||||
// note is changed solely via the explicit sync toggle (enable/disable) or a
|
||||
// successful save (push).
|
||||
|
||||
import "./test-helpers/setup-localstorage.mjs";
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
import { ProfileWalletController } from "../js/ui/profileModal/ProfileWalletController.js";
|
||||
|
||||
const PUBKEY = "a".repeat(64);
|
||||
|
||||
function makeController() {
|
||||
const calls = { disable: 0, push: 0 };
|
||||
const mainController = {
|
||||
normalizeHexPubkey: (v) => (typeof v === "string" ? v : ""),
|
||||
getActivePubkey: () => PUBKEY,
|
||||
showSuccess: () => {},
|
||||
showStatus: () => {},
|
||||
showError: () => {},
|
||||
callbacks: { onWalletSave: () => {} },
|
||||
services: { nwcSettings: {} },
|
||||
};
|
||||
const controller = new ProfileWalletController(mainController);
|
||||
|
||||
// Stub the instance methods handleWalletSave depends on (no real DOM/services).
|
||||
controller.isWalletBusy = () => false;
|
||||
controller.getWalletFormValues = () => ({ uri: "", defaultZap: null, error: null });
|
||||
controller.validateWalletUri = () => ({ valid: true, sanitized: "", message: "" });
|
||||
controller.persistWalletSettings = async () => ({});
|
||||
controller.setWalletPaneBusy = () => {};
|
||||
controller.updateWalletStatus = () => {};
|
||||
controller.refreshWalletPaneState = () => {};
|
||||
|
||||
// Sync is opted-in on this device (the dangerous precondition), with spies.
|
||||
controller.getWalletSyncService = () => ({
|
||||
isEnabled: () => true,
|
||||
disable: async () => {
|
||||
calls.disable += 1;
|
||||
return { ok: true };
|
||||
},
|
||||
push: async () => {
|
||||
calls.push += 1;
|
||||
return { ok: true };
|
||||
},
|
||||
});
|
||||
|
||||
return { controller, calls };
|
||||
}
|
||||
|
||||
test("empty save does NOT clear the remote synced wallet note", async () => {
|
||||
const { controller, calls } = makeController();
|
||||
const result = await controller.handleWalletSave();
|
||||
|
||||
assert.equal(result.reason, "cleared", "empty save takes the removal branch");
|
||||
assert.equal(
|
||||
calls.disable,
|
||||
0,
|
||||
"an empty save must NOT call walletSync.disable() (no remote {cleared} publish)",
|
||||
);
|
||||
assert.equal(calls.push, 0, "an empty save must not push either");
|
||||
});
|
||||
|
||||
test("a real save with a URI DOES push the synced copy (sanity)", async () => {
|
||||
const { controller, calls } = makeController();
|
||||
controller.getWalletFormValues = () => ({
|
||||
uri: "nostr+walletconnect://abc",
|
||||
defaultZap: null,
|
||||
error: null,
|
||||
});
|
||||
controller.validateWalletUri = () => ({
|
||||
valid: true,
|
||||
sanitized: "nostr+walletconnect://abc",
|
||||
message: "",
|
||||
});
|
||||
|
||||
const result = await controller.handleWalletSave();
|
||||
assert.equal(result.reason, "saved");
|
||||
assert.equal(calls.push, 1, "a real save re-pushes when sync is enabled");
|
||||
assert.equal(calls.disable, 0, "a real save never clears the remote");
|
||||
});
|
||||
Reference in new issue
Block a user