Skip to content

Commit 650dcc0

Browse files
rafa-jsgalt-tr
andauthored
fix(bump): skip datahub peers whose coinbase BUMP does not reconcile (#167) (#172)
A datahub peer can report the correct block-header merkle root while serving a coinbase BUMP (and subtree decomposition) that computes a different root. Such a peer passes the fetch-time validator, so if it sorts ahead of a consistent peer the fetch loop selects it on every attempt and every Kafka retry, deterministically failing the post-build ValidateCompoundRoot and blocking the block from ever building a valid BUMP even when a consistent peer is healthy. Observed on mainnet block 950675. Reject such peers in the fetch loop by verifying the coinbase BUMP computes the reported header merkle root (the coinbase is leaf 0, so its BUMP must), letting the loop fall through to a consistent peer. No-op when a peer omits the coinbase BUMP. Tests use the real consistent/inconsistent coinbase BUMPs captured for block 950675. Co-authored-by: Dylan <64976002+galt-tr@users.noreply.github.com>
1 parent b078c86 commit 650dcc0

3 files changed

Lines changed: 208 additions & 0 deletions

File tree

bump/datahub.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,26 @@ func FetchBlockDataForBUMPWithOptions(ctx context.Context, datahubURLs []string,
121121
continue
122122
}
123123
}
124+
// Reject endpoints whose coinbase BUMP does not reconcile to the header
125+
// merkle root they served. The coinbase tx sits at level 0 offset 0 of
126+
// the block merkle tree, so its BUMP MUST compute that header root; an
127+
// endpoint that serves a coinbase BUMP computing a different root is
128+
// internally inconsistent (stale/pruned/buggy peer) and would only
129+
// produce a compound BUMP that fails the post-build ValidateCompoundRoot.
130+
// Without this, a bad peer that sorts ahead of a good one is selected on
131+
// every fetch and every retry, deterministically blocking the block from
132+
// ever building a valid BUMP even when a consistent peer is healthy.
133+
// Skipping it here lets the loop fall through to that consistent peer.
134+
if cbErr := coinbaseBUMPReconciles(cbBUMP, root); cbErr != nil {
135+
logger.Warn(
136+
"datahub coinbase BUMP does not reconcile to header merkle root",
137+
zap.Int("idx", i),
138+
zap.String("url", dataHubURL),
139+
zap.Error(cbErr),
140+
)
141+
urlErrors = append(urlErrors, fmt.Sprintf("url[%d] %q: coinbase BUMP mismatch: %v", i, dataHubURL, cbErr))
142+
continue
143+
}
124144
return hashes, cbBUMP, root, nil
125145
}
126146
return nil, nil, nil, fmt.Errorf("all DataHub URLs failed for block %s:\n %s", blockHash, strings.Join(urlErrors, "\n "))
@@ -147,6 +167,35 @@ func SubtreeCountValidator(minSubtrees int) BlockDataValidator {
147167
}
148168
}
149169

170+
// coinbaseBUMPReconciles verifies that the coinbase BUMP computes the expected
171+
// block-header merkle root. The coinbase transaction is leaf 0 of the block
172+
// merkle tree, so folding it up its own BUMP must yield the header merkle root.
173+
//
174+
// Returns nil when there is nothing to check: a nil expected root, or an empty
175+
// coinbase BUMP (some peers legitimately omit it; downstream BUMP construction
176+
// handles a missing coinbase BUMP separately, so we must not reject those here).
177+
func coinbaseBUMPReconciles(coinbaseBUMP []byte, headerMerkleRoot *chainhash.Hash) error {
178+
if headerMerkleRoot == nil || len(coinbaseBUMP) == 0 {
179+
return nil
180+
}
181+
cbPath, err := transaction.NewMerklePathFromBinary(coinbaseBUMP)
182+
if err != nil {
183+
return fmt.Errorf("parse coinbase BUMP: %w", err)
184+
}
185+
cbTxID := extractCoinbaseTxID(coinbaseBUMP)
186+
if cbTxID == nil {
187+
return fmt.Errorf("coinbase BUMP has no coinbase txid at level 0 offset 0")
188+
}
189+
got, err := cbPath.ComputeRoot(cbTxID)
190+
if err != nil {
191+
return fmt.Errorf("compute coinbase BUMP root: %w", err)
192+
}
193+
if !got.IsEqual(headerMerkleRoot) {
194+
return fmt.Errorf("coinbase BUMP root %s != header merkle root %s", got, headerMerkleRoot)
195+
}
196+
return nil
197+
}
198+
150199
// fetchBlockBinary fetches a block from the binary endpoint and parses
151200
// subtree hashes, coinbase BUMP, and the header merkle root from the response.
152201
// Returns the HTTP status code (0 on transport error before a response was

