Merge remote-tracking branch 'cce/fix-kvmods-deltas' into kvs_catchpo…

…int_stats

ledger/accountdb.go

@@ -3825,11 +3825,18 @@ func accountsNewRoundImpl(
 	}
 
 	updatedKVs = make(map[string]persistedKVData, len(kvPairs))
-	for key, value := range kvPairs {
-		if value.data != nil {
-			err = writer.upsertKvPair(key, value.data)
-			updatedKVs[key] = persistedKVData{value: value.data, round: lastUpdateRound}
+	for key, mv := range kvPairs {
+		if mv.data != nil {
+			// reminder: check oldData for nil here, b/c bytes.Equal conflates nil and "".
+			if mv.oldData != nil && bytes.Equal(mv.oldData, mv.data) {
+				continue // changed back within the delta span
+			}
+			err = writer.upsertKvPair(key, mv.data)
+			updatedKVs[key] = persistedKVData{value: mv.data, round: lastUpdateRound}
 		} else {
+			if mv.oldData == nil { // Came and went within the delta span
+				continue
+			}
 			err = writer.deleteKvPair(key)
 			updatedKVs[key] = persistedKVData{value: nil, round: lastUpdateRound}
 		}

ledger/acctupdates_test.go

@@ -1499,7 +1499,9 @@ func TestKVCache(t *testing.T) {
 		if i < kvCnt/kvsPerBlock {
 			for j := 0; j < kvsPerBlock; j++ {
 				name := fmt.Sprintf("%d", curKV)
-				kvMods[name] = ledgercore.KvValueDelta{Data: nil}
+				// needs an old data, else optimized away.
+				// if oldData = "" there is the best chance of a bug, so we use that
+				kvMods[name] = ledgercore.KvValueDelta{Data: nil, OldData: []byte("")}
 				curKV++
 			}
 		}

ledger/boxtxn_test.go

@@ -19,6 +19,7 @@ package ledger
 import (
 	"bytes"
 	"encoding/binary"
+	"strings"
 	"testing"
 	"time"
 
@@ -156,9 +157,21 @@ func TestBoxCreate(t *testing.T) {
 		adam := call.Args("create", "adam")
 		dl.txn(adam, "invalid Box reference adam")
 		adam.Boxes = []transactions.BoxRef{{Index: 0, Name: []byte("adam")}}
+
+		dl.beginBlock()
 		dl.txn(adam)
+		vb := dl.endBlock()
+
+		// confirm the deltas has the creation
+		require.Len(t, vb.Delta().KvMods, 1)
+		for _, kvDelta := range vb.Delta().KvMods { // There's only one
+			require.Nil(t, kvDelta.OldData) // A creation has nil OldData
+			require.Len(t, kvDelta.Data, 24)
+		}
+
 		dl.txn(adam.Args("check", "adam", "\x00\x00"))
 		dl.txgroup("box_create\nassert", adam.Noted("one"), adam.Noted("two"))
+
 		bobo := call.Args("create", "bobo")
 		dl.txn(bobo, "invalid Box reference bobo")
 		bobo.Boxes = []transactions.BoxRef{{Index: 0, Name: []byte("bobo")}}
@@ -354,6 +367,7 @@ func TestBoxRW(t *testing.T) {
 
 	genBalances, addrs, _ := ledgertesting.NewTestGenesis()
 	ledgertesting.TestConsensusRange(t, boxVersion, 0, func(t *testing.T, ver int, cv protocol.ConsensusVersion) {
+		t.Parallel()
 		dl := NewDoubleLedger(t, genBalances, cv)
 		defer dl.Close()
 
@@ -369,8 +383,19 @@ func TestBoxRW(t *testing.T) {
 			Boxes:         []transactions.BoxRef{{Index: 0, Name: []byte("x")}},
 		}
 
-		dl.txn(call.Args("create", "x", "\x10"))    // 16
+		dl.txn(call.Args("create", "x", "\x10")) // 16
+		dl.beginBlock()
 		dl.txn(call.Args("set", "x", "ABCDEFGHIJ")) // 10 long
+		vb := dl.endBlock()
+		// confirm the deltas has the change, including the old value
+		require.Len(t, vb.Delta().KvMods, 1)
+		for _, kvDelta := range vb.Delta().KvMods { // There's only one
+			require.Equal(t, kvDelta.OldData,
+				[]byte(strings.Repeat("\x00", 16)))
+			require.Equal(t, kvDelta.Data,
+				[]byte("ABCDEFGHIJ\x00\x00\x00\x00\x00\x00"))
+		}
+
 		dl.txn(call.Args("check", "x", "ABCDE"))
 		dl.txn(call.Args("check", "x", "ABCDEFGHIJ"))
 		dl.txn(call.Args("check", "x", "ABCDEFGHIJ\x00"))

ledger/catchpointtracker.go

@@ -1008,6 +1008,7 @@ func (ct *catchpointTracker) accountsUpdateBalances(accountsDeltas compactAccoun
 			continue
 		}
 		if mv.oldData != nil {
+			// reminder: check mv.data for nil here, b/c bytes.Equal conflates nil and "".
 			if mv.data != nil && bytes.Equal(mv.oldData, mv.data) {
 				continue // changed back within the delta span
 			}

ledger/internal/cow.go

@@ -116,10 +116,6 @@ func makeRoundCowState(b roundCowParent, hdr bookkeeping.BlockHeader, proto conf
 }
 
 func (cb *roundCowState) deltas() ledgercore.StateDelta {
-	if len(cb.sdeltas) == 0 {
-		return cb.mods
-	}
-
 	// Apply storage deltas to account deltas
 	for addr, smap := range cb.sdeltas {
 		for aapp, storeDelta := range smap {