chore(state): Copy Iterator Key() and Value() APIs return value…

…s. (cometbft#3541)

Related to cometbft/cometbft-db#168.

#### Changes
We recently updated cometbft-db `Iterator` type `Key()` and `Value()`
APIs to return their values directly instead of a copy.
Therefore, this PR ensures that code in cometbft that modifies or stores
references to the return value of those APIs creates a copy before
continuing.

---

#### PR checklist

~- [ ] Tests written/updated~
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

.changelog/unreleased/bug-fixes/3541-copy-iter-key-value.md

@@ -0,0 +1,3 @@
+- code that modifies or stores references to the return value
+  of Iterator Key() and Value() APIs creates a copy of it
+  ([\#3541](https://github.com/cometbft/cometbft/pull/3541))

light/store/db/db_key_layout.go

@@ -10,6 +10,8 @@ import (
 )
 
 type LightStoreKeyLayout interface {
+	// Implementations of ParseLBKey should create a copy of the key parameter,
+	// rather than modify it in place.
 	ParseLBKey(key []byte, storePrefix string) (height int64, err error)
 	LBKey(height int64, prefix string) []byte
 	SizeKey(prefix string) []byte

state/indexer/block/kv/kv.go

@@ -116,7 +116,10 @@ func getKeys(indexer BlockerIndexer) [][]byte {
 		panic(err)
 	}
 	for ; itr.Valid(); itr.Next() {
-		keys = append(keys, itr.Key())
+		key := make([]byte, len(itr.Key()))
+		copy(key, itr.Key())
+
+		keys = append(keys, key)
 	}
 	return keys
 }
@@ -537,8 +540,16 @@ func (*BlockerIndexer) setTmpHeights(tmpHeights map[string][]byte, it dbm.Iterat
 	// If we return attributes that occur within the same events, then store the event sequence in the
 	// result map as well
 	eventSeq, _ := parseEventSeqFromEventKey(it.Key())
-	retVal := it.Value()
-	tmpHeights[string(retVal)+strconv.FormatInt(eventSeq, 10)] = it.Value()
+
+	// value comes from cometbft-db Iterator interface Value() API.
+	// Therefore, we must make a copy before storing references to it.
+	var (
+		value   = it.Value()
+		valueCp = make([]byte, len(value))
+	)
+	copy(valueCp, value)
+
+	tmpHeights[string(valueCp)+strconv.FormatInt(eventSeq, 10)] = valueCp
 }
 
 // match returns all matching heights that meet a given query condition and start

state/txindex/kv/kv.go

@@ -632,12 +632,17 @@ type TxInfo struct {
 }
 
 func (*TxIndex) setTmpHashes(tmpHeights map[string]TxInfo, key, value []byte, height int64) {
+	// value comes from cometbft-db Iterator interface Value() API.
+	// Therefore, we must make a copy before storing references to it.
+	valueCp := make([]byte, len(value))
+	copy(valueCp, value)
+
 	eventSeq := extractEventSeqFromKey(key)
 	txInfo := TxInfo{
-		TxBytes: value,
+		TxBytes: valueCp,
 		Height:  height,
 	}
-	tmpHeights[string(value)+eventSeq] = txInfo
+	tmpHeights[string(valueCp)+eventSeq] = txInfo
 }
 
 // match returns all matching txs by hash that meet a given condition and start
@@ -859,9 +864,6 @@ func (txi *TxIndex) matchRange(
 
 LOOP:
 	for ; it.Valid(); it.Next() {
-		// TODO: We need to make a function for getting it.Key() as a byte slice with no copies.
-		// It currently copies the source data (which can change on a subsequent .Next() call) but that
-		// is not an issue for us.
 		key := it.Key()
 		if !isTagKey(key) {
 			continue

state/txindex/kv/utils.go

@@ -128,7 +128,10 @@ func getKeys(indexer *TxIndex) [][]byte {
 		panic(err)
 	}
 	for ; itr.Valid(); itr.Next() {
-		keys = append(keys, itr.Key())
+		key := make([]byte, len(itr.Key()))
+		copy(key, itr.Key())
+
+		keys = append(keys, key)
 	}
 	return keys
 }