bump/datahub_coinbase_test.go

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package bump
2+
3+
import (
4+
"context"
5+
"encoding/hex"
6+
"net/http"
7+
"net/http/httptest"
8+
"strings"
9+
"testing"
10+
11+
"github.com/bsv-blockchain/go-sdk/chainhash"
12+
"github.com/bsv-blockchain/go-sdk/transaction"
13+
"github.com/bsv-blockchain/go-sdk/util"
14+
"go.uber.org/zap"
15+
)
16+
17+
func decodeHex(t *testing.T, s string) []byte {
18+
t.Helper()
19+
b, err := hex.DecodeString(strings.ReplaceAll(strings.TrimSpace(s), "\n", ""))
20+
if err != nil {
21+
t.Fatalf("decode hex: %v", err)
22+
}
23+
return b
24+
}
25+
26+
// TestCoinbaseBUMPReconciles_RealData exercises the new fetch-time check with
27+
// the real coinbase BUMPs captured from production for block 950675: the
28+
// consistent one reconciles to the header root, the inconsistent one (served by
29+
// a stale peer) does not.
30+
func TestCoinbaseBUMPReconciles_RealData(t *testing.T) {
31+
good := decodeHex(t, issue167CoinbaseBUMPHex)
32+
bad := decodeHex(t, issue167BadCoinbaseBUMPHex)
33+
headerRoot, _ := chainhash.NewHashFromHex(issue167HeaderMerkleRoot)
34+
35+
if err := coinbaseBUMPReconciles(good, headerRoot); err != nil {
36+
t.Errorf("good coinbase BUMP should reconcile, got: %v", err)
37+
}
38+
if err := coinbaseBUMPReconciles(bad, headerRoot); err == nil {
39+
t.Error("inconsistent coinbase BUMP should be rejected, got nil")
40+
}
41+
// Guard rails: nothing to check => nil.
42+
if err := coinbaseBUMPReconciles(nil, headerRoot); err != nil {
43+
t.Errorf("empty coinbase BUMP should be a no-op, got: %v", err)
44+
}
45+
if err := coinbaseBUMPReconciles(good, nil); err != nil {
46+
t.Errorf("nil header root should be a no-op, got: %v", err)
47+
}
48+
}
49+
50+
// blockWithCoinbaseBUMP builds a datahub block-binary response carrying one
51+
// subtree hash plus a coinbase transaction and coinbase BUMP, matching the
52+
// format parseBlockBinary expects.
53+
func blockWithCoinbaseBUMP(t *testing.T, headerMerkleRoot chainhash.Hash, coinbaseBUMP []byte) []byte {
54+
t.Helper()
55+
out := make([]byte, 0, 256+len(coinbaseBUMP))
56+
header := make([]byte, 80)
57+
copy(header[36:68], headerMerkleRoot[:]) // internal byte order
58+
out = append(out, header...)
59+
out = append(out, 0x00, 0x00) // txCount=0, sizeBytes=0
60+
out = append(out, 0x01) // subtreeCount=1
61+
subtreeHash := make([]byte, 32)
62+
for i := range subtreeHash {
63+
subtreeHash[i] = 0xAB
64+
}
65+
out = append(out, subtreeHash...)
66+
// coinbase tx (skipped by the parser, but must be parseable)
67+
cbTx := transaction.NewTransaction().Bytes()
68+
out = append(out, cbTx...)
69+
out = append(out, util.VarInt(950675).Bytes()...) // blockHeight
70+
out = append(out, util.VarInt(uint64(len(coinbaseBUMP))).Bytes()...) // coinbaseBUMP length
71+
out = append(out, coinbaseBUMP...)
72+
return out
73+
}
74+
75+
// TestFetchBlockData_SkipsInconsistentCoinbaseEndpoint reproduces the block
76+
// 950675 production scenario: one datahub peer serves a coinbase BUMP that does
77+
// not reconcile to the (correct) header root it reports, while another serves a
78+
// consistent one. The fetch loop must skip the bad peer and return the good
79+
// peer's data, rather than deterministically selecting the bad peer on every
80+
// fetch and blocking the block from ever building a valid BUMP.
81+
func TestFetchBlockData_SkipsInconsistentCoinbaseEndpoint(t *testing.T) {
82+
headerRoot, _ := chainhash.NewHashFromHex(issue167HeaderMerkleRoot)
83+
good := decodeHex(t, issue167CoinbaseBUMPHex)
84+
bad := decodeHex(t, issue167BadCoinbaseBUMPHex)
85+
86+
// Sanity: the helper produces a body the parser reads a coinbase BUMP from.
87+
if _, cb, _, perr := parseBlockBinary(blockWithCoinbaseBUMP(t, *headerRoot, good)); perr != nil || len(cb) == 0 {
88+
t.Fatalf("test fixture invalid: parse err=%v cbLen=%d", perr, len(cb))
89+
}
90+
91+
badBody := blockWithCoinbaseBUMP(t, *headerRoot, bad)
92+
goodBody := blockWithCoinbaseBUMP(t, *headerRoot, good)
93+
94+
badSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
95+
_, _ = w.Write(badBody)
96+
}))
97+
t.Cleanup(badSrv.Close)
98+
goodSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
99+
_, _ = w.Write(goodBody)
100+
}))
101+
t.Cleanup(goodSrv.Close)
102+
103+
_, cbBUMP, root, err := FetchBlockDataForBUMPWithOptions(
104+
context.Background(),
105+
[]string{badSrv.URL, goodSrv.URL}, // bad peer sorts first
106+
"00000000000000000eb6ea115a5cb140ffeae7b673b4c5b6f3fac62e0adae3c5",
107+
1<<20, nil, zap.NewNop(),
108+
)
109+
if err != nil {
110+
t.Fatalf("expected fall-through to the consistent peer, got: %v", err)
111+
}
112+
if root == nil || !root.IsEqual(headerRoot) {
113+
t.Fatalf("unexpected header root: %v", root)
114+
}
115+
if !bytesEqual(cbBUMP, good) {
116+
t.Fatal("expected the good peer's coinbase BUMP to be selected")
117+
}
118+
119+
// And if ONLY the bad peer exists, the loop surfaces the mismatch error.
120+
_, _, _, err = FetchBlockDataForBUMPWithOptions(
121+
context.Background(),
122+
[]string{badSrv.URL},
123+
"00000000000000000eb6ea115a5cb140ffeae7b673b4c5b6f3fac62e0adae3c5",
124+
1<<20, nil, zap.NewNop(),
125+
)
126+
if err == nil || !strings.Contains(err.Error(), "coinbase BUMP mismatch") {
127+
t.Fatalf("expected coinbase BUMP mismatch error, got: %v", err)
128+
}
129+
}
130+
131+
func bytesEqual(a, b []byte) bool {
132+
if len(a) != len(b) {
133+
return false
134+
}
135+
for i := range a {
136+
if a[i] != b[i] {
137+
return false
138+
}
139+
}
140+
return true
141+
}

