added pmt builder considering the witnesses by julianlen · Pull Request #44 · rsksmart/pmt-builder

julianlen · 2026-03-30T19:07:45Z

No description provided.

github-actions · 2026-03-30T19:07:56Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA e578cf1.

Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

…ndex file

Copilot

Pull request overview

Adds support tooling to build Partial Merkle Trees using witness transaction IDs (wtxids), and extends the existing “registerBtcTransaction” helper to also return a PMT computed over wtxids.

Changes:

Added getWtxid helper for computing wtxids from raw transaction hex.
Added a new CLI tool to build a PMT using wtxids (tool/pmt-witness-builder.js).
Extended getInformationReadyForRegisterBtcTransaction to return an additional pmtConsideringWitness field and added a unit test for getWtxid.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
tool/pmt-witness-builder.js	New CLI script that builds a PMT using wtxids for a block containing a target tx.
tool/pmt-builder-utils.js	New helper for deriving wtxid from raw tx hex via `bitcoinjs-lib`.
tool/getInformationReadyForRegisterBtcTransaction.js	Now computes both txid-based PMT and wtxid-based PMT and returns both.
test/test.js	Adds unit coverage for `getWtxid` and updates require style.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tool/getInformationReadyForRegisterBtcTransaction.js

tool/pmt-builder-utils.js

Copilot · 2026-03-30T19:56:38Z

tool/pmt-builder-utils.js

@@ -0,0 +1,9 @@
+const bitcoin = require('bitcoinjs-lib');


