bcoin 5-MOAD scan: 1 defect (bcoin-0001 gettxoutproof O(H*T) linear scan, 48x)
This commit is contained in:
parent
65389dd651
commit
2002c492a0
3 changed files with 209 additions and 0 deletions
55
defects/bcoin/SCAN-NOTES.md
Normal file
55
defects/bcoin/SCAN-NOTES.md
Normal file
|
|
@ -0,0 +1,55 @@
|
||||||
|
# bcoin 5-MOAD Scan
|
||||||
|
|
||||||
|
Target: bcoin (JavaScript Bitcoin full node implementation)
|
||||||
|
Source: ~/git/bcoin
|
||||||
|
Date: 2026-03-31
|
||||||
|
|
||||||
|
## MOAD-0001 (CWE-407): 1 DEFECT
|
||||||
|
|
||||||
|
### bcoin-0001: RPC gettxoutproof Block.hasTX linear scan O(H*T)
|
||||||
|
|
||||||
|
- File: lib/node/rpc.js:1035-1040
|
||||||
|
- Pattern: `for (hash of hashes) { block.hasTX(hash) }` where hasTX calls
|
||||||
|
Block.indexOf which linearly scans block.txs
|
||||||
|
- Complexity: O(H*T) where H = user-provided txids, T = transactions in block
|
||||||
|
- Severity: LOW-MEDIUM (RPC method, auth-gated, H typically small)
|
||||||
|
- Ratio: 48.3x at T=4000, H=100
|
||||||
|
- Fix: Build BufferSet from block.txs before loop, O(H+T)
|
||||||
|
|
||||||
|
Other indexOf/includes sites examined and cleared:
|
||||||
|
- rpc.js:1440 deps.indexOf(dep): deps is per-tx, bounded by input count (1-5 typical)
|
||||||
|
- rpc.js:1482/1491/1579 rules.indexOf(name): rules is user-provided deploy names (handful)
|
||||||
|
- descriptor/*.js .includes(): constant enum arrays (2-3 elements)
|
||||||
|
- block.js:269 hasTX(hash): only called in loop at rpc.js:1035 (patched above)
|
||||||
|
- merkleblock.js:118 hasTX: uses Map internally, O(1)
|
||||||
|
- mtx.js:597 prev.indexOf(ring.publicKey): Script.indexOf, single call per ring
|
||||||
|
- All net/pool, mempool, blockchain, wallet, mining use BufferSet/BufferMap/Map
|
||||||
|
|
||||||
|
## MOAD-0002 (Intertangle): CLEAN
|
||||||
|
|
||||||
|
Node base class has chain/mempool/pool/miner properties, but they are assembled
|
||||||
|
by FullNode constructor with proper dependency injection. Each subsystem receives
|
||||||
|
its dependencies through constructor options (chain passed to mempool, mempool
|
||||||
|
passed to pool). Standard composition pattern, not shared mutable global state.
|
||||||
|
|
||||||
|
## MOAD-0003 (Leaked Context): CLEAN
|
||||||
|
|
||||||
|
No AsyncLocalStorage, cls-hooked, or domain-based context usage found anywhere
|
||||||
|
in the codebase. Request context is handled through explicit req/res parameters
|
||||||
|
in HTTP handlers.
|
||||||
|
|
||||||
|
## MOAD-0004 (Logged Secret): CLEAN
|
||||||
|
|
||||||
|
Auth logging in node/http.js and wallet/http.js only logs socket.host and
|
||||||
|
wallet ID on success/failure. API keys are hashed before comparison, never
|
||||||
|
logged. RPC request logging only logs method and path, not headers or body
|
||||||
|
content. No verbatim header or credential logging found.
|
||||||
|
|
||||||
|
## MOAD-0005 (Thundering Herd): CLEAN
|
||||||
|
|
||||||
|
Node.js is single-threaded, so traditional thundering herd on cache population
|
||||||
|
does not apply. The SigCache (script/sigcache.js) uses BufferMap for O(1)
|
||||||
|
lookups. ChainDB uses LRU caches with proper get-then-fetch patterns. The
|
||||||
|
blockchain locker (bmutex Lock) serializes concurrent access to chain state.
|
||||||
|
Pool uses BufferSet for block/tx dedup maps. No unguarded cache-miss-compute-put
|
||||||
|
patterns found.
|
||||||
|
|
@ -0,0 +1,19 @@
|
||||||
|
--- a/lib/node/rpc.js
|
||||||
|
+++ b/lib/node/rpc.js
|
||||||
|
@@ -1032,8 +1032,14 @@ class RPC extends RPCBase {
|
||||||
|
if (!block)
|
||||||
|
throw new RPCError(errs.MISC_ERROR, 'Block not found.');
|
||||||
|
|
||||||
|
- for (const hash of hashes) {
|
||||||
|
- if (!block.hasTX(hash)) {
|
||||||
|
+ // Build a hash set for O(1) membership checks instead of
|
||||||
|
+ // calling block.hasTX() which does O(T) linear scan per call.
|
||||||
|
+ const txSet = new BufferSet();
|
||||||
|
+ for (const tx of block.txs)
|
||||||
|
+ txSet.add(tx.hash());
|
||||||
|
+
|
||||||
|
+ for (const hash of hashes) {
|
||||||
|
+ if (!txSet.has(hash)) {
|
||||||
|
throw new RPCError(errs.VERIFY_ERROR,
|
||||||
|
'Block does not contain all txids.');
|
||||||
|
}
|
||||||
135
defects/bcoin/test/bcoin-0001-test.js
Normal file
135
defects/bcoin/test/bcoin-0001-test.js
Normal file
|
|
@ -0,0 +1,135 @@
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* bcoin-0001: gettxoutproof Block.hasTX linear scan O(H*T)
|
||||||
|
*
|
||||||
|
* Block.hasTX(hash) calls Block.indexOf(hash) which scans
|
||||||
|
* block.txs linearly. When called in a loop over H hashes,
|
||||||
|
* total cost is O(H*T). Fix: build a Set from tx hashes
|
||||||
|
* before the loop, reducing to O(H+T).
|
||||||
|
*
|
||||||
|
* This test verifies the fix by simulating the pattern with
|
||||||
|
* realistic block sizes and measuring operation counts.
|
||||||
|
*/
|
||||||
|
|
||||||
|
// Simulate Buffer hashes
|
||||||
|
function makeHash(i) {
|
||||||
|
const buf = Buffer.alloc(32, 0);
|
||||||
|
buf.writeUInt32LE(i, 0);
|
||||||
|
return buf;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Simulate block.txs array
|
||||||
|
function makeBlock(txCount) {
|
||||||
|
const txs = [];
|
||||||
|
for (let i = 0; i < txCount; i++) {
|
||||||
|
txs.push({ _hash: makeHash(i), hash() { return this._hash; } });
|
||||||
|
}
|
||||||
|
return { txs };
|
||||||
|
}
|
||||||
|
|
||||||
|
// BEFORE: linear scan per hasTX call (original Block.indexOf pattern)
|
||||||
|
function hasTXLinear(block, hash) {
|
||||||
|
for (let i = 0; i < block.txs.length; i++) {
|
||||||
|
if (block.txs[i].hash().equals(hash))
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
function verifyBefore(block, hashes) {
|
||||||
|
let ops = 0;
|
||||||
|
for (const hash of hashes) {
|
||||||
|
for (let i = 0; i < block.txs.length; i++) {
|
||||||
|
ops++;
|
||||||
|
if (block.txs[i].hash().equals(hash))
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return ops;
|
||||||
|
}
|
||||||
|
|
||||||
|
// AFTER: build hash set, O(1) lookup
|
||||||
|
function verifyAfter(block, hashes) {
|
||||||
|
let ops = 0;
|
||||||
|
const txSet = new Map();
|
||||||
|
for (const tx of block.txs) {
|
||||||
|
ops++;
|
||||||
|
txSet.set(tx.hash().toString('hex'), true);
|
||||||
|
}
|
||||||
|
for (const hash of hashes) {
|
||||||
|
ops++;
|
||||||
|
txSet.has(hash.toString('hex'));
|
||||||
|
}
|
||||||
|
return ops;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test correctness
|
||||||
|
function testCorrectness() {
|
||||||
|
const block = makeBlock(100);
|
||||||
|
const hashes = [makeHash(0), makeHash(50), makeHash(99)];
|
||||||
|
|
||||||
|
// Linear should find all
|
||||||
|
for (const hash of hashes) {
|
||||||
|
if (!hasTXLinear(block, hash)) {
|
||||||
|
console.log('FAIL: linear scan did not find hash');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Set-based should also find all
|
||||||
|
const txSet = new Map();
|
||||||
|
for (const tx of block.txs)
|
||||||
|
txSet.set(tx.hash().toString('hex'), true);
|
||||||
|
|
||||||
|
for (const hash of hashes) {
|
||||||
|
if (!txSet.has(hash.toString('hex'))) {
|
||||||
|
console.log('FAIL: set-based lookup did not find hash');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Should NOT find missing hash
|
||||||
|
const missing = makeHash(999);
|
||||||
|
if (hasTXLinear(block, missing)) {
|
||||||
|
console.log('FAIL: linear scan found non-existent hash');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
if (txSet.has(missing.toString('hex'))) {
|
||||||
|
console.log('FAIL: set-based found non-existent hash');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('PASS: correctness');
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test performance ratio
|
||||||
|
function testPerformance() {
|
||||||
|
const T = 4000; // max block txs
|
||||||
|
const H = 100; // number of hashes to check
|
||||||
|
const block = makeBlock(T);
|
||||||
|
|
||||||
|
// Pick H hashes spread across the block
|
||||||
|
const hashes = [];
|
||||||
|
for (let i = 0; i < H; i++)
|
||||||
|
hashes.push(makeHash(Math.floor(i * T / H)));
|
||||||
|
|
||||||
|
const opsBefore = verifyBefore(block, hashes);
|
||||||
|
const opsAfter = verifyAfter(block, hashes);
|
||||||
|
const ratio = opsBefore / opsAfter;
|
||||||
|
|
||||||
|
console.log(`T=${T}, H=${H}`);
|
||||||
|
console.log(`Before: ${opsBefore} ops`);
|
||||||
|
console.log(`After: ${opsAfter} ops`);
|
||||||
|
console.log(`Ratio: ${ratio.toFixed(1)}x`);
|
||||||
|
|
||||||
|
if (ratio < 2) {
|
||||||
|
console.log('FAIL: expected at least 2x improvement');
|
||||||
|
process.exit(1);
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('PASS: performance');
|
||||||
|
}
|
||||||
|
|
||||||
|
testCorrectness();
|
||||||
|
testPerformance();
|
||||||
Loading…
Add table
Add a link
Reference in a new issue