diff --git a/.changelog/unreleased/bug-fixes/3541-copy-iter-key-value.md b/.changelog/unreleased/bug-fixes/3541-copy-iter-key-value.md new file mode 100644 index 00000000000..e44439acdc8 --- /dev/null +++ b/.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)) \ No newline at end of file diff --git a/light/store/db/db_key_layout.go b/light/store/db/db_key_layout.go index dee4274b0ef..0fb639f0f09 100644 --- a/light/store/db/db_key_layout.go +++ b/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 diff --git a/state/indexer/block/kv/kv.go b/state/indexer/block/kv/kv.go index 271b501ac8c..a93c94b4c6b 100644 --- a/state/indexer/block/kv/kv.go +++ b/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 diff --git a/state/txindex/kv/kv.go b/state/txindex/kv/kv.go index 1c4224cd8d8..c5f06486e2e 100644 --- a/state/txindex/kv/kv.go +++ b/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 diff --git a/state/txindex/kv/utils.go b/state/txindex/kv/utils.go index 467cad35488..1a0003f2227 100644 --- a/state/txindex/kv/utils.go +++ b/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 }