From f52c48153bd6c58d8c2e3815ea45b54518c20094 Mon Sep 17 00:00:00 2001 From: jramos0 Date: Thu, 6 Aug 2026 04:14:45 -0600 Subject: [PATCH] Feat: RBF Diff MVP copilot feedback --- .../rbf-timeline/rbf-timeline.component.html | 19 +++- .../rbf-timeline/rbf-timeline.component.scss | 36 +++++++- .../rbf-timeline/rbf-timeline.component.ts | 92 +++++++++++-------- frontend/src/app/shared/rbf-diff.utils.ts | 82 +++++++++++++---- 4 files changed, 165 insertions(+), 64 deletions(-) diff --git a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.html b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.html index f9f9f0816..ebf79cb66 100644 --- a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.html +++ b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.html @@ -28,15 +28,22 @@
+ link is disabled rather than navigating away mid-comparison. + Dropping routerLink also drops the href, so the role, tab stop + and keyboard handlers are restored explicitly. -->
+ @@ -49,11 +56,17 @@ - +
+ [attr.role]="showDiff && cell.replacement?.replacedBy ? 'button' : null" + [attr.tabindex]="showDiff && cell.replacement?.replacedBy ? 0 : null" + (click)="onEdgeClick($event, cell.replacement)" + (keydown.enter)="onEdgeClick($event, cell.replacement)" + (keydown.space)="onEdgeClick($event, cell.replacement)">
+
diff --git a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.scss b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.scss index bb0eb6c21..0da93fd09 100644 --- a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.scss +++ b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.scss @@ -36,6 +36,22 @@ right: 0; background: linear-gradient(to left, var(--box-bg), var(--box-bg), transparent); } + + // On a phone the wrapper is barely wider than a couple of nodes, so a 2em + // band of solid background reads as the line being cut rather than fading. + @media (max-width: 575.98px) { + &::before, &::after { + width: 1em; + } + + &::before { + background: linear-gradient(to right, var(--box-bg), transparent); + } + + &::after { + background: linear-gradient(to left, var(--box-bg), transparent); + } + } } .fade-out { @@ -261,9 +277,10 @@ } .diff-hint { - width: 100%; + width: calc(100% - 2em); + margin: 0.5em auto 0; + padding: 0 0.5em; text-align: center; - margin: 0.5em 0 0; font-size: 0.85rem; color: var(--transparent-fg); @@ -306,6 +323,21 @@ width: 38%; } + // styles.scss applies `tr { white-space: nowrap }` to every table, which makes a + // long label like "Output (address changed)" spill out of its column and land on + // top of the address next to it. Labels wrap inside their own column instead. + tbody tr td:first-child { + white-space: normal; + overflow-wrap: break-word; + } + + @media (max-width: 575.98px) { + // the tables stack below this width, so the label can afford to be wider + col.label-col { + width: 45%; + } + } + .previous-box { border-top: 3px solid var(--red); } diff --git a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.ts b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.ts index 66ebdba6e..0ecfff863 100644 --- a/frontend/src/app/components/rbf-timeline/rbf-timeline.component.ts +++ b/frontend/src/app/components/rbf-timeline/rbf-timeline.component.ts @@ -4,7 +4,7 @@ import { RbfTree, RbfTransaction } from '@interfaces/node-api.interface'; import { StateService } from '@app/services/state.service'; import { ApiService } from '@app/services/api.service'; import { forkJoin, of, Subject } from 'rxjs'; -import { catchError, takeUntil } from 'rxjs/operators'; +import { catchError, switchMap, takeUntil } from 'rxjs/operators'; import { Transaction, Vout } from '@interfaces/electrs.interface'; import { calculateRbfDiff } from '@app/shared/rbf-diff.utils'; @@ -91,6 +91,9 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { private nodeIndex = new Map(); private destroy$ = new Subject(); + // Comparisons go through one stream so a slower earlier request can never land + // on top of a newer selection. A null request cancels whatever is in flight. + private diffRequest$ = new Subject<{ oldTxid: string, newTxid: string } | null>(); constructor( private router: Router, @@ -101,6 +104,27 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { if (this.locale.startsWith('ar') || this.locale.startsWith('fa') || this.locale.startsWith('he')) { this.dir = 'rtl'; } + // subscribed here rather than in ngOnInit because ngOnChanges runs first and + // can already have queued a comparison + this.diffRequest$.pipe( + switchMap((request) => request ? forkJoin({ + oldTx: this.apiService.getRbfCachedTx$(request.oldTxid).pipe(catchError(() => of(null))), + newTx: this.apiService.getRbfCachedTx$(request.newTxid).pipe(catchError(() => of(null))), + }) : of(null)), + takeUntil(this.destroy$), + ).subscribe((result) => { + if (!result) { + return; // cancelled by a newer selection + } + this.diffLoading = false; + if (!result.oldTx || !result.newTx) { + this.diffError = true; + return; + } + this.selectedOldTx = result.oldTx; + this.selectedNewTx = result.newTx; + this.diffView = this.buildDiffView(result.oldTx, result.newTx); + }); } ngOnInit(): void { @@ -375,16 +399,30 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { this.pendingAnchorTxid = null; this.diffOldTxid = null; this.diffNewTxid = null; - // default to the transaction being viewed against what it replaced, falling - // back to the tip of the tree when nothing is selected (the replacements list) + const current = (this.txid ? this.nodeIndex.get(this.txid) : null) ?? null; - const node = current?.replaces.length - ? current - : (current?.replacedBy ? this.nodeIndex.get(current.replacedBy.txid) : null) - ?? (this.hasReplacements ? this.replacements : null); - if (node?.replaces.length) { - this.diffOldTxid = node.replaces[0].tx.txid; - this.diffNewTxid = node.tx.txid; + + // the viewed transaction is itself a replacement: diff it against what it replaced + if (current?.replaces.length) { + this.diffOldTxid = current.replaces[0].tx.txid; + this.diffNewTxid = current.tx.txid; + return; + } + + // the viewed transaction was replaced: keep it as the old endpoint. Using the + // parent's first child instead would open the diff on a sibling whenever the + // replacement swallowed several transactions at once. + const parent = current?.replacedBy ? this.nodeIndex.get(current.replacedBy.txid) : null; + if (current && parent) { + this.diffOldTxid = current.tx.txid; + this.diffNewTxid = parent.tx.txid; + return; + } + + // nothing selected (the replacements list): fall back to the tip of the tree + if (this.hasReplacements) { + this.diffOldTxid = this.replacements.replaces[0].tx.txid; + this.diffNewTxid = this.replacements.tx.txid; } } @@ -393,7 +431,7 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { * The first click anchors one end, the second picks the other — which is what * makes it possible to compare transactions that aren't next to each other. */ - onNodeClick(event: MouseEvent, node: RbfTree): void { + onNodeClick(event: Event, node: RbfTree): void { if (!this.showDiff) { return; } @@ -424,7 +462,7 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { * Clicking the line joining two dots compares exactly those two — the quick * path for the common case of two consecutive replacements. */ - onEdgeClick(event: MouseEvent, node: RbfTree | undefined): void { + onEdgeClick(event: Event, node: RbfTree | undefined): void { if (!this.showDiff || !node?.replacedBy) { return; } @@ -437,6 +475,7 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { } private clearDiffResult(): void { + this.diffRequest$.next(null); // drop anything still in flight this.diffLoading = false; this.diffError = false; this.selectedOldTx = null; @@ -448,37 +487,10 @@ export class RbfTimelineComponent implements OnInit, OnChanges, OnDestroy { if (!this.diffOldTxid || !this.diffNewTxid) { return; } - this.compareTxs(this.diffOldTxid, this.diffNewTxid); - } - - /** - * Fetches both transactions from the RBF cache and computes their structural diff. - * @param oldTxid - the replaced transaction - * @param newTxid - the replacement - */ - private compareTxs(oldTxid: string, newTxid: string): void { this.diffError = false; this.diffLoading = true; this.diffView = null; - forkJoin({ - oldTx: this.apiService.getRbfCachedTx$(oldTxid).pipe( - catchError(() => of(null)) - ), - newTx: this.apiService.getRbfCachedTx$(newTxid).pipe( - catchError(() => of(null)) - ), - }).pipe( - takeUntil(this.destroy$) - ).subscribe((result) => { - this.diffLoading = false; - if (!result.oldTx || !result.newTx) { - this.diffError = true; - return; - } - this.selectedOldTx = result.oldTx; - this.selectedNewTx = result.newTx; - this.diffView = this.buildDiffView(result.oldTx, result.newTx); - }); + this.diffRequest$.next({ oldTxid: this.diffOldTxid, newTxid: this.diffNewTxid }); } /** diff --git a/frontend/src/app/shared/rbf-diff.utils.ts b/frontend/src/app/shared/rbf-diff.utils.ts index 537d02647..def56d743 100644 --- a/frontend/src/app/shared/rbf-diff.utils.ts +++ b/frontend/src/app/shared/rbf-diff.utils.ts @@ -28,6 +28,45 @@ export interface RbfDiff { }; } +interface OutputEntry { + out: Vout; + index: number; + matched: boolean; + outputId: string; +} + +/** + * A FIFO of candidate outputs sharing a key. Keeping the original order and a + * cursor means popping returns the first still-unmatched candidate, exactly as a + * linear scan would, without re-walking the whole list for every output. + */ +interface CandidateQueue { + entries: OutputEntry[]; + cursor: number; +} + +function indexBy(entries: OutputEntry[], keyOf: (entry: OutputEntry) => string): Map { + const index = new Map(); + for (const entry of entries) { + const key = keyOf(entry); + const queue = index.get(key); + if (queue) { + queue.entries.push(entry); + } else { + index.set(key, { entries: [entry], cursor: 0 }); + } + } + return index; +} + +function takeUnmatched(queue: CandidateQueue | undefined): OutputEntry | undefined { + if (!queue) { return undefined; } + while (queue.cursor < queue.entries.length && queue.entries[queue.cursor].matched) { + queue.cursor++; + } + return queue.entries[queue.cursor]; +} + // Compares structural differences between an original transaction and its RBF replacement export function calculateRbfDiff(oldTx: Transaction, newTx: Transaction): RbfDiff { @@ -99,14 +138,17 @@ export function calculateRbfDiff(oldTx: Transaction, newTx: Transaction): RbfDif newAddressCounts.set(addr, (newAddressCounts.get(addr) ?? 0) + 1); } + // Candidate indexes, so each pass looks up its matches instead of rescanning + // every remaining output for every output it has to place + const byIdAndValue = indexBy(newOutputs, (entry) => `${entry.outputId}|${entry.out.value}`); + const byId = indexBy(newOutputs, (entry) => entry.outputId); + const byValue = indexBy(newOutputs, (entry) => `${entry.out.value}`); + const byIndex = indexBy(newOutputs, (entry) => `${entry.index}`); + let anyCursor = 0; + // Pass 1: match truly unchanged outputs (same address AND value, regardless of position) for (const oldItem of oldOutputs) { - const match = newOutputs.find( - (newItem) => - !newItem.matched && - oldItem.outputId === newItem.outputId && - oldItem.out.value === newItem.out.value - ); + const match = takeUnmatched(byIdAndValue.get(`${oldItem.outputId}|${oldItem.out.value}`)); if (match) { oldItem.matched = true; match.matched = true; @@ -117,11 +159,7 @@ export function calculateRbfDiff(oldTx: Transaction, newTx: Transaction): RbfDif // Pass 2: match remaining outputs by address to detect fee-adjusted or value-modified outputs for (const oldItem of oldOutputs) { if (oldItem.matched) { continue; } - const match = newOutputs.find( - (newItem) => - !newItem.matched && - oldItem.outputId === newItem.outputId - ); + const match = takeUnmatched(byId.get(oldItem.outputId)); if (!match) { continue; } oldItem.matched = true; match.matched = true; @@ -145,18 +183,24 @@ export function calculateRbfDiff(oldTx: Transaction, newTx: Transaction): RbfDif // then one at the same index, before falling back to document order. for (const oldItem of oldOutputs) { if (oldItem.matched) { continue; } - const match = - newOutputs.find((newItem) => !newItem.matched && newItem.out.value === oldItem.out.value) ?? - newOutputs.find((newItem) => !newItem.matched && newItem.index === oldItem.index) ?? - newOutputs.find((newItem) => !newItem.matched); + let match = + takeUnmatched(byValue.get(`${oldItem.out.value}`)) ?? + takeUnmatched(byIndex.get(`${oldItem.index}`)); + if (!match) { + while (anyCursor < newOutputs.length && newOutputs[anyCursor].matched) { anyCursor++; } + match = newOutputs[anyCursor]; + } if (!match) { continue; } oldItem.matched = true; match.matched = true; - const addressChanged = oldItem.out.scriptpubkey_address !== match.out.scriptpubkey_address; + // Compare the stable output id, not the address: two different addressless + // scripts (OP_RETURN and friends) both have an undefined address, which would + // otherwise report a script replacement as an unchanged destination. + const addressChanged = oldItem.outputId !== match.outputId; const valueChanged = oldItem.out.value !== match.out.value; - const changeType: 'address' | 'value' | 'both' = - addressChanged && valueChanged ? 'both' : - addressChanged ? 'address' : 'value'; + // Anything reaching this pass had no same-id candidate left, so the + // destination necessarily differs; only the value may or may not have moved. + const changeType: 'address' | 'both' = addressChanged && valueChanged ? 'both' : 'address'; modifiedOutputs.push({ old: oldItem.out, new: match.out, index: oldItem.index, changeType }); }