Exclude KvMods and address review feedback

ledger/internal/appcow.go

@@ -243,11 +243,11 @@ func (cb *roundCowState) AllocateApp(addr basics.Address, aidx basics.AppIndex,
 	lsd.maxCounts = &space
 
 	if global {
-		cb.mods.Creatables[basics.CreatableIndex(aidx)] = ledgercore.ModifiedCreatable{
+		cb.mods.AddCreatable(basics.CreatableIndex(aidx), ledgercore.ModifiedCreatable{
 			Ctype:   basics.AppCreatable,
 			Creator: addr,
 			Created: true,
-		}
+		})
 	}
 	return nil
 }
@@ -275,11 +275,11 @@ func (cb *roundCowState) DeallocateApp(addr basics.Address, aidx basics.AppIndex
 	lsd.kvCow = make(stateDelta)
 
 	if global {
-		cb.mods.Creatables[basics.CreatableIndex(aidx)] = ledgercore.ModifiedCreatable{
+		cb.mods.AddCreatable(basics.CreatableIndex(aidx), ledgercore.ModifiedCreatable{
 			Ctype:   basics.AppCreatable,
 			Creator: addr,
 			Created: false,
-		}
+		})
 	}
 	return nil
 }

ledger/internal/appcow_test.go

@@ -119,7 +119,7 @@ func getCow(creatables []modsData) *roundCowState {
 		proto: config.Consensus[protocol.ConsensusCurrentVersion],
 	}
 	for _, e := range creatables {
-		cs.mods.UpsertCreatable(e.cidx, ledgercore.ModifiedCreatable{Ctype: e.ctype, Creator: e.addr, Created: true})
+		cs.mods.AddCreatable(e.cidx, ledgercore.ModifiedCreatable{Ctype: e.ctype, Creator: e.addr, Created: true})
 	}
 	return cs
 }

ledger/internal/applications.go

@@ -197,12 +197,12 @@ func (cb *roundCowBase) kvGet(key string) ([]byte, bool, error) {
 }
 
 func (cs *roundCowState) kvPut(key string, value []byte) error {
-	cs.mods.KvMods[key] = ledgercore.KvValueDelta{Data: value}
+	cs.mods.AddKvMod(key, ledgercore.KvValueDelta{Data: value})
 	return nil
 }
 
 func (cs *roundCowState) kvDel(key string) error {
-	cs.mods.KvMods[key] = ledgercore.KvValueDelta{Data: nil}
+	cs.mods.AddKvMod(key, ledgercore.KvValueDelta{Data: nil})
 	return nil
 }
 

ledger/internal/assetcow.go

