diff --git a/execution/commitment/commitment.go b/execution/commitment/commitment.go index 431b14341f7..f8c3d1bb57e 100644 --- a/execution/commitment/commitment.go +++ b/execution/commitment/commitment.go @@ -566,23 +566,29 @@ func (be *BranchEncoder) CollectUpdate( prefix []byte, bitmap, touchMap, afterMap uint16, cells *[16]cellEncodeData, + isNew bool, ) error { var prev []byte var foundInCache bool var err error - if be.cache != nil { - prev, foundInCache = be.cache.GetAndEvictBranch(prefix) - if foundInCache && be.metrics != nil { - be.metrics.cacheBranch.Add(1) + if !isNew { + if be.cache != nil { + prev, foundInCache = be.cache.GetAndEvictBranch(prefix) + if foundInCache && be.metrics != nil { + be.metrics.cacheBranch.Add(1) + } } - } - if !foundInCache { - prev, _, err = ctx.Branch(prefix) - if err != nil { - return err + if !foundInCache { + prev, _, err = ctx.Branch(prefix) + if err != nil { + return err + } } } + if prev == nil { + prev = []byte{} + } update, err := be.EncodeBranch(bitmap, touchMap, afterMap, cells) if err != nil { @@ -624,6 +630,7 @@ func (be *BranchEncoder) CollectDeferredUpdate( prefix []byte, bitmap, touchMap, afterMap uint16, cells *[16]cellEncodeData, + isNew bool, ) error { // Flush if duplicate prefix or too many deferred updates limit := be.maxDeferredUpdates @@ -642,24 +649,28 @@ func (be *BranchEncoder) CollectDeferredUpdate( be.ClearDeferred() } - // try to get previous data from cache var ( prev []byte foundInCache bool err error ) - if be.cache != nil { - prev, foundInCache = be.cache.GetAndEvictBranch(prefix) - if foundInCache && be.metrics != nil { - be.metrics.cacheBranch.Add(1) + if !isNew { + if be.cache != nil { + prev, foundInCache = be.cache.GetAndEvictBranch(prefix) + if foundInCache && be.metrics != nil { + be.metrics.cacheBranch.Add(1) + } + } + if !foundInCache { + prev, _, err = ctx.Branch(prefix) + } + if err != nil { + return err } } - if !foundInCache { - prev, _, err = ctx.Branch(prefix) - } - if err != nil { - return err + if prev == nil { + prev = []byte{} } // Track this prefix as pending diff --git a/execution/commitment/commitment_test.go b/execution/commitment/commitment_test.go index 1a19822e78a..f8539715d7f 100644 --- a/execution/commitment/commitment_test.go +++ b/execution/commitment/commitment_test.go @@ -944,6 +944,78 @@ func TestUpdates_TouchPlainKey(t *testing.T) { require.NoError(t, err) } +// recordingCtx captures Branch call count and PutBranch arguments for assertions. +type recordingCtx struct { + branchCalls int + puts []struct{ prefix, data, prev []byte } +} + +func (r *recordingCtx) Branch(_ []byte) ([]byte, kv.Step, error) { + r.branchCalls++ + return nil, 0, nil +} +func (r *recordingCtx) PutBranch(prefix, data, prev []byte) error { + r.puts = append(r.puts, struct{ prefix, data, prev []byte }{ + common.Copy(prefix), common.Copy(data), common.Copy(prev), + }) + return nil +} +func (r *recordingCtx) Account(_ []byte) (*Update, error) { return nil, nil } +func (r *recordingCtx) Storage(_ []byte) (*Update, error) { return nil, nil } +func (r *recordingCtx) TxNum() uint64 { return 0 } + +func TestCollectUpdate_IsNewSkipsLookupAndMatchesNilPath(t *testing.T) { + t.Parallel() + prefix := []byte{0xab, 0xcd} + row, bm := generateCellRow(t, 4) + cells := generateCellEncodeDataRow(t, row, bm) + + // isNew=false: Branch is probed but returns nil (key doesn't exist yet) + ctxA := &recordingCtx{} + beA := NewBranchEncoder(1024) + require.NoError(t, beA.CollectUpdate(ctxA, prefix, bm, bm, bm, &cells, false)) + require.Equal(t, 1, ctxA.branchCalls, "isNew=false must probe Branch") + require.Len(t, ctxA.puts, 1) + + // isNew=true: Branch must not be called, but PutBranch output must be identical + ctxB := &recordingCtx{} + beB := NewBranchEncoder(1024) + require.NoError(t, beB.CollectUpdate(ctxB, prefix, bm, bm, bm, &cells, true)) + require.Equal(t, 0, ctxB.branchCalls, "isNew=true must not probe Branch") + require.Len(t, ctxB.puts, 1) + + require.Equal(t, ctxA.puts[0].data, ctxB.puts[0].data) + require.Equal(t, ctxA.puts[0].prev, ctxB.puts[0].prev) +} + +func TestCollectDeferredUpdate_IsNewSkipsLookupAndMatchesNilPath(t *testing.T) { + t.Parallel() + prefix := []byte{0x11, 0x22} + row, bm := generateCellRow(t, 4) + cells := generateCellEncodeDataRow(t, row, bm) + + // isNew=false: Branch is probed but returns nil + ctxA := &recordingCtx{} + beA := NewBranchEncoder(1024) + beA.setDeferUpdates(true) + require.NoError(t, beA.CollectDeferredUpdate(ctxA, prefix, bm, bm, bm, &cells, false)) + require.NoError(t, beA.ApplyDeferredUpdates(1, ctxA.PutBranch)) + require.Equal(t, 1, ctxA.branchCalls, "isNew=false must probe Branch") + require.Len(t, ctxA.puts, 1) + + // isNew=true: Branch must not be called, deferred output must match + ctxB := &recordingCtx{} + beB := NewBranchEncoder(1024) + beB.setDeferUpdates(true) + require.NoError(t, beB.CollectDeferredUpdate(ctxB, prefix, bm, bm, bm, &cells, true)) + require.NoError(t, beB.ApplyDeferredUpdates(1, ctxB.PutBranch)) + require.Equal(t, 0, ctxB.branchCalls, "isNew=true must not probe Branch") + require.Len(t, ctxB.puts, 1) + + require.Equal(t, ctxA.puts[0].data, ctxB.puts[0].data) + require.Equal(t, ctxA.puts[0].prev, ctxB.puts[0].prev) +} + func TestUpdates_TouchStorageClearsDeleteOnRewrite(t *testing.T) { t.Parallel() diff --git a/execution/commitment/hex_patricia_hashed.go b/execution/commitment/hex_patricia_hashed.go index 8da75bc089d..8d93a58699c 100644 --- a/execution/commitment/hex_patricia_hashed.go +++ b/execution/commitment/hex_patricia_hashed.go @@ -1959,11 +1959,11 @@ func (hph *HexPatriciaHashed) foldBranch(row int, nibble, upDepth, depth int16, } if hph.branchEncoder.DeferUpdatesEnabled() { - if err := hph.branchEncoder.CollectDeferredUpdate(hph.ctx, updateKey, bitmap, hph.touchMap[row], hph.afterMap[row], &cellData); err != nil { + if err := hph.branchEncoder.CollectDeferredUpdate(hph.ctx, updateKey, bitmap, hph.touchMap[row], hph.afterMap[row], &cellData, !hph.branchBefore[row]); err != nil { return fmt.Errorf("failed to collect deferred branch update: %w", err) } } else { - if err := hph.branchEncoder.CollectUpdate(hph.ctx, updateKey, bitmap, hph.touchMap[row], hph.afterMap[row], &cellData); err != nil { + if err := hph.branchEncoder.CollectUpdate(hph.ctx, updateKey, bitmap, hph.touchMap[row], hph.afterMap[row], &cellData, !hph.branchBefore[row]); err != nil { return fmt.Errorf("failed to encode branch update: %w", err) } } @@ -2231,7 +2231,7 @@ func (hph *HexPatriciaHashed) foldDelete(row int, nibble, upDepth int16, upCell // If evictCache is true, it also evicts the branch from the cache. func (hph *HexPatriciaHashed) collectDeleteUpdate(updateKey []byte, row int, evictCache bool) error { if hph.branchBefore[row] { - if err := hph.branchEncoder.CollectUpdate(hph.ctx, updateKey, 0, hph.touchMap[row], 0, nil); err != nil { + if err := hph.branchEncoder.CollectUpdate(hph.ctx, updateKey, 0, hph.touchMap[row], 0, nil, false); err != nil { return fmt.Errorf("failed to encode leaf node update: %w", err) } if evictCache && hph.cache != nil {