Skip to content

Commit 3f7ad14

Browse files
liamliam
committed
4148: fix subtree layers
1 parent d26c18f commit 3f7ad14

File tree

2 files changed

+226
-1
lines changed

2 files changed

+226
-1
lines changed

‎services/subtreevalidation/SubtreeValidation.go‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1350,9 +1350,25 @@ func (u *Server) prepareTxsPerLevel(ctx context.Context, transactions []missingT
13501350
return level
13511351
}
13521352

1353-
// If no dependencies in subtree, level is 0
1353+
// Get parents that are in the subtree
13541354
parents := dependencies[txHash]
1355+
1356+
// If no dependencies in subtree, check if transaction has ANY inputs
1357+
// Transactions with external parents (inputs not in subtree) should NOT be level 0
1358+
// because they depend on UTXOs that may not be available yet
13551359
if len(parents) == 0 {
1360+
// Get the actual transaction to check if it has inputs
1361+
if wrapper, exists := txMap[txHash]; exists && wrapper.missingTx.tx != nil {
1362+
// If transaction has inputs but no parents in subtree, it depends on external UTXOs
1363+
// These should be processed at a higher level to ensure parent UTXOs are available
1364+
if len(wrapper.missingTx.tx.Inputs) > 0 {
1365+
// Assign to level 1 to ensure external dependencies are resolved first
1366+
levelCache[txHash] = 1
1367+
return 1
1368+
}
1369+
}
1370+
1371+
// No inputs at all (shouldn't happen for non-coinbase) - level 0
13561372
levelCache[txHash] = 0
13571373
return 0
13581374
}

