XBullAdapter leaks an account-change listener on every reconnect, and disconnect() removes only the last one

[woahwhattheheck]

Description

XBullAdapter.setupAccountChangeListener() overwrites this.unsubscribe without calling the previous one, so every call to connect() registers an additional account-change listener with xBull and orphans the one before it. disconnect() then invokes only the most recent unsubscribe, leaving the earlier listeners live.

The sharp consequence is not the leak itself but what the orphans keep doing: each one still holds this and still runs this.currentPublicKey = publicKey. So after disconnect() has deliberately set currentPublicKey = null, the next account-change event from the wallet writes an address straight back into the adapter the app believes it has torn down.

connect() being called more than once is ordinary usage — reconnect after a dropped session, an account switch, or a component remount that re-runs connection setup.

The other two adapters already handle this

Both siblings state the intended behaviour, in two different ways:

FreighterAdapter.startAccountChangePolling() refuses to double-register:

private startAccountChangePolling(): void {
    if (this.pollInterval) return;

LobstrAdapter.setupAccountChangeListener() removes the previous registration before installing a new one:

if (this.accountChangedHandler && typeof window.lobstr.off === "function") {
    window.lobstr.off("accountChanged", this.accountChangedHandler);
}

XBullAdapter does neither, and it is the only one of the three that does not.

Reproduce

Driving the real XBullAdapter against a window.xbull double that counts its live listeners:

one connect                          -> 1 live listener        (expected 1)
three connects, no disconnect        -> 3 live listeners       (expected 1)
disconnect()                         -> 2 live listeners       (expected 0)
wallet emits accountChanged after disconnect
    adapter.currentPublicKey         -> "GC-SOME-OTHER-ADDRESS"  (expected null)

The last line is the one that matters: disconnect() nulls currentPublicKey, and a leaked listener puts an address back.

Suggested fix

Mirror LobstrAdapter — drop the previous registration before taking a new one, so at most one is ever live:

private setupAccountChangeListener(): void {
    if (!window.xbull) return;

    if (this.unsubscribe) {
      this.unsubscribe();
      this.unsubscribe = null;
    }

    this.unsubscribe = window.xbull.onAccountChange((publicKey: string) => {
      // unchanged
    });
}

disconnect() then needs no change: with one live listener its existing unsubscribe is sufficient.

Acceptance criteria

• Repeated connect() calls leave at most one live xBull account-change listener
• After disconnect(), no listener remains registered with the wallet
• An account-change event delivered after disconnect() does not modify adapter state
• FreighterAdapter and LobstrAdapter behaviour unchanged
• Unit tests pass

Context

• Target file: src/wallets/adapters/XBullAdapter.ts
• Related: src/wallets/adapters/LobstrAdapter.ts and src/wallets/adapters/FreighterAdapter.ts, which both already avoid this

Per CONTRIBUTING I'm not starting on it unassigned — I'd like to work on this, and I have the fix and regression tests ready to push if a maintainer assigns it to me.

[Iker2522]

I'd like to take this issue on!
​I can fix the listener accumulation bug in XBullAdapter.setupAccountChangeListener() in src/wallets/adapters/XBullAdapter.ts.
​I will update setupAccountChangeListener() to call and clear this.unsubscribe before attaching a new onAccountChange listener, bringing it into parity with LobstrAdapter. This ensures that repeated connect() calls leave at most one active listener, and that calling disconnect() cleanly unsubscribes the remaining listener so subsequent wallet events cannot overwrite currentPublicKey.
​I will also add unit tests verifying single-listener bounds across multiple connections and confirming adapter state remains null post-disconnect. All existing unit tests will pass cleanly. PR ready in 1 day.

[woahwhattheheck]

@Iker2522 — that's exactly the right fix, and it matches what I outlined at the bottom of the report: drop the previous registration before taking a new one, the way LobstrAdapter already does.

One thing so you don't spend time duplicating it: I opened this issue and said in it that I have the fix and regression tests ready to push if a maintainer assigns it to me. So there are two of us with the same change in hand.

I don't want to sit on it if you'd rather do it — maintainers, please assign whichever of us you prefer and I'll be glad either way. @Iker2522, if it goes to you, the cases worth keeping are: repeated connect() leaves at most one live listener; nothing remains registered after disconnect(); and an account-change event delivered after disconnect() does not write currentPublicKey back. That last one is the reason the bug matters rather than just being untidy, and it's the one a test is most likely to miss.

[JohnArayaE]

I would like to work on this issue because repeated connect() calls in XBullAdapter can leave orphaned account-change listeners active, allowing wallet events to mutate currentPublicKey even after the adapter has been explicitly disconnected. I can align the xBull behavior with the existing LobstrAdapter pattern by unsubscribing any previous listener before registering a new one, ensuring that only one listener is ever active at a time and that the existing disconnect() cleanup remains sufficient. I can also add focused regression tests covering repeated connects, listener cleanup after disconnect, and post-disconnect account-change events, while keeping the Freighter and Lobstr adapter behavior unchanged.

[xtep103]

Hi there, I’ve carefully reviewed the issue details and I’m confident I can provide a clean, efficient, and maintainable solution.

[guptakumarranjeet150]

Hello @maintainers,

I would like to resolve this issue. Here is my proposed execution plan:

• Implementation: Deliver a clean, modular solution for XBullAdapter leaks an account-change listener on every reconnect, and disconnect() removes only the last one strictly following Stellar-split/split-sdk conventions.
• Testing & Verification: Cover changes with automated tests ensuring CI passes smoothly.
• Delivery: Ready to ship a thoroughly tested PR within 12–24 hours with passing CI.

Excited to work on this—please assign to me and I'll submit a clean PR shortly!