Skip to content

Commit 568f0e2

Browse files
authored
fix: check the correctness of the leaf ranges (#268)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview Closes: #267 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Strengthened error handling for subtree root verification and leaf range calculations to improve robustness against invalid inputs. - **Bug Fixes** - Enhanced validation processes to prevent incorrect operations and ensure reliable range processing. - **Tests** - Refined test cases to focus on relevant scenarios and added new tests for improved coverage of edge cases related to error handling. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 1278ba2 commit 568f0e2

2 files changed

Lines changed: 69 additions & 20 deletions

File tree

proof.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,10 @@ func (proof Proof) VerifySubtreeRootInclusion(nth *NmtHasher, subtreeRoots [][]b
528528
return popIfNonEmpty(&subtreeRoots), nil
529529
}
530530

531+
if end-start == 1 {
532+
return nil, fmt.Errorf("the provided range [%d, %d) does not reference a valid inner node", proof.start, proof.end)
533+
}
534+
531535
// Recursively get left and right subtree
532536
k := getSplitPoint(end - start)
533537
left, err := computeRoot(start, start+k)
@@ -634,7 +638,13 @@ func nextLeafRange(currentStart, currentEnd, subtreeWidth int) (LeafRange, error
634638
if err != nil {
635639
return LeafRange{}, err
636640
}
637-
return LeafRange{Start: currentStart, End: currentStart + currentRange}, nil
641+
rangeEnd := currentStart + currentRange
642+
idealTreeSize := nextSubtreeSize(uint64(currentStart), uint64(rangeEnd))
643+
if currentStart+idealTreeSize != rangeEnd {
644+
// this will happen if the calculated range does not correctly reference an inner node in the tree.
645+
return LeafRange{}, fmt.Errorf("provided subtree width %d doesn't allow creating a valid leaf range [%d, %d)", subtreeWidth, currentStart, rangeEnd)
646+
}
647+
return LeafRange{Start: currentStart, End: rangeEnd}, nil
638648
}
639649

640650
// largestPowerOfTwo calculates the largest power of two

proof_test.go

Lines changed: 58 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,18 +1332,6 @@ func TestNextLeafRange(t *testing.T) {
13321332
subtreeRootMaximumLeafRange: 8,
13331333
expectedRange: LeafRange{Start: 4, End: 8},
13341334
},
1335-
{
1336-
currentStart: 4,
1337-
currentEnd: 20,
1338-
subtreeRootMaximumLeafRange: 16,
1339-
expectedRange: LeafRange{Start: 4, End: 20},
1340-
},
1341-
{
1342-
currentStart: 4,
1343-
currentEnd: 20,
1344-
subtreeRootMaximumLeafRange: 1,
1345-
expectedRange: LeafRange{Start: 4, End: 5},
1346-
},
13471335
{
13481336
currentStart: 4,
13491337
currentEnd: 20,
@@ -1356,12 +1344,6 @@ func TestNextLeafRange(t *testing.T) {
13561344
subtreeRootMaximumLeafRange: 4,
13571345
expectedRange: LeafRange{Start: 4, End: 8},
13581346
},
1359-
{
1360-
currentStart: 4,
1361-
currentEnd: 20,
1362-
subtreeRootMaximumLeafRange: 8,
1363-
expectedRange: LeafRange{Start: 4, End: 12},
1364-
},
13651347
{
13661348
currentStart: 0,
13671349
currentEnd: 1,
@@ -1392,6 +1374,36 @@ func TestNextLeafRange(t *testing.T) {
13921374
subtreeRootMaximumLeafRange: 0,
13931375
expectError: true,
13941376
},
1377+
{ // A range not referencing any inner node
1378+
currentStart: 1,
1379+
currentEnd: 3,
1380+
subtreeRootMaximumLeafRange: 4,
1381+
expectError: true,
1382+
},
1383+
{ // A range not referencing any inner node
1384+
currentStart: 1,
1385+
currentEnd: 5,
1386+
subtreeRootMaximumLeafRange: 4,
1387+
expectError: true,
1388+
},
1389+
{ // A range not referencing any inner node
1390+
currentStart: 1,
1391+
currentEnd: 6,
1392+
subtreeRootMaximumLeafRange: 4,
1393+
expectError: true,
1394+
},
1395+
{ // A range not referencing any inner node
1396+
currentStart: 1,
1397+
currentEnd: 7,
1398+
subtreeRootMaximumLeafRange: 4,
1399+
expectError: true,
1400+
},
1401+
{ // A range not referencing any inner node
1402+
currentStart: 2,
1403+
currentEnd: 8,
1404+
subtreeRootMaximumLeafRange: 4,
1405+
expectError: true,
1406+
},
13951407
}
13961408

13971409
for _, tt := range tests {
@@ -1798,7 +1810,6 @@ func TestVerifySubtreeRootInclusion(t *testing.T) {
17981810
root: root,
17991811
expectError: true,
18001812
},
1801-
18021813
{
18031814
proof: func() Proof {
18041815
p, err := tree.ProveRange(0, 8)
@@ -1828,3 +1839,31 @@ func TestVerifySubtreeRootInclusion(t *testing.T) {
18281839
})
18291840
}
18301841
}
1842+
1843+
// TestVerifySubtreeRootInclusion_infiniteRecursion is motivated by a failing test
1844+
// case in celestia-node
1845+
func TestVerifySubtreeRootInclusion_infiniteRecursion(t *testing.T) {
1846+
namespaceIDs := bytes.Repeat([]byte{1}, 64)
1847+
tree := exampleNMT(1, true, namespaceIDs...)
1848+
root, err := tree.Root()
1849+
require.NoError(t, err)
1850+
1851+
nmthasher := tree.treeHasher
1852+
hasher := nmthasher.(*NmtHasher)
1853+
subtreeRoot, err := tree.ComputeSubtreeRoot(0, 4)
1854+
require.NoError(t, err)
1855+
subtreeRoots := [][]byte{subtreeRoot, subtreeRoot, subtreeRoot, subtreeRoot, subtreeRoot, subtreeRoot, subtreeRoot}
1856+
subtreeWidth := 8
1857+
1858+
proof, err := tree.ProveRange(19, 64)
1859+
require.NoError(t, err)
1860+
1861+
require.NotPanics(t, func() {
1862+
// This previously hits:
1863+
// runtime: goroutine stack exceeds 1000000000-byte limit
1864+
// runtime: sp=0x14020160480 stack=[0x14020160000, 0x14040160000]
1865+
// fatal error: stack overflow
1866+
_, err = proof.VerifySubtreeRootInclusion(hasher, subtreeRoots, subtreeWidth, root)
1867+
require.Error(t, err)
1868+
})
1869+
}

0 commit comments

Comments
 (0)