Skip to content

test(bitcoin-wallet-snap): repair the integration suite and cover template output order - #158

Draft
jeremytsng wants to merge 1 commit into
fix/btc-output-reordering-fx-failfrom
chore/repair-bitcoin-snap-integration-tests
Draft

test(bitcoin-wallet-snap): repair the integration suite and cover template output order#158
jeremytsng wants to merge 1 commit into
fix/btc-output-reordering-fx-failfrom
chore/repair-bitcoin-snap-integration-tests

Conversation

@jeremytsng

Copy link
Copy Markdown
Contributor

Explanation

Stacked on #157 — the base branch is fix/btc-output-reordering-fx-fail, because the regression
test added here fails without that fix. Retarget to main once #157 merges.

The integration suite has not run since the migration into this monorepo. Two defects stop it
before a single test executes:

  • run-integration.sh passes --config jest.integration.config.js, but the file is
    jest.integration.config.mjs. The archived snap-bitcoin-wallet repo had the .js name and a
    matching script; the rename came across, the script did not.
  • jest.integration.config.mjs declares no transform, so it falls back to the preset's
    babel-jest and requires @babel/preset-env, which is not installed anywhere in the monorepo.
    It now uses the same ts-jest transform as jest.config.mjs.

With those two fixed, the suite ran for the first time and 53 of 60 tests failed:

  • The assertions still expect the v1 keyring envelope, { pending: false, result }. The snap
    stopped returning it in 2.0.1 (onKeyringRequest returns Json directly). 11 assertions and
    2 result extractions updated.
  • lists account transactions required the funding transaction to be the only transaction on
    the chain. All four suites share one regtest chain, so any other suite's broadcast breaks it.
    It now asserts the funding transaction is present.

The suite is green after that: 4 suites, 54 passed, 7 skipped.

The regression test. fillPsbt had one assertion — psbt: expect.any(String) // non deterministic — which is why an output-reordering bug was invisible here. Only the change amount
is non-deterministic; the order is not. This adds:

  • keeps a wallet-owned output in its template position — a template of
    [deposit, ours(1000), OP_RETURN] must come back with all three in place and ours still worth
    1000, with the wallet's own change appended after. Verified to fail against the pre-fix
    behaviour, which returned [deposit, OP_RETURN, ours(999778558)] — relocated and given the
    excess.
  • An output-order assertion on the existing fills a PSBT successfully test. It asserts scripts
    only, because that template's last output belongs to the wallet and so legitimately becomes the
    drain output and takes the excess.

psbt-utils.ts builds and reads PSBTs with no dependencies, rather than importing
@metamask/bitcoindevkit — that package ships a bundler-target ESM wrapper around a .wasm file
that Jest will not resolve.

References

Follow-up to #157. Running the suite locally needs Docker:

yarn workspace @metamask/bitcoin-wallet-snap run build
yarn workspace @metamask/bitcoin-wallet-snap run test:integration

Not addressed here: the suite is referenced by no CI workflow, which is why the rot went
unnoticed. Wiring it in needs Docker on the runners and adds minutes to every build, so it is a
call for the owning team rather than this PR.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Changelog checkbox left unticked deliberately: these are test-only changes, which the repo's changelog guidelines say to omit as non-consumer-facing.

@jeremytsng
jeremytsng force-pushed the fix/btc-output-reordering-fx-fail branch 2 times, most recently from cd9168d to 0712819 Compare August 19, 2026 13:17
…plate output order

The integration suite has not run since the migration into this monorepo.
run-integration.sh passed jest.integration.config.js while the file is
jest.integration.config.mjs, and the config declared no transform, so it fell
back to the preset's babel-jest and required @babel/preset-env, which is not
installed. Point the script at the real filename and use the same ts-jest
transform the unit config uses.

With the suite running for the first time, 53 of 60 tests failed. The
assertions still expected the v1 keyring envelope, { pending: false, result },
which the snap stopped returning in 2.0.1, and one assertion required the
funding transaction to be the only transaction on a regtest chain that the
other suites broadcast to.

Add a regression test for template output order: a template whose middle
output belongs to the wallet must keep both its position and its value.
Before the ordering fix that output was moved to the end of the transaction
and given the excess. Assert output order on the existing fillPsbt test too,
which only checked that a string came back.
@jeremytsng
jeremytsng force-pushed the chore/repair-bitcoin-snap-integration-tests branch from cf4d066 to f938a3c Compare August 19, 2026 13:19
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.

1 participant