Skip to content

Commit 9fb51b1

Browse files
committed
fix(commitment): count first-nibble branches in CanDoConcurrentNext to fix flaky test
Test_Trie_CorrectSwitchForConcurrentAndSequential fails intermittently (CI run 22561824327, job 65349926766) because CanDoConcurrentNext() only checks if nibble-0 has a branch node. The test uses a globally-shared rand.NewSource(42) protected by a mutex, so different parallel test runs consume the random stream at different offsets — occasionally producing a 150-key set where zero (or one) keys land at nibble 0 after keccak256 hashing. In that case the function returns false even though the trie is wide enough to parallelize, causing the require.True at line 280 to fail. Root cause: arbitrary choice of nibble 0 as the single probe. P(zero keys at nibble 0 with 150 random keccak hashes) ≈ (15/16)^150 ≈ 5.6e-5 per run — invisible in dev but visible across thousands of CI runs. Fix: instead of probing a single nibble, count how many of the 16 first-nibble sub-tries have stored branch data. Require at least 4 to justify spawning 16 parallel goroutines. - 150 random keys: ~15.997 of 16 nibbles filled (P(< 4 filled) ≈ 0) → true ✓ - 2 remaining keys: at most 2 nibbles filled → false ✓ This makes the decision independent of any particular nibble's occupancy, eliminating the flakiness while keeping the correct semantic (small key sets → sequential).
1 parent 3d203a2 commit 9fb51b1

File tree

1 file changed

+25
-9
lines changed

1 file changed

+25
-9
lines changed

execution/commitment/hex_concurrent_patricia_hashed.go

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -315,20 +315,36 @@ func (p *ConcurrentPatriciaHashed) Process(ctx context.Context, updates *Updates
315315
return rootHash, nil
316316
}
317317

318+
// CanDoConcurrentNext returns true when the trie is wide enough at the root to benefit
319+
// from parallel commitment on the next call. It checks whether the root has no extension
320+
// (i.e. it is a branch node) AND that at least minNibblesForConcurrent distinct first-nibble
321+
// sub-tries have stored branch data.
322+
//
323+
// Checking a single nibble (e.g. nibble 0) is fragile: with a randomly-distributed key set,
324+
// any particular nibble may happen to be empty, producing a spurious false negative. Requiring
325+
// a count threshold makes the decision robust: 150 random keys reliably fill ≥ 14 of 16
326+
// nibbles, whereas a small key set (e.g. 2 keys) fills at most 2.
318327
func (p *ConcurrentPatriciaHashed) CanDoConcurrentNext() (bool, error) {
319-
if p.root.root.extLen == 0 {
320-
zeroPrefixBranch, _, err := p.root.ctx.Branch(HexNibblesToCompactBytes([]byte{0}))
328+
if p.root.root.extLen != 0 {
329+
// root has an extension prefix → single subtrie, no benefit from parallelism
330+
return false, nil
331+
}
332+
// Count distinct first-nibble positions that have stored branch data.
333+
// We need at least this many to justify spawning 16 parallel goroutines.
334+
const minNibblesForConcurrent = 4
335+
nibbleCount := 0
336+
for nib := 0; nib < 16; nib++ {
337+
branch, _, err := p.root.ctx.Branch(HexNibblesToCompactBytes([]byte{byte(nib)}))
321338
if err != nil {
322-
return false, fmt.Errorf("checking shortes prefix branch failed: %w", err)
339+
return false, fmt.Errorf("checking prefix branch for nibble %d failed: %w", nib, err)
323340
}
324-
if len(zeroPrefixBranch) > 4 { // tm+am+cells
325-
// if root has no extension and there is a branch of zero prefix, can use parallel commitment next time
326-
// fmt.Printf("use concurrent next\n")
327-
return true, nil
341+
if len(branch) > 4 { // tm + am + at least one cell
342+
nibbleCount++
343+
if nibbleCount >= minNibblesForConcurrent {
344+
return true, nil
345+
}
328346
}
329-
// fmt.Printf(" 00 [branch %x len %d]\n", zeroPrefixBranch, len(zeroPrefixBranch))
330347
}
331-
// fmt.Printf("use seq trie next [root extLen=%d][ext '%x']\n", p.root.root.extLen, p.root.root.extension[:p.root.root.extLen])
332348
return false, nil
333349
}
334350

0 commit comments

Comments
 (0)