test(bitcoin-wallet-snap): repair the integration suite and cover template output order - #158
Draft
jeremytsng wants to merge 1 commit into
Conversation
jeremytsng
force-pushed
the
fix/btc-output-reordering-fx-fail
branch
2 times, most recently
from
August 19, 2026 13:17
cd9168d to
0712819
Compare
…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
force-pushed
the
chore/repair-bitcoin-snap-integration-tests
branch
from
August 19, 2026 13:19
cf4d066 to
f938a3c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Stacked on #157 — the base branch is
fix/btc-output-reordering-fx-fail, because the regressiontest added here fails without that fix. Retarget to
mainonce #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.shpasses--config jest.integration.config.js, but the file isjest.integration.config.mjs. The archivedsnap-bitcoin-walletrepo had the.jsname and amatching script; the rename came across, the script did not.
jest.integration.config.mjsdeclares notransform, so it falls back to the preset'sbabel-jest and requires
@babel/preset-env, which is not installed anywhere in the monorepo.It now uses the same
ts-jesttransform asjest.config.mjs.With those two fixed, the suite ran for the first time and 53 of 60 tests failed:
{ pending: false, result }. The snapstopped returning it in 2.0.1 (
onKeyringRequestreturnsJsondirectly). 11 assertions and2 result extractions updated.
lists account transactionsrequired the funding transaction to be the only transaction onthe 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.
fillPsbthad one assertion —psbt: expect.any(String) // non deterministic— which is why an output-reordering bug was invisible here. Only the change amountis 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 andoursstill worth1000, 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 theexcess.
fills a PSBT successfullytest. It asserts scriptsonly, because that template's last output belongs to the wallet and so legitimately becomes the
drain output and takes the excess.
psbt-utils.tsbuilds and reads PSBTs with no dependencies, rather than importing@metamask/bitcoindevkit— that package ships a bundler-target ESM wrapper around a.wasmfilethat Jest will not resolve.
References
Follow-up to #157. Running the suite locally needs Docker:
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
Changelog checkbox left unticked deliberately: these are test-only changes, which the repo's changelog guidelines say to omit as non-consumer-facing.