@@ -23,7 +23,7 @@ import (
 
 func (cs *roundCowState) AllocateAsset(addr basics.Address, index basics.AssetIndex, global bool) error {
 	if global {
-		cs.mods.UpsertCreatable(
+		cs.mods.AddCreatable(
 			basics.CreatableIndex(index),
 			ledgercore.ModifiedCreatable{
 				Ctype:   basics.AssetCreatable,
@@ -37,7 +37,7 @@ func (cs *roundCowState) AllocateAsset(addr basics.Address, index basics.AssetIn
 
 func (cs *roundCowState) DeallocateAsset(addr basics.Address, index basics.AssetIndex, global bool) error {
 	if global {
-		cs.mods.UpsertCreatable(
+		cs.mods.AddCreatable(
 			basics.CreatableIndex(index),
 			ledgercore.ModifiedCreatable{
 				Ctype:   basics.AssetCreatable,

ledger/internal/cow.go

@@ -253,7 +253,7 @@ func (cb *roundCowState) addTx(txn transactions.Transaction, txid transactions.T
 	cb.mods.Txids[txid] = ledgercore.IncludedTransactions{LastValid: txn.LastValid, Intra: uint64(len(cb.mods.Txids))}
 	cb.incTxnCount()
 	if txn.Lease != [32]byte{} {
-		cb.mods.UpsertTxLease(ledgercore.Txlease{Sender: txn.Sender, Lease: txn.Lease}, txn.LastValid)
+		cb.mods.AddTxLease(ledgercore.Txlease{Sender: txn.Sender, Lease: txn.Lease}, txn.LastValid)
 	}
 }
 
@@ -287,10 +287,10 @@ func (cb *roundCowState) commitToParent() {
 	cb.commitParent.txnCount += cb.txnCount
 
 	for txl, expires := range cb.mods.Txleases {
-		cb.commitParent.mods.UpsertTxLease(txl, expires)
+		cb.commitParent.mods.AddTxLease(txl, expires)
 	}
 	for cidx, delta := range cb.mods.Creatables {
-		cb.commitParent.mods.UpsertCreatable(cidx, delta)
+		cb.commitParent.mods.AddCreatable(cidx, delta)
 	}
 	for addr, smod := range cb.sdeltas {
 		for aapp, nsd := range smod {
@@ -309,7 +309,7 @@ func (cb *roundCowState) commitToParent() {
 	cb.commitParent.mods.StateProofNext = cb.mods.StateProofNext
 
 	for key, value := range cb.mods.KvMods {
-		cb.commitParent.mods.KvMods[key] = value
+		cb.commitParent.mods.AddKvMod(key, value)
 	}
 }
 

ledger/ledgercore/statedelta.go

@@ -90,17 +90,18 @@ type StateDelta struct {
 	Accts AccountDeltas
 
 	// modified kv pairs (nil == delete)
+	// not preallocated use .AddKvMod to insert instead of direct assignment
 	KvMods map[string]KvValueDelta
 
 	// new Txids for the txtail and TxnCounter, mapped to txn.LastValid
 	Txids map[transactions.Txid]IncludedTransactions
 
 	// new txleases for the txtail mapped to expiration
-	// not pre-allocated so use .UpsertTxleases to modify instead of direct assignment
+	// not pre-allocated so use .AddTxLease to insert instead of direct assignment
 	Txleases map[Txlease]basics.Round
 
 	// new creatables creator lookup table
-	// not pre-allocated so use .UpsertCreatables to modify instead of direct assignment
+	// not pre-allocated so use .AddCreatable to insert instead of direct assignment
 	Creatables map[basics.CreatableIndex]ModifiedCreatable
 
 	// new block header; read-only
@@ -195,9 +196,8 @@ type AccountDeltas struct {
 // This does not play well for AssetConfig and ApplicationCall transactions on scale
 func MakeStateDelta(hdr *bookkeeping.BlockHeader, prevTimestamp int64, hint int, stateProofNext basics.Round) StateDelta {
 	return StateDelta{
-		Accts:  MakeAccountDeltas(hint),
-		KvMods: make(map[string]KvValueDelta),
-		Txids:  make(map[transactions.Txid]IncludedTransactions, hint),
+		Accts: MakeAccountDeltas(hint),
+		Txids: make(map[transactions.Txid]IncludedTransactions, hint),
 		// asset or application creation are considered as rare events so do not pre-allocate space for them
 		Hdr:                      hdr,
 		StateProofNext:           stateProofNext,
@@ -402,22 +402,30 @@ func (ad *AccountDeltas) UpsertAssetResource(addr basics.Address, aidx basics.As
 	ad.assetResourcesCache[key] = last
 }
 
-// UpsertTxLease adds a new TxLease to the StateDelta
-func (sd *StateDelta) UpsertTxLease(txLease Txlease, expired basics.Round) {
+// AddTxLease adds a new TxLease to the StateDelta
+func (sd *StateDelta) AddTxLease(txLease Txlease, expired basics.Round) {
 	if sd.Txleases == nil {
 		sd.Txleases = make(map[Txlease]basics.Round)
 	}
 	sd.Txleases[txLease] = expired
 }
 
-// UpsertCreatable adds a new Creatable to the StateDelta
-func (sd *StateDelta) UpsertCreatable(idx basics.CreatableIndex, creatable ModifiedCreatable) {
+// AddCreatable adds a new Creatable to the StateDelta
+func (sd *StateDelta) AddCreatable(idx basics.CreatableIndex, creatable ModifiedCreatable) {
 	if sd.Creatables == nil {
 		sd.Creatables = make(map[basics.CreatableIndex]ModifiedCreatable)
 	}
 	sd.Creatables[idx] = creatable
 }
 
+// AddKvMod adds a new KvMod to the StateDelta
+func (sd *StateDelta) AddKvMod(key string, delta KvValueDelta) {
+	if sd.KvMods == nil {
+		sd.KvMods = make(map[string]KvValueDelta)
+	}
+	sd.KvMods[key] = delta
+}
+
 // OptimizeAllocatedMemory by reallocating maps to needed capacity
 // For each data structure, reallocate if it would save us at least 50MB aggregate
 // If provided maxBalLookback or maxTxnLife are zero, dependent optimizations will not occur.

ledger/ledgercore/statedelta_test.go

@@ -102,11 +102,14 @@ func TestMakeStateDeltaMaps(t *testing.T) {
 	sd := MakeStateDelta(nil, 0, 23000, basics.Round(2))
 	require.Nil(t, sd.Txleases)
 	require.Nil(t, sd.Creatables)
+	require.Nil(t, sd.KvMods)
 
-	sd.UpsertTxLease(Txlease{}, basics.Round(10))
-	require.Equal(t, 1, len(sd.Txleases))
-	sd.UpsertCreatable(basics.CreatableIndex(5), ModifiedCreatable{})
-	require.Equal(t, 1, len(sd.Creatables))
+	sd.AddTxLease(Txlease{}, basics.Round(10))
+	require.Len(t, sd.Txleases, 1)
+	sd.AddCreatable(basics.CreatableIndex(5), ModifiedCreatable{})
+	require.Len(t, sd.Creatables, 1)
+	sd.AddKvMod("key", KvValueDelta{Data: []byte("value")})
+	require.Len(t, sd.KvMods, 1)
 
 	txLeaseMap := make(map[Txlease]basics.Round)
 	txLeaseMap[Txlease{}] = basics.Round(10)
@@ -116,6 +119,10 @@ func TestMakeStateDeltaMaps(t *testing.T) {
 	creatableMap[basics.CreatableIndex(5)] = ModifiedCreatable{}
 	require.Equal(t, sd.Creatables, creatableMap)
 
+	kvModMap := make(map[string]KvValueDelta)
+	kvModMap["key"] = KvValueDelta{Data: []byte("value")}
+	require.Equal(t, sd.KvMods, kvModMap)
+
 }
 
 func BenchmarkMakeStateDelta(b *testing.B) {

ledger/txtail.go

@@ -213,9 +213,6 @@ func (t *txTail) newBlock(blk bookkeeping.Block, delta ledgercore.StateDelta) {
 
 	t.tailMu.Lock()
 	defer t.tailMu.Unlock()
-	if delta.Txleases == nil {
-		delta.Txleases = make(map[ledgercore.Txlease]basics.Round)
-	}
 	t.recent[rnd] = roundLeases{
 		txleases: delta.Txleases,
 		proto:    config.Consensus[blk.CurrentProtocol],

ledger/txtail_test.go

@@ -279,7 +279,7 @@ func TestTxTailDeltaTracking(t *testing.T) {
 					LastValid: basics.Round(i + 50),
 					Intra:     0,
 				}
-				deltas.UpsertTxLease(ledgercore.Txlease{Sender: blk.Payset[0].Txn.Sender, Lease: blk.Payset[0].Txn.Lease}, basics.Round(i+50))
+				deltas.AddTxLease(ledgercore.Txlease{Sender: blk.Payset[0].Txn.Sender, Lease: blk.Payset[0].Txn.Lease}, basics.Round(i+50))
 
 				txtail.newBlock(blk, deltas)
 				txtail.committedUpTo(basics.Round(i))