From 185eae00e9af812032b37852d4551c2b4ef6a04a Mon Sep 17 00:00:00 2001 From: Mononaut Date: Sun, 25 Aug 2024 22:38:00 +0000 Subject: [PATCH] Fix RBF tracking inconsistencies --- backend/src/api/rbf-cache.ts | 121 ++++++++++++++++++++++++----------- 1 file changed, 82 insertions(+), 39 deletions(-) diff --git a/backend/src/api/rbf-cache.ts b/backend/src/api/rbf-cache.ts index a087abbe0..f4b192d3a 100644 --- a/backend/src/api/rbf-cache.ts +++ b/backend/src/api/rbf-cache.ts @@ -44,6 +44,22 @@ interface CacheEvent { value?: any, } +/** + * Singleton for tracking RBF trees + * + * Maintains a set of RBF trees, where each tree represents a sequence of + * consecutive RBF replacements. + * + * Trees are identified by the txid of the root transaction. + * + * To maintain consistency, the following invariants must be upheld: + * - Symmetry: replacedBy(A) = B <=> A in replaces(B) + * - Unique id: treeMap(treeMap(X)) = treeMap(X) + * - Unique tree: A in replaces(B) => treeMap(A) == treeMap(B) + * - Existence: X in treeMap => treeMap(X) in rbfTrees + * - Completeness: X in replacedBy => X in treeMap, Y in replaces => Y in treeMap + */ + class RbfCache { private replacedBy: Map = new Map(); private replaces: Map = new Map(); @@ -61,6 +77,10 @@ class RbfCache { setInterval(this.cleanup.bind(this), 1000 * 60 * 10); } + /** + * Low level cache operations + */ + private addTx(txid: string, tx: MempoolTransactionExtended): void { this.txs.set(txid, tx); this.cacheQueue.push({ op: CacheOp.Add, type: 'tx', txid }); @@ -92,6 +112,12 @@ class RbfCache { this.cacheQueue.push({ op: CacheOp.Remove, type: 'exp', txid }); } + /** + * Basic data structure operations + * must uphold tree invariants + */ + + public add(replaced: MempoolTransactionExtended[], newTxExtended: MempoolTransactionExtended): void { if (!newTxExtended || !replaced?.length || this.txs.has(newTxExtended.txid)) { return; @@ -114,6 +140,10 @@ class RbfCache { if (!replacedTx.rbf) { txFullRbf = true; } + if (this.replacedBy.has(replacedTx.txid)) { + // should never happen + continue; + } this.replacedBy.set(replacedTx.txid, newTx.txid); if (this.treeMap.has(replacedTx.txid)) { const treeId = this.treeMap.get(replacedTx.txid); @@ -140,18 +170,47 @@ class RbfCache { } } newTx.fullRbf = txFullRbf; - const treeId = replacedTrees[0].tx.txid; const newTree = { tx: newTx, time: newTime, fullRbf: treeFullRbf, replaces: replacedTrees }; - this.addTree(treeId, newTree); - this.updateTreeMap(treeId, newTree); + this.addTree(newTree.tx.txid, newTree); + this.updateTreeMap(newTree.tx.txid, newTree); this.replaces.set(newTx.txid, replacedTrees.map(tree => tree.tx.txid)); } + public mined(txid): void { + if (!this.txs.has(txid)) { + return; + } + const treeId = this.treeMap.get(txid); + if (treeId && this.rbfTrees.has(treeId)) { + const tree = this.rbfTrees.get(treeId); + if (tree) { + this.setTreeMined(tree, txid); + tree.mined = true; + this.dirtyTrees.add(treeId); + this.cacheQueue.push({ op: CacheOp.Change, type: 'tree', txid: treeId }); + } + } + this.evict(txid); + } + + // flag a transaction as removed from the mempool + public evict(txid: string, fast: boolean = false): void { + this.evictionCount++; + if (this.txs.has(txid) && (fast || !this.expiring.has(txid))) { + const expiryTime = fast ? Date.now() + (1000 * 60 * 10) : Date.now() + (1000 * 86400); // 24 hours + this.addExpiration(txid, expiryTime); + } + } + + /** + * Read-only public interface + */ + public has(txId: string): boolean { return this.txs.has(txId); } @@ -232,32 +291,6 @@ class RbfCache { return changes; } - public mined(txid): void { - if (!this.txs.has(txid)) { - return; - } - const treeId = this.treeMap.get(txid); - if (treeId && this.rbfTrees.has(treeId)) { - const tree = this.rbfTrees.get(treeId); - if (tree) { - this.setTreeMined(tree, txid); - tree.mined = true; - this.dirtyTrees.add(treeId); - this.cacheQueue.push({ op: CacheOp.Change, type: 'tree', txid: treeId }); - } - } - this.evict(txid); - } - - // flag a transaction as removed from the mempool - public evict(txid: string, fast: boolean = false): void { - this.evictionCount++; - if (this.txs.has(txid) && (fast || !this.expiring.has(txid))) { - const expiryTime = fast ? Date.now() + (1000 * 60 * 10) : Date.now() + (1000 * 86400); // 24 hours - this.addExpiration(txid, expiryTime); - } - } - // is the transaction involved in a full rbf replacement? public isFullRbf(txid: string): boolean { const treeId = this.treeMap.get(txid); @@ -271,6 +304,10 @@ class RbfCache { return tree?.fullRbf; } + /** + * Cache maintenance & utility functions + */ + private cleanup(): void { const now = Date.now(); for (const txid of this.expiring.keys()) { @@ -299,10 +336,6 @@ class RbfCache { for (const tx of (replaces || [])) { // recursively remove prior versions from the cache this.replacedBy.delete(tx); - // if this is the id of a tree, remove that too - if (this.treeMap.get(tx) === tx) { - this.removeTree(tx); - } this.remove(tx); } } @@ -376,8 +409,15 @@ class RbfCache { this.txs.set(txEntry.value.txid, txEntry.value); }); this.staleCount = 0; - for (const deflatedTree of trees) { - await this.importTree(mempool, deflatedTree.root, deflatedTree.root, deflatedTree, this.txs); + for (const deflatedTree of trees.sort((a, b) => Object.keys(b).length - Object.keys(a).length)) { + const tree = await this.importTree(mempool, deflatedTree.root, deflatedTree.root, deflatedTree, this.txs); + if (tree) { + this.addTree(tree.tx.txid, tree); + this.updateTreeMap(tree.tx.txid, tree); + if (tree.mined) { + this.evict(tree.tx.txid); + } + } } expiring.forEach(expiringEntry => { if (this.txs.has(expiringEntry.key)) { @@ -426,6 +466,12 @@ class RbfCache { return; } + // if this tx is already in the cache, return early + if (this.treeMap.has(txid)) { + this.removeTree(deflated.key); + return; + } + // recursively reconstruct child trees for (const childId of treeInfo.replaces) { const replaced = await this.importTree(mempool, root, childId, deflated, txs, mined); @@ -457,10 +503,6 @@ class RbfCache { fullRbf: treeInfo.fullRbf, replaces, }; - this.treeMap.set(txid, root); - if (root === txid) { - this.addTree(root, tree); - } return tree; } @@ -511,6 +553,7 @@ class RbfCache { processTxs(txs); } + // evict missing transactions for (const txid of txids) { if (!found[txid]) { this.evict(txid, false);