Skip to content

test: run the integration tests against a Tapyrus node - #30

Merged
azuchi merged 11 commits into
masterfrom
test/tapyrus_integration
Sep 30, 2026
Merged

azuchi merged 11 commits into
masterfrom
test/tapyrus_integration

Conversation

@Yamaguchi

@Yamaguchi Yamaguchi commented Sep 27, 2026 •

Copy link
Copy Markdown

Proposed body for PR #30

This is the current body with the minimum edits needed to match the code and to answer the review. Every changed passage is marked with a <!-- changed --> comment and can be found by searching for it; remove the comments before pasting.

Changes from the current body:

Where Change
Intro "Five commits, one purpose each." no longer matches; reworded
1. Drop the SegWit... HD test: "HD signing is still covered" corrected. The csv.spec.ts finalizer sentence corrected
2. Talk to a node unspents row removed (no such method). faucet now stops after a bounded number of blocks. Timeout is reported separately from an unreachable node
3. Coloured coins The Psbt finalizer paragraph replaced: the tests use Pstt now
4. CI integration.yml also runs on pull requests, and waits through a compose healthcheck
How to verify 3,140 → 3,159, and up -d --wait

Summary

test/integration/ pointed at https://regtest.bitbank.cc/1, the Bitcoin
regtest service bitcoinjs runs, and addresses.spec.ts also queried
blockchain.info. Nothing in it could pass, and since the SegWit removal
(#23) it did not even compile:

$ npx tsc -p test/tsconfig.json --noEmit
12 errors

which is why npm test stopped at build:tests with exit code 2.

This replaces the harness with a small JSON-RPC client for tapyrus-core, drops
what only made sense on Bitcoin, and adds coverage for coloured coins.

The commits are grouped by purpose below.

1. Drop the SegWit and Bitcoin cases

Eight tests in transactions.spec.ts (P2WPKH, P2WSH, P2SH(P2WSH) and their
nonWitnessUtxo variants), four address cases, the BIP49 derivation, the P2WSH
and P2SH(P2WSH) rounds in payments.spec.ts along with p2wpkh itself, the
P2WSH case in csv.spec.ts, and the blockchain.info lookup. The HD test was
kept and moved to P2PKH.

Pstt has no signInputHD, so that test wraps the derived child key in an
ECPair and calls signInput. bip32Derivation is only recorded on the input,
so the test checks an ordinary signature made with a BIP32-derived key, not HD
signing itself. It is named accordingly.

csv.spec.ts signs with Pstt and builds the final script by hand, and its
P2WSH case is gone.

Two things here are not SegWit but were wrong all the same:

  • The tests derived the broadcast txid from reverseBuffer(tx.getHash()). Since
    fix: refer to the hashMalFix txid in outpoints and fix coloured input handling #26 that is not what an outpoint or the node refers to. Now tx.getId().
  • blocks.spec.ts parsed a Bitcoin SegWit coinbase, which throws
    Transaction has unexpected data after fix: remove SegWit code paths that Tapyrus does not have #23. It now uses the coinbase of
    testnet block 896574be…de2a, and additionally asserts that Tapyrus repeats
    the height in the coinbase input's outpoint index.
  • The 1-to-1 Transaction example embedded a Bitcoin previous transaction with
    version = 2. Under the Tapyrus rules its outpoint no longer matches, so it
    was rebuilt at features = 1 and the expected hex regenerated.

2. Talk to a node

tapyrusjs-client is gone. Its RegtestUtils speaks the HTTP API of bitcoinjs'
regtest-server, not tapyrus-core's JSON-RPC, so pointing APIURL at a Tapyrus
node would not have worked. It was also pinned as
git+ssh://git@github.com/…, which makes npm ci fail wherever there is no SSH
key.

_regtest.ts now calls the node directly:

method RPC
broadcast sendrawtransaction
mine generatetoaddress nblocks address privkey
height getblockcount
fetch getrawtransaction txid true
faucet sendtoaddress then a block
faucetComplex fundrawtransaction → signrawtransactionwithwallet → sendrawtransaction

faucet locates its own output by scanning the transaction, because the
addresses under test are not in the node's wallet.

If the wallet cannot pay, it mines at most 500 blocks and then fails with an
error saying so, instead of looping until mocha's timeout.

Starting the node is deliberately not the test suite's job. Mixing the two makes
it hard to tell which one broke, and it rules out running against a node you
already have. docker-compose.integration.yml, test/integration/tapyrus.conf
and test/integration/README.md cover the setup instead, and the tests fail with
a message naming the command rather than hanging:

Cannot reach a Tapyrus node at http://127.0.0.1:12382. Start one with
`docker compose -f docker-compose.integration.yml up -d`
(see test/integration/README.md).

An RPC that runs past its timeout is reported as such, with the method name and
the timeout, not as an unreachable node.

The genesis block and the signer WIF are tapyrus-core's own dev mode example
from doc/docker_image.md. README.md says so and explains why a key has to be
in the repository at all: Tapyrus has no proof of work, so generatetoaddress
takes the aggregate key as an argument.

nobuild:coverage was limited to 'test/*.js', so npm test runs the unit
tests only. npm run integration is the way to run these.

3. Coloured coins

coloredcoins.spec.ts is new: it issues a token all three ways, transfers it,
burns it, and holds one in a CP2SH output. These are the paths a Bitcoin derived
library has no reason to get right, and until now nothing exercised them.

The NFT case also checks that the node refuses an amount other than 1
(bad-txns-nft-amount).

The CP2SH case also spends the held token to a CP2PKH address, so the node
checks the CP2SH signing path.

Every coloured transaction carries a TPC input for the fee; without one
tapyrus-core answers bad-txns-token-without-fee.

The transactions are built with Pstt. Its built-in Input Finalizer already
handles the coloured scripts, so the tests call finalizeAllInputs() and need
no finalizer of their own.

4. CI

With test/integration/ out of nobuild:coverage, npm test passes, so
ci.yml now runs it instead of lint and unit. That adds format:ci and the
90% coverage threshold to what a pull request has to pass, and CI checks what
CONTRIBUTING.md asks contributors to run. lint:tests is not part of
npm test and stays a step of its own. scripts/check.sh runs the same two.

.github/workflows/integration.yml runs on workflow_dispatch, on pull
requests, and on a push to master. It shows up as a check of its own next to
ci.yml, so a node that fails to start does not turn the unit test result red.
Keeping it out of the required checks also keeps it from blocking a merge. The
bugs it catches, such as the outpoint hash in #26 and the colour id checks in
#28, are better found before merging than after.

It uses the same compose file the README gives and waits with
docker compose up -d --wait, which relies on a healthcheck in that file
(tapyrus-cli getblockcount). tapyrus-core answers every RPC error with HTTP
500, the Loading block index it returns while warming up included, so a
reachable port is not a ready node. The same healthcheck covers the local run in
the README. The workflow prints the node log if anything fails.

Affected areas

test/integration/, package.json, .github/workflows/ and scripts/check.sh. No library code.

The compose file binds the RPC port to 127.0.0.1. The node holds nothing
worth stealing, but generatetoaddress and sendtoaddress are one user:pass
away, and there is no reason to offer those to whatever network the development
machine is on.

How to verify

Unit tests, format and lint, the same as CI:

npm ci
./scripts/check.sh   # 3,159 passing, coverage above 90%

Integration tests:

docker compose -f docker-compose.integration.yml up -d --wait
npm run integration
docker compose -f docker-compose.integration.yml down

docker compose down throws the chain away, so each run starts at the same
height with an empty wallet. No test needs that — each one funds a newly
generated key, so its colour identifiers are new every time — but a chain that
keeps growing makes a failure harder to read, and the image ignores
GENESIS_BLOCK_WITH_SIG once a genesis file exists, so a kept volume would
outlive a change to the genesis block.

What was checked without a node

The issuance, transfer, burn and CP2SH transactions were built offline and
inspected: features is 1, the coloured outputs classify as
coloredpubkeyhash and coloredscripthash, and the fee comes out at the
intended 10,000 satoshi. Acceptance by a node is what the workflow above is for.

Base branch

Stacked on feat/color_identifier, whose coloridentifier module the coloured
coin tests use. Please merge that first; GitHub retargets this PR at master
when it does.

@Yamaguchi
Yamaguchi force-pushed the test/tapyrus_integration branch from be0c193 to 3c91d69 Compare September 27, 2026 15:21
@Yamaguchi
Yamaguchi requested a review from azuchi September 28, 2026 00:44

@azuchi azuchi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

マージを妨げる問題は見つかりませんでした。今回は手元で実際に Tapyrus ノードを起動して確認しています。

検証項目 結果
npm run integration(tapyrus/tapyrusd:v0.7.2、新規チェーン) 39 件パス
同じチェーンのまま 2 回目 39 件パス
ノードを起動せずに実行 ハングせず即座に失敗、メッセージも明確
./scripts/check.sh 3,138 件パス、カバレッジは全指標で 90% 超(分岐 91.97%)
実行後の作業ツリー 差分なし

一番お願いしたいのは、統合テストのワークフローをプルリクエストでも走らせることです(integration.yml のインラインコメントに理由を書きました)。それ以外は、テストの検出力とドキュメントの不整合についての非ブロッキングな指摘です。

diff に現れない箇所で 2 点あります。

ルートの README.md の例一覧が古い(93〜109 行目)
統合テストへのリンク一覧が、この PR で削除・改名したテストを指したままです。「Support the retrieval of transactions for an address (3rd party blockchain)」と「Create a BIP49, bitcoin testnet, …」は削除され、「broadcast via 3PBP」は「broadcast to a node」に改名されています。coloredcoins.spec.ts の例も一覧にありません。

PR 本文が後続コミットに追従していない

  • 「Psbt は彩色スクリプトを finalize できないので、transfer と burn は独自の finalizer を finalizeInput に渡す」とありますが、実装は Pstt に移行済みで、組み込みの finalizer を finalizeAllInputs() で使っています。
  • 対応表の unspents → listunspent は RegtestUtils に存在しません。
  • テスト件数は 3,140 ではなく 3,138 でした。

on:
workflow_dispatch:
push:
branches: [master]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

プルリクエストでも走るようにしてください。 pull_request をトリガーに加えるのがよいと思います。

on:
  workflow_dispatch:
  pull_request:
  push:
    branches: [master]

理由は次のとおりです。

  • 検出したい不具合は、マージ前に見つけたい種類のものです。 README にあるとおり、統合テストは「ユニットテストは通るがノードが拒否する」誤りを捕まえる唯一の手段です。このシリーズで直してきた outpoint の hashMalFix(fix: refer to the hashMalFix txid in outpoints and fix coloured input handling #26)や colorId の検証(fix: validate the type byte of colour identifiers #28)がまさにその類いで、master に入ってから赤くなるのでは、原因のコミットを特定して revert する手間が増えます。
  • この PR 自身が、CI 上で一度も統合テストを実行できていません。 現在のトリガーでは master への push か手動実行でしか動かないので、ワークフローの YAML が正しいかどうかもマージするまで分かりません。
  • 「無関係な理由でプルリクエストが赤くなる」懸念は、ワークフローが分かれていることで解消できます。 ci.yml とは別のチェックとして表示されるので、ユニットテストの結果は汚れません。ノードの起動失敗で止めたくなければ、ブランチ保護の required checks に含めなければ済みます。
  • コストは小さいです。 手元ではノードは起動から 2 秒で RPC に応答し、39 件のテストは約 1 秒で終わりました。シークレットも不要なので、fork からのプルリクエストでもそのまま動きます。

冒頭のコメント(3〜5 行目)も、あわせて書き換えが必要です。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4ae6245 で修正してます。

Comment thread test/integration/cltv.spec.ts Outdated
@@ -258,7 +255,7 @@ describe('bitcoinjs-lib (transactions w/ CLTV)', () => {
await regtestUtils.broadcast(tx.toHex()).catch(err => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

拒否を期待するテストですが、検証が .catch(...) の中にしかないので、ブロードキャストが成功した場合は何も assert せずに合格します。期限前の CLTV 償還がノードに受理されるような退行が起きても、このテストは緑のままです。csv.spec.ts:231 の non-BIP68-final も同じ形です。

この PR で正規表現を書き換えた行なので、coloredcoins.spec.ts の NFT のテストと同じ形に揃えるのがよいと思います。

await assert.rejects(
  regtestUtils.broadcast(tx.toHex()),
  /non-final \(code 64\)/,
);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31a5480 で修正してます。cltv.spec.ts および、同じ形の csv.spec.ts(non-BIP68-final)を変更してます。
どちらも assert.rejects(regtestUtils.broadcast(tx.toHex()), /…/) にしたので、ブロードキャストが成功するとテストが失敗します。

Comment thread test/integration/transactions.spec.ts Outdated
Buffer.from(childNode.privateKey!),
{ network: regtest },
);
pstt.signInput(0, childKeyPair);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントにあるとおり Pstt には signInputHD がないため、導出済みの子鍵を ECPair に包み直して signInput しています。bip32Derivation は入力に記録されるだけで署名時には参照されないので、パスやフィンガープリントが誤っていてもこのテストは通ります。

つまり検証しているのは「HD で導出した鍵による通常の署名」で、HD 署名そのものではありません。テスト名の「using HD」と PR 本文の「HD signing is still covered」は実態と合わないので、テスト名を実態に合わせる(例: w/ a P2PKH input whose key is derived with BIP32)か、Pstt に HD 署名の手段を用意してから検証するかのどちらかがよいと思います。後者は別 PR で構いません。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31a5480 で修正しました。
1 つ目の案を採用し、テスト名を w/ a P2PKH input whose key is derived with BIP32 に変えました。

private async ensureFunds(value: number): Promise<void> {
const needed = (value + 1e6) / 1e8;
while ((await this.rpc<number>('getbalance')) < needed) {
await this.mine(1);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このループには上限がありません。README は既存ノードでの実行を案内していますが、ウォレットの残高が増えないノード(ウォレットが無効、別のアドレスに採掘される、など)では無限ループになり、原因を示さないまま mocha のタイムアウトで落ちます。試行回数に上限を設けて、超えたら「N ブロック採掘しても残高が X に届かない」というエラーにすると、原因が分かります。

手元の新規チェーンでは全 39 件が約 1 秒で終わったので、速度面の問題は確認していません。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31a5480 で直しました。ensureFunds は最大 500 ブロックまでmineするようにしました。それでも残高が足りなければ、現在の残高と必要額を示し、考えられる原因(Blocks may be paying an address the node wallet does not hold, or the wallet may be disabled.)を添えたエラーで止まります。

}),
});
} catch (err) {
throw new Error(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetch の例外をすべて「Cannot reach a Tapyrus node … Start one with docker compose」に変換しているので、AbortSignal.timeout によるタイムアウトも未起動として報告されます。ノードは動いているが RPC が 20 秒を超えた、という場合に原因を取り違えます。err.name === 'TimeoutError' を分けて、メソッド名とタイムアウト値を出すのがよいと思います。

なお未起動時の挙動は手元で確認しました。ハングせず即座に失敗し、メッセージも明確でした。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31a5480 で直しました。TimeoutError は別に扱い、<method> did not answer within 20000ms. The node at <url> is reachable but not responding in time. と報告するようにしてます。それ以外の失敗は、従来どおり「Cannot reach a Tapyrus node」のメッセージです。

# in test/integration/_regtest.ts opens this chain by design.
services:
tapyrusd:
image: tapyrus/tapyrusd:v0.7.2

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

起動待ちのロジックが integration.yml の curl ループにしかなく、README の手順(up -d の直後に npm run integration)では、ノードが Loading block index... を返している間にテストが始まる可能性があります。ここに healthcheck を定義して docker compose up -d --wait を使うと、待機がローカルと CI で一本化され、ワークフロー側のループも不要になります。

    healthcheck:
      test: ['CMD', 'tapyrus-cli', '-conf=/etc/tapyrus/tapyrus.conf', 'getblockcount']
      interval: 1s
      timeout: 3s
      retries: 60

手元では起動から 2 秒で応答したので、実害が出る場面は限られます。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4564efc で修正しました。
docker-compose.integration.yml にhealthcheckの定義を追加して、ci側では--wait で待機するようにしました。

Comment thread test/integration/coloredcoins.spec.ts Outdated
assert.strictEqual(burned.outs[0].token, 'TPC');
});

it('can hold a token in a CP2SH output', async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このテストは CP2SH の出力にトークンを保持するところまでで、そこから使用していません。#26 で直した CP2SH の署名経路を実ノードで確かめられる好機なので、保持したトークンを別のアドレスへ送るところまで含めると、テストの価値が上がると思います。Pstt の finalizer が CP2SH に対応していない場合は、その旨をコメントに残すだけでも構いません。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

31a5480 で対応しました。テスト名は can hold a token in a CP2SH output, and spend it に変更してます。また、保持したトークンを、手数料を払う TPC の入力とあわせて、CP2PKH のアドレスへ送るようにしてます。

Comment thread .github/workflows/ci.yml Outdated
run: npm run lint:tests
- name: Run unit test
run: npm run unit
# npm run unit builds first, so src/ and types/ are up to date here

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このコメントは npm run unit を前提にしていますが、そのステップはこの PR で削除されています。git diff --exit-code src types の前提になるビルドは、いまは npm test の中の npm run build が担っているので、コメントをそれに合わせて更新してください。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4ae6245 でコメントを修正してます。

docker compose -f docker-compose.integration.yml down
```

The node listens on `127.0.0.1:12382`. If nothing is listening there, every

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「every test fails」とありますが、ノードを起動せずに実行したところ、ノードを使わない 12 件(addresses、bip32、blocks の各 spec と「can create a 1-to-1 Transaction」)は成功し、残りが失敗しました。「ノードを必要とするテストは」のような書き方が正確です。

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4564efc で修正しました。

@Yamaguchi
Yamaguchi force-pushed the feat/color_identifier branch from 6ce7f2a to a87b2a4 Compare September 29, 2026 11:33
@Yamaguchi

Yamaguchi commented Sep 29, 2026 •

Copy link
Copy Markdown
Author

ルートの README.md の例一覧が古い(93〜109 行目) 統合テストへのリンク一覧が、この PR で削除・改名したテストを指したままです。「Support the retrieval of transactions for an address (3rd party blockchain)」と「Create a BIP49, bitcoin testnet, …」は削除され、「broadcast via 3PBP」は「broadcast to a node」に改名されています。coloredcoins.spec.ts の例も一覧にありません。

README.mdを 4564efc で修正しました。

@Yamaguchi
Yamaguchi requested a review from azuchi September 30, 2026 01:32
@azuchi

azuchi commented Sep 30, 2026

Copy link
Copy Markdown

更新分を確認しました。前回の指摘はすべて対応されており、内容面ではマージ可能です。CI の統合テストのジョブがプルリクエストで実際に走って 39 件パスしていること、手元でも新しい compose 定義(healthcheck + up --wait)でノードが 2 秒で healthy になり、39 件が 2 回続けてパスすること、./scripts/check.sh が 3,159 件パスすることを確認しました。

マージ順についてお願いです。 ベースの #29 は、#28 がリベースマージされて別 SHA になった影響で master と競合しており(src/metadata.js)、master へのリベースが必要です。このブランチは #29 の旧コミットと origin/feat/color_identifier の取り込みマージを履歴に含んでいるので、#29 をリベースしてマージした後に、こちらも master へリベースしてからマージしてください。#29 の旧コミットはパッチが同一なのでリベース時に自動でスキップされ、内容の競合は見込まれません。

@Yamaguchi
Yamaguchi force-pushed the feat/color_identifier branch from a87b2a4 to 591b689 Compare September 30, 2026 07:34
Base automatically changed from feat/color_identifier to master September 30, 2026 08:01
@Yamaguchi
Yamaguchi force-pushed the test/tapyrus_integration branch from 45bb88b to a818ffd Compare September 30, 2026 08:52
@azuchi
azuchi merged commit 3ddb968 into master Sep 30, 2026
7 checks passed
@azuchi
azuchi deleted the test/tapyrus_integration branch September 30, 2026 09:04
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.

2 participants