bump/issue167_fixtures_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,21 @@ ad5d5e3993a0beb4d6ecd2b1b4732ba91edc1f321a61a63e23010100d84dd236137cd4b8d4f6afef
121121
205be34d735d6c422f890101001ce56aaaa53cb05c37e2bf09fad7d2ce27a93de69c5528bfb7811ff3709b6ef9010100f9c6
122122
d33366f96e88d6f089d2e7fc790a4589d2c2761ef24cc0cb138a7629bf5d01010032bdc4314e21d17578a511c1cb3abae62e
123123
190f9bdc788f9ed122ce1451983514010100f3a74f645f3f131ecca71c47d3e6965c7a1b377ad2bf00d91a1214c66d8508ee`
124+
125+
// issue167BadCoinbaseBUMPHex is the INCONSISTENT coinbase BUMP a stale/buggy
126+
// datahub peer served for block 950675: it parses but computes a root
127+
// (0a4d286f...) that does NOT equal the real header merkle root. Used to test
128+
// that such an endpoint is skipped by the datahub fetch loop.
129+
const issue167BadCoinbaseBUMPHex = `
130+
fe93810e0010020002b3a9aab3b87cc9cd8ba9256369e9cea3923417b3c34526aeb5197bf41dd04d15010008c9ddef1d1d86
131+
56f4940830ba88d022aafa619bbe063f1ac9a8a246fa17c272010100357681967b505a3adcfc2d535f6190c7a01d733fe156
132+
e1de92bd2bc0b429148c0101006c0193888b6c8eb3448deef10ac8626615cd2fbb7d10dce44a9e813412dda4f7010100089e
133+
29effe6a1fe5cdbbbf447c19ed2c5f4c69af40de60bc62a42d9b4d5f2094010100a810dfc80ea1b60b23762ff0769432cab5
134+
813526b2ac545e8543f268c2f8ed26010100fbf384ca863cf444fed6126d3accbe0b06506acb303100c8e38d3cc254970904
135+
0101005d12951c271db674c8d745f17971b4d9d54b6af385ba4d5086cd3598a0fffd97010100c32b7c062e4f43f192b5f633
136+
faa79186c096bba3d49b61e7aa44a6f26be3b11c010100c7d9904a406b137fe28c9e3e9593df313af7237e0fe46148397eef
137+
3a0e61306b010100a3f59b185427b1ddce06dee086115206a532ca8f1f1a597753cd5fe854ad6b39010100dec0c1788c1385
138+
ad5d5e3993a0beb4d6ecd2b1b4732ba91edc1f321a61a63e23010100d84dd236137cd4b8d4f6afefee7068b3186d74bc8160
139+
205be34d735d6c422f890101001ce56aaaa53cb05c37e2bf09fad7d2ce27a93de69c5528bfb7811ff3709b6ef9010100f9c6
140+
d33366f96e88d6f089d2e7fc790a4589d2c2761ef24cc0cb138a7629bf5d01010032bdc4314e21d17578a511c1cb3abae62e
141+
190f9bdc788f9ed122ce14519835140101009e0bd9a9ddc1449415341056886b10a27e642c4236993962319d00a9f50c8805`

0 commit comments

Comments
 (0)