This tool code depends on bitcoinjs-lib at runtime (via require('bitcoinjs-lib')), but bitcoinjs-lib is currently listed under devDependencies. If these tool/* scripts are intended to be usable from an installed package (without devDependencies), move bitcoinjs-lib to dependencies or document that tools require a dev install.

So far, all the code in /tool is just to use locally, not to be installed and used from another project. So, bitcoinjs-lib as a dev dependency is acceptable for now.

tool/pmt-witness-builder.js

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T16:39:56Z

tool/pmt-builder-utils.js

+    const tx = bitcoin.Transaction.fromHex(rawTx);
+    const wtxid = tx.getHash(true).reverse().toString('hex');
+    return wtxid;
+}


getWtxid is missing a trailing semicolon after the function expression. The rest of this module uses semicolons consistently, so this stands out and can cause style/lint inconsistencies.

Suggested change

}

};

tool/pmt-builder-utils.js

Copilot · 2026-04-01T16:39:57Z

tool/getInformationReadyForRegisterBtcTransaction.js

-
    const pmt = resultPmt.hex;
+    const blockHeight = transaction.status.block_height;
+    const rawTargetBtcTransaction = await getTransactionWithRetry(transactionsClient, transactionHash);


Same issue for the target transaction: rawTargetBtcTransaction can be null from getTransactionWithRetry, which will break getWtxid and also produce tx: 0xnull in the returned object. Handle the null case explicitly (e.g., throw with txid context) before computing targetWtxid / building the witness PMT.

Suggested change

const rawTargetBtcTransaction = await getTransactionWithRetry(transactionsClient, transactionHash);

const rawTargetBtcTransaction = await getTransactionWithRetry(transactionsClient, transactionHash);

if (rawTargetBtcTransaction == null) {

throw new Error(`Failed to fetch raw Bitcoin transaction for txid ${transactionHash}`);

}

Copilot · 2026-04-01T16:39:57Z

tool/getInformationReadyForRegisterBtcTransaction.js

 (async () => {
    try {
-
        const network = process.argv[2];
        const transactionHash = process.argv[3];



This module executes the CLI IIFE unconditionally. Importing/requiring it from another script will trigger network calls and console output as a side effect. Guard the CLI entry with if (require.main === module) (or move the CLI to a separate entry file) so the exported function can be reused safely.

so far, each file is an independent tool

Copilot · 2026-04-01T16:39:57Z

tool/pmt-witness-builder.js

+        const txid = blockTxIds[i];
+        const rawTx = await getTransactionWithRetry(transactionsClient, txid);
+        const wtxid = getWtxid(rawTx);
+        blockWtxids.push(wtxid);


getTransactionWithRetry can return null; calling getWtxid(rawTx) will throw and make it hard to diagnose which tx in the block failed to fetch. Add an explicit null check here (include the txid in the error) before computing/pushing the wtxid.

Copilot · 2026-04-01T16:39:57Z

tool/pmt-witness-builder.js

+    const rawTx = await getTransactionWithRetry(transactionsClient, txHash);
+    const targetTxWTxId = getWtxid(rawTx);
+    const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetTxWTxId);


Same null-handling issue for the target tx: rawTx can be null from getTransactionWithRetry, which will break getWtxid and PMT building. Handle the null case explicitly before computing targetTxWTxId / calling buildPMT.

Copilot · 2026-04-01T16:39:58Z

tool/getInformationReadyForRegisterCoinbaseBtcTransaction.js

 /**
 * Retrieves all transaction objects in a block by their transaction IDs.
 * @param {string[]} txIds - An array of txIds.
 * @returns {Promise<string[]>} An array of transactions.


The getAllTxs JSDoc is now inaccurate: it says it returns Promise<string[]>, but the function pushes bitcoinJs.Transaction.fromHex(tx) and returns an array of Transaction objects. Update the return type/doc to match the actual return value.

Suggested change

* @returns {Promise<string[]>} An array of transactions.

* @returns {Promise<import('bitcoinjs-lib').Transaction[]>} A promise that resolves to an array of bitcoinjs-lib Transaction objects.

Copilot · 2026-04-01T16:39:58Z

tool/getInformationReadyForRegisterCoinbaseBtcTransaction.js

        // mempool.js getTx method returns an object that directly contains 'wtxid'
-        const tx = await getTransactionWithRetry(txId);
+        const tx = await getTransactionWithRetry(transactionsClient, txId);


This inline comment is misleading: it references mempool.js getTx returning an object with wtxid, but the code here calls getTransactionWithRetry(...) which uses getTxHex and returns raw hex. Please update/remove the comment to reflect what’s actually happening.

Copilot · 2026-04-01T16:39:58Z

tool/pmt-witness-builder.js

+(async () => {
+    try {
+        const network = process.argv[2];
+        const txHash = process.argv[3];
+
+        const pmtInformation = await getPmtInformationWithWitness(network, txHash);
+
+        console.log('PMT information considering wtxid: ', pmtInformation);
+
+    } catch (e) {
+        console.log(e);
+    }
+})();


This file runs its CLI IIFE unconditionally. If it’s ever imported/reused programmatically, it will immediately perform network calls and write to stdout. Consider guarding the CLI entry with if (require.main === module) (or move the CLI into a dedicated entry file) to keep the module side-effect free on import.

Suggested change

(async () => {

try {

const network = process.argv[2];

const txHash = process.argv[3];

const pmtInformation = await getPmtInformationWithWitness(network, txHash);

console.log('PMT information considering wtxid: ', pmtInformation);

} catch (e) {

console.log(e);

}

})();

if (require.main === module) {

(async () => {

try {

const network = process.argv[2];

const txHash = process.argv[3];

const pmtInformation = await getPmtInformationWithWitness(network, txHash);

console.log('PMT information considering wtxid: ', pmtInformation);

} catch (e) {

console.log(e);

}

})();

}

Copilot · 2026-04-01T16:39:58Z

tool/pmt-witness-builder.js

+    const targetTxWTxId = getWtxid(rawTx);
+    const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetTxWTxId);


Variable naming is inconsistent here (targetTxWTxId). Since this value is a wtxid, consider renaming to something like targetWtxid for clarity and consistency with blockWtxids.

Suggested change

const targetTxWTxId = getWtxid(rawTx);

const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetTxWTxId);

const targetWtxid = getWtxid(rawTx);

const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetWtxid);

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

tool/getInformationReadyForRegisterCoinbaseBtcTransaction.js:106

Witness merkle roots per BIP141 treat the coinbase leaf’s wtxid as 32 bytes of 00..00 (not the actual coinbase wtxid). hashesWithWitness is currently built from tx.getHash(true) for every tx including coinbase, so the computed witnessMerkleRoot will not match the block’s witness commitment. Special-case index 0 to Buffer.alloc(32, 0) before building the merkle tree.

    // Calculate witnessRoot
    const hashesWithWitness = txs.map( x => Buffer.from(x.getHash(true)));
    const witnessMerkleTree = merkleLib(hashesWithWitness, bitcoinJs.crypto.hash256);
    // Get witness merkleRoot from witnessMerkleTree. This is equal to the last element in witnessMerkleTree array
    const witnessMerkleRoot = witnessMerkleTree[witnessMerkleTree.length-1].reverse();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T20:08:45Z

tool/getInformationReadyForRegisterCoinbaseBtcTransaction.js

 * Retrieves all transaction objects in a block by their transaction IDs.
+ * @param {Object} transactionsClient - Client instance used to fetch transaction data (must provide `getTxHex`).
 * @param {string[]} txIds - An array of txIds.
 * @returns {Promise<string[]>} An array of transactions.


The getAllTxs JSDoc says it returns Promise<string[]>, but the implementation pushes bitcoinJs.Transaction.fromHex(tx) and returns an array of Transaction objects. Please update the return type to match the actual return value (e.g., Promise<bitcoinjs-lib.Transaction[]>).

Suggested change

* @returns {Promise<string[]>} An array of transactions.

* @returns {Promise<bitcoinJs.Transaction[]>} An array of transactions.

Copilot · 2026-04-01T20:08:45Z

tool/pmt-witness-builder.js

+    const blockWtxids = [];
+    for (let i = 0; i < blockTxIds.length; i++) {
+        const txid = blockTxIds[i];
+        const rawTx = await getTransactionWithRetry(transactionsClient, txid);
+
+        if (!rawTx) {
+            throw new Error(`Failed to fetch transaction details for txId: ${txid}. It might not exist or is malformed.`);
+        }
+
+        const wtxid = getWtxid(rawTx);
+        blockWtxids.push(wtxid);
+
+        if (i < blockTxIds.length - 1) {
+            await sleep();
+        }


For the witness merkle tree (BIP141), the coinbase leaf must be the all-zero wtxid (32 bytes of 00..00). This loop currently computes and includes the actual coinbase wtxid, so the resulting witness PMT won’t correspond to the block’s witness commitment. Please special-case the first tx in the block (coinbase) to use the zero wtxid, and ensure targetWtxid is also zero when the target tx is coinbase.

…o modified the tests with real txs

jeremy-then · 2026-04-06T18:26:37Z

tool/pmt-builder-utils.js

+        return await transactionsClient.getTxHex({txid: txId});
+    } catch (error) {
+        // mempool.js might wrap the error, so we check for common indicators of 429
+        const isRateLimitError = error.response && error.response.status === 429;


Should we put this 429 in a constant? TOO_MANY_REQUEST_ERROR_CODE or something like that.

jeremy-then

LGTM. Just left some suggestions.

julia-zack · 2026-04-06T19:00:05Z

tool/pmt-builder-utils.js

+    try {
+        return await transactionsClient.getTxHex({txid: txId});
+    } catch (error) {
+        // mempool.js might wrap the error, so we check for common indicators of 429


Suggested change

// mempool.js might wrap the error, so we check for common indicators of 429

// mempool.js might wrap the error, so we check for common indicators of too many requests error code

tool/pmt-builder-utils.js

julia-zack · 2026-04-06T19:35:15Z

test/test.js

+    it('should return the txid as the wtxid for a block without witness transactions', async () => {
+        const blockTxids = ['2bf2c26cf756564f1d7f17f89882da32f20d694d9f2c8f04962c1116ccdf0bcf'];
+        const targetTxId = blockTxids[0];
+        const transactionsClient = createTransactionsClientMock();
+
+        const result = await getWtxids(transactionsClient, blockTxids, targetTxId);
+
+        expect(result.targetWtxid).to.equal(transactions[targetTxId].wtxid);
+        expect(result.blockWtxids.length).to.equal(1);
+        expect(result.blockWtxids).to.be.an('array').that.includes(transactions[targetTxId].wtxid);
+    });
+
+    it('should return the witness hash for a block with witness transactions', async () => {


test with an array bigger than one tx?
a block with both txs with and without witness?

I think intead of having 2 tests with 1 tx, I'll write 3 tests with many txs: 1 all with witness, 1 all without witness, and another mixed

that sounds good yes

julia-zack · 2026-04-06T19:37:52Z

test/test.js

+
+        const result = await getWtxids(transactionsClient, blockTxids, targetTxId);
+
+        expect(result.targetWtxid).to.equal(transactions[targetTxId].wtxid);


if the tx does not have witness shouldnt the wtxid be equal to the txid?

Suggested change

expect(result.targetWtxid).to.equal(transactions[targetTxId].wtxid);

expect(result.targetWtxid).to.equal(transactions[targetTxId].txid);

julia-zack · 2026-04-06T20:02:55Z

tool/pmt-witness-builder.js

+    const bitcoin = mempoolJS({
+        hostname: 'mempool.space',
+        network // 'testnet' | 'mainnet'
+    });
+
+    const transactionsClient = bitcoin.bitcoin.transactions;
+    const blocksClient = bitcoin.bitcoin.blocks;
+
+    const transaction = await transactionsClient.getTx({ txid: txHash });
+
+    const blockHash = transaction.status.block_hash;
+    const blockTxIds = await blocksClient.getBlockTxids({ hash: blockHash });
+
+    const { blockWtxids, targetWtxid } = await getWtxids(transactionsClient, blockTxIds, txHash);
+    const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetWtxid);
+    return resultPmt;


except for the getWtxids part, you have almost the exact same code in pmt-builder

Done. Also, I extracted shared behavior between the two getInformationReadyForRegister tools. However, I am not sure about having a method getBlockInfoByTransactionHash that returns an object with multiple elements. So consider this solution as an alternative. wdyt? aaf2d0f

…Retry

…s for consistency

julia-zack · 2026-04-07T15:12:28Z

tool/pmt-builder-utils.js

        const isRateLimitError = error.response && error.response.status === TOO_MANY_REQUESTS_ERROR_CODE;

-        // Sometimes the error message might contain clues if status is not directly 429
+        // Sometimes the error message might contain clues if status code is not directly 429


// Sometimes the error message might contain clues if status code is not the too many requests error code

…ss-builder reuse that method

…ed a method. However this method does things similar to getBlockTxidsByTransactionHash, so this last calls getBlockInfoByTransactionHash and obtains blockTxIds

julianlen requested a review from a team as a code owner March 30, 2026 19:07

added pmt builder considering the witnesses

252f877

julianlen force-pushed the witness-pmt-builder branch from 3c823ca to 252f877 Compare March 30, 2026 19:10

index is left as it was, getWtxid is more an utils than part of the i…

a033ac2

…ndex file

julianlen requested a review from Copilot March 30, 2026 19:53

Copilot started reviewing on behalf of julianlen March 30, 2026 19:53 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

renamings, deleted useless exports, moved shared methods into a utils

94d878e

julianlen requested a review from Copilot April 1, 2026 16:36

Copilot started reviewing on behalf of julianlen April 1, 2026 16:36 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

julianlen added 5 commits April 1, 2026 13:47

modified getTransactionWithRetry docjs and added semicolon to functions

f1dc68c

fixed getAllTxs jsdoc with transactionsClient

247f327

deleted unaccurate comment

2816e5b

renaming targetTxWTxId to targetWtxid

74c65da

added to fail if there is no rawTx

8a39ee6

julianlen requested a review from Copilot April 1, 2026 20:04

Copilot started reviewing on behalf of julianlen April 1, 2026 20:05 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

julianlen added 2 commits April 6, 2026 13:57

fixed README

ab4f395

Improved getWtxid and started using getWtxids to avoid repeating. Als…

59cb5d0

…o modified the tests with real txs

jeremy-then reviewed Apr 6, 2026

View reviewed changes

jeremy-then previously approved these changes Apr 6, 2026

View reviewed changes

extracted 429 to a constant

c01ecf4

julianlen dismissed jeremy-then’s stale review via c01ecf4 April 6, 2026 18:32

julia-zack reviewed Apr 6, 2026

View reviewed changes

julianlen added 3 commits April 7, 2026 11:24

getTransactionWithRetry will throw an error instead of returning null

8794e52

Modified comments and improved the error thrown by getTransactionWith…

765c82b

…Retry

renamed and reorder parameters in getInformationReadyForRegister file…

987ae3f

…s for consistency

julia-zack reviewed Apr 7, 2026

View reviewed changes

julianlen force-pushed the witness-pmt-builder branch 2 times, most recently from 58e0fe3 to 915b83a Compare April 7, 2026 15:43

Extracted getBlockTxIdsByTransactionHash so pmt-builder and pmt-witne…

db4801f

…ss-builder reuse that method

julianlen force-pushed the witness-pmt-builder branch from 915b83a to db4801f Compare April 7, 2026 16:02

julianlen added 2 commits April 7, 2026 13:09

both getInformationReadyForRegister shared the same setup, so extract…

aaf2d0f

…ed a method. However this method does things similar to getBlockTxidsByTransactionHash, so this last calls getBlockInfoByTransactionHash and obtains blockTxIds

modified tests to have blockIds with witness, without, and mixed

e578cf1

	* @returns {Promise<string[]>} An array of transactions.
	* @returns {Promise<import('bitcoinjs-lib').Transaction[]>} A promise that resolves to an array of bitcoinjs-lib Transaction objects.

		const targetTxWTxId = getWtxid(rawTx);
		const resultPmt = pmtBuilder.buildPMT(blockWtxids, targetTxWTxId);

	* @returns {Promise<string[]>} An array of transactions.
	* @returns {Promise<bitcoinJs.Transaction[]>} An array of transactions.

	// mempool.js might wrap the error, so we check for common indicators of 429
	// mempool.js might wrap the error, so we check for common indicators of too many requests error code


		const result = await getWtxids(transactionsClient, blockTxids, targetTxId);

		expect(result.targetWtxid).to.equal(transactions[targetTxId].wtxid);

Conversation

julianlen commented Mar 30, 2026

Uh oh!

github-actions bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Snapshot Warnings

Scanned Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

github-actions bot commented Mar 30, 2026 •

edited

Loading

julianlen Apr 7, 2026 •

edited

Loading