Skip to content

fix: broadcast fan-out waits for first success instead of first result#42

Merged
mrz1836 merged 6 commits intomainfrom
fix/broadcast-first-success
Apr 7, 2026
Merged

fix: broadcast fan-out waits for first success instead of first result#42
mrz1836 merged 6 commits intomainfrom
fix/broadcast-first-success

Conversation

@shruggr
Copy link
Copy Markdown
Collaborator

@shruggr shruggr commented Apr 6, 2026

Summary

  • Broadcast fan-out: returns immediately on first definitive result — success (ACCEPTED/SENT) or rejection (4xx). Only service errors (5xx) wait for other endpoints to respond before giving up.
  • SERVICE_ERROR vs REJECTED: 4xx from teranode = genuine rejection, tx is invalid. 5xx = service error, transient failure. New SERVICE_ERROR status lets callers distinguish between the two.
  • Coinbase in merkle proofs: fetchBlockSubtrees was downloading the full block binary but only extracting subtree hashes, discarding the coinbase transaction. Without the coinbase txid, the all-FF placeholder at subtree index 0 was never replaced, producing invalid merkle proofs for any transaction whose proof path crosses the left side of the tree. Renamed to fetchBlock, now parses the coinbase and returns it alongside subtree hashes. Both P2P and catch-up paths now get the coinbase from the block data directly.

Test plan

  • Single broadcast endpoint: behavior unchanged
  • Multiple endpoints, one returning 5xx: success from healthy endpoint is returned
  • Genuine rejection (4xx): returned immediately without waiting for other endpoints
  • All endpoints return 5xx: last service error is returned after all respond
  • Merkle proofs: verify proof for a mined transaction computes to the correct merkle root
  • Catch-up processing: verify coinbase replacement works when processing missed blocks

When multiple teranode endpoints are configured, the broadcast now
returns on the first successful response (ACCEPTED/SENT). Rejections
from one endpoint no longer short-circuit if another endpoint may
accept the transaction. Status is only persisted and published once
after the final result is determined.
@shruggr shruggr requested a review from mrz1836 as a code owner April 6, 2026 20:01
@github-actions github-actions bot added size/M Medium change (51–200 lines) bug-P3 Lowest rated bug, affects nearly none or low-impact labels Apr 6, 2026
@shruggr shruggr marked this pull request as draft April 6, 2026 20:05
shruggr added 5 commits April 6, 2026 16:10
4xx responses from teranode are genuine rejections (invalid tx) and
return immediately. 5xx responses are service errors — other endpoints
may still accept the transaction. Fan-out only waits on service errors,
not rejections.
fetchBlockSubtrees was downloading the full block binary from the datahub
but only extracting subtree hashes, discarding the coinbase transaction.
The coinbase txid is needed to replace the all-FF placeholder at subtree
index 0 — without it, every hash on the left side of the merkle tree is
computed incorrectly, producing invalid proofs.

Renamed fetchBlockSubtrees to fetchBlock, now returns a blockData struct
with both SubtreeHashes and CoinbaseTxID. processBlockTransactions uses
the coinbase from block data directly instead of relying on the P2P
BlockMessage.Coinbase field, which may not always be set (e.g. during
catch-up processing).

Removed parseCoinbaseTxID as it's no longer needed.
@shruggr
Copy link
Copy Markdown
Collaborator Author

shruggr commented Apr 7, 2026

Nancy flags CVE-2026-34986 in go-jose/v4@v4.1.3. This is pulled in by google.golang.org/grpc, which is at the latest stable release (v1.80.0). grpc has not yet bumped its go-jose dependency to the fixed v4.1.4, including in its dev branch (v1.81.0-dev). This can't be resolved until grpc updates — could be added to the Nancy excludes in the meantime.

@shruggr shruggr marked this pull request as ready for review April 7, 2026 13:44
Copy link
Copy Markdown
Collaborator

@mrz1836 mrz1836 left a comment

Choose a reason for hiding this comment

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

LGTM

@mrz1836 mrz1836 merged commit 70c3707 into main Apr 7, 2026
65 of 67 checks passed
@github-actions github-actions bot deleted the fix/broadcast-first-success branch April 7, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-P3 Lowest rated bug, affects nearly none or low-impact size/M Medium change (51–200 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants