batch waitForConfirmation is broken two ways: wrong target height, and it calls a method that does not exist

[WiktorStarczewski]

BatchOptions.waitForConfirmation does not do what it says, for two independent reasons. Both are pre-existing; surfacing them here because #313 corrects the documented meaning of the number the option polls against, which is what makes the first one visible.

1. It waits on a height the client has usually already reached

#waitForBlock polls until getSyncHeight() >= blockNumber, where blockNumber is what submitNewTransactionBatch returned. That value is the node's chain tip as of submission — upstream BatchBuilder::submit says so explicitly:

Returns the node's chain tip at submission (not the block the batch is committed). The submitted transactions are recorded locally as pending; call sync_state to get the block they commit in.

The batch commits at some height strictly greater than that. So the loop can exit after a single sync, before the batch has landed, and report confirmation for a block that already existed.

The strongest evidence is in our own test suite: test/batch.browser.test.ts does not trust the returned number at all. It polls the sender's nonce for up to 120s to decide the batch landed.

Note that js/__tests__/resources/transactions.test.js has a case named "polls sync height until block lands" which feeds heights [99, 99, 100] against a returned 100. It encodes the wrong premise and passes only because the mock is free to lag — it should be updated with whatever fix lands here.

2. It calls a method that does not exist

#waitForBlock calls syncStateWithTimeout, which is not a method on the client. So waitForConfirmation: true on a batch throws before the polling logic above ever matters.

Suggested direction

The V1 batch API returns only a block number, and no batch or per-transaction identifier, so there is nothing to poll per-transaction the way singular waitFor does. Options, roughly in order of preference:

1. Have the Rust side return something identifying — a batch id, or the transaction ids it built — and poll on that.
2. Wait for height > blockNumber plus a store-side check that the account state actually advanced.
3. If neither is feasible under V1, remove the option rather than ship one that cannot work, and document nonce / transactions.list() polling as the way to observe a batch.

Whichever way it goes, BatchOptions.waitForConfirmation and #waitForBlock need docstrings that match. #313 has already reworded them to describe the current (weak) guarantee rather than the intended one.

[WiktorStarczewski]

Found while reviewing #313: the poll is not merely weaker than its name suggests, it cannot advance the height on its own.

#waitForBlock calls this.#inner.syncStateWithTimeout(0) (crates/web-client/js/resources/transactions.js). That method does not exist — it is not a WebClient member, not in SYNC_METHODS/READ_METHODS/WRITE_METHODS, and not a Rust export (the exports are syncStateImpl, syncChainImpl, syncNoteTransportImpl). The proxy therefore yields undefined, calling it throws a TypeError, and the bare catch {} immediately below swallows it. The loop then spins on getSyncHeight() without ever syncing, so it succeeds only if something else — an auto-sync tick, another caller — happens to catch the client up inside the timeout. Otherwise it runs the full 60s and throws Batch confirmation timed out.

Two things kept this invisible:

1. The unit test invents the method. js/__tests__/resources/transactions.test.js builds its fake inner client with syncStateWithTimeout: vi.fn() and then asserts expect(inner.syncStateWithTimeout).toHaveBeenCalled(). It passes precisely because the mock supplies what production lacks, so it can never fail on this. Worth fixing alongside — a test asserting a call to a nonexistent method is worse than no test.
2. batch.browser.test.ts polls the account nonce for 120s instead of using the option, so the live-network path never exercised it either.

Two names look plausible as the intended target: syncState() (takes the sync lock) or a non-blocking variant that was planned but never landed — the docblocks in js/index.js and js/client.js both reference syncStateWithTimeout as though it exists, which suggests the latter.

#313 does not touch this code, but it did rewrite the surrounding prose, so it now documents the option as broken and its examples no longer recommend it.

[kutluhaneth46]

Working the syncStateWithTimeout half via #320 (same root cause as the confirmation wait never driving sync). Will keep the wrong-target-height piece separate unless it falls out cleanly in the same change.