‎services/subtreevalidation/SubtreeValidation_levels_bug_test.go‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
package subtreevalidation
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/bsv-blockchain/go-bt/v2"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// TestPrepareTxsPerLevel_ExternalParents tests that transactions with parents
12+
// outside the subtree are not incorrectly assigned to level 0.
13+
//
14+
// This test demonstrates the bug where transactions with external parents
15+
// (parents not in the current subtree) are assigned level 0, causing them
16+
// to be validated before their parents are available, resulting in TX_NOT_FOUND errors.
17+
func TestPrepareTxsPerLevel_ExternalParents(t *testing.T) {
18+
t.Run("transaction with external parent should not be level 0", func(t *testing.T) {
19+
s := &Server{}
20+
21+
// Create a simple chain:
22+
// parentTx (external, not in subtree) -> childTx (in subtree)
23+
parentTx := tx1.Clone()
24+
childTx := bt.NewTx()
25+
26+
// Make childTx spend from parentTx (which is NOT in the subtree)
27+
err := childTx.FromUTXOs(&bt.UTXO{
28+
TxIDHash: parentTx.TxIDChainHash(),
29+
Vout: 0,
30+
LockingScript: parentTx.Outputs[0].LockingScript,
31+
Satoshis: parentTx.Outputs[0].Satoshis,
32+
})
33+
require.NoError(t, err)
34+
35+
err = childTx.AddP2PKHOutputFromScript(parentTx.Outputs[0].LockingScript, 1000)
36+
require.NoError(t, err)
37+
38+
// Create transactions slice with ONLY childTx (parentTx is external)
39+
transactions := []missingTx{
40+
{
41+
tx: childTx,
42+
idx: 0,
43+
},
44+
}
45+
46+
// Call prepareTxsPerLevel
47+
maxLevel, txsPerLevel, err := s.prepareTxsPerLevel(context.Background(), transactions)
48+
require.NoError(t, err)
49+
50+
// BUG DEMONSTRATION: Currently, childTx is assigned to level 0 because its parent
51+
// is not in the subtree, so len(dependencies[childTx]) == 0
52+
//
53+
// This is INCORRECT because childTx has a parent (just not in the subtree).
54+
// When childTx is validated at level 0, it will fail with TX_NOT_FOUND
55+
// because parentTx is not available yet.
56+
57+
// Verify the current buggy behaviour exists
58+
currentMaxLevel := maxLevel
59+
currentLevel0Count := len(txsPerLevel[0])
60+
61+
// Document what currently happens (will pass, showing the bug exists)
62+
t.Logf("Current buggy behaviour: maxLevel=%d, level0_count=%d", currentMaxLevel, currentLevel0Count)
63+
64+
// This assertion will FAIL when the bug is fixed
65+
// The bug is that transactions with external parents are assigned to level 0
66+
if currentMaxLevel == 0 && currentLevel0Count == 1 {
67+
t.Fatal("BUG CONFIRMED: Transaction with external parent is incorrectly assigned to level 0. " +
68+
"This will cause TX_NOT_FOUND errors during validation because the parent transaction " +
69+
"is not in the subtree and won't be available when this transaction is validated.")
70+
}
71+
})
72+
73+
t.Run("mixed transactions with internal and external parents", func(t *testing.T) {
74+
s := &Server{}
75+
76+
// Create a more complex scenario:
77+
// externalParent (not in subtree) -> tx1 (in subtree) -> tx2 (in subtree)
78+
externalParent := parentTx1.Clone()
79+
tx1InSubtree := bt.NewTx()
80+
tx2InSubtree := bt.NewTx()
81+
82+
// tx1 spends from external parent
83+
err := tx1InSubtree.FromUTXOs(&bt.UTXO{
84+
TxIDHash: externalParent.TxIDChainHash(),
85+
Vout: 0,
86+
LockingScript: externalParent.Outputs[0].LockingScript,
87+
Satoshis: externalParent.Outputs[0].Satoshis,
88+
})
89+
require.NoError(t, err)
90+
91+
err = tx1InSubtree.AddP2PKHOutputFromScript(externalParent.Outputs[0].LockingScript, 5000)
92+
require.NoError(t, err)
93+
94+
// tx2 spends from tx1 (both in subtree)
95+
err = tx2InSubtree.FromUTXOs(&bt.UTXO{
96+
TxIDHash: tx1InSubtree.TxIDChainHash(),
97+
Vout: 0,
98+
LockingScript: tx1InSubtree.Outputs[0].LockingScript,
99+
Satoshis: tx1InSubtree.Outputs[0].Satoshis,
100+
})
101+
require.NoError(t, err)
102+
103+
err = tx2InSubtree.AddP2PKHOutputFromScript(tx1InSubtree.Outputs[0].LockingScript, 4000)
104+
require.NoError(t, err)
105+
106+
// Only tx1 and tx2 are in the subtree (externalParent is not)
107+
transactions := []missingTx{
108+
{
109+
tx: tx1InSubtree,
110+
idx: 0,
111+
},
112+
{
113+
tx: tx2InSubtree,
114+
idx: 1,
115+
},
116+
}
117+
118+
maxLevel, txsPerLevel, err := s.prepareTxsPerLevel(context.Background(), transactions)
119+
require.NoError(t, err)
120+
121+
// Find which level each tx is at
122+
tx1Level := -1
123+
tx2Level := -1
124+
125+
for level, txs := range txsPerLevel {
126+
for _, mtx := range txs {
127+
if mtx.tx.TxID() == tx1InSubtree.TxID() {
128+
tx1Level = level
129+
}
130+
if mtx.tx.TxID() == tx2InSubtree.TxID() {
131+
tx2Level = level
132+
}
133+
}
134+
}
135+
136+
t.Logf("Current behaviour: tx1_level=%d, tx2_level=%d, maxLevel=%d", tx1Level, tx2Level, maxLevel)
137+
138+
// BUG CONFIRMED: tx1 is at level 0 even though it has an external parent
139+
// When tx1 is validated at level 0, it will try to look up externalParent
140+
// and fail with TX_NOT_FOUND, exactly like the error in the bug report.
141+
if tx1Level == 0 && tx2Level == 1 {
142+
t.Fatal("BUG CONFIRMED: tx1 is at level 0 despite having an external parent. " +
143+
"This causes TX_NOT_FOUND when trying to validate tx1 before its parent is available. " +
144+
"tx2 is correctly at level 1 (depends on tx1 which is in subtree).")
145+
}
146+
})
147+
148+
t.Run("all transactions have external parents", func(t *testing.T) {
149+
s := &Server{}
150+
151+
// Scenario: Multiple independent transactions, all with external parents
152+
// None of them depend on each other, but all depend on external txs
153+
externalParent1 := parentTx1.Clone()
154+
externalParent2 := tx1.Clone()
155+
156+
childTx1 := bt.NewTx()
157+
childTx2 := bt.NewTx()
158+
159+
// childTx1 spends from externalParent1 (not in subtree)
160+
err := childTx1.FromUTXOs(&bt.UTXO{
161+
TxIDHash: externalParent1.TxIDChainHash(),
162+
Vout: 0,
163+
LockingScript: externalParent1.Outputs[0].LockingScript,
164+
Satoshis: externalParent1.Outputs[0].Satoshis,
165+
})
166+
require.NoError(t, err)
167+
168+
err = childTx1.AddP2PKHOutputFromScript(externalParent1.Outputs[0].LockingScript, 3000)
169+
require.NoError(t, err)
170+
171+
// childTx2 spends from externalParent2 (not in subtree)
172+
err = childTx2.FromUTXOs(&bt.UTXO{
173+
TxIDHash: externalParent2.TxIDChainHash(),
174+
Vout: 0,
175+
LockingScript: externalParent2.Outputs[0].LockingScript,
176+
Satoshis: externalParent2.Outputs[0].Satoshis,
177+
})
178+
require.NoError(t, err)
179+
180+
err = childTx2.AddP2PKHOutputFromScript(externalParent2.Outputs[0].LockingScript, 2000)
181+
require.NoError(t, err)
182+
183+
// Both child transactions are in the subtree, but their parents are not
184+
transactions := []missingTx{
185+
{
186+
tx: childTx1,
187+
idx: 0,
188+
},
189+
{
190+
tx: childTx2,
191+
idx: 1,
192+
},
193+
}
194+
195+
maxLevel, txsPerLevel, err := s.prepareTxsPerLevel(context.Background(), transactions)
196+
require.NoError(t, err)
197+
198+
t.Logf("Current behaviour: maxLevel=%d, level0_count=%d", maxLevel, len(txsPerLevel[0]))
199+
200+
// BUG CONFIRMED: Both transactions are assigned to level 0
201+
// This is problematic because when these transactions are validated,
202+
// they will all fail with TX_NOT_FOUND errors for their parent transactions.
203+
if maxLevel == 0 && len(txsPerLevel[0]) == 2 {
204+
t.Fatal("BUG CONFIRMED: Both transactions are at level 0 despite having external parents. " +
205+
"During validation, both will fail with TX_NOT_FOUND when trying to look up their " +
206+
"parent transactions that are not in the subtree.")
207+
}
208+
})
209+
}

0 commit comments

Comments
 (0)