Simplify ChangeProofs to have a change list instead of puts and deletes lists (#1385)

Author: dboehm-avalabs

x/merkledb/codec.go

@@ -26,14 +26,14 @@ const (
 	minCodecVersionLen   = minVarIntLen
 	minSerializedPathLen = minVarIntLen
 	minByteSliceLen      = minVarIntLen
-	minDeletedKeyLen     = minByteSliceLen
 	minMaybeByteSliceLen = boolLen
 	minProofPathLen      = minVarIntLen
 	minKeyValueLen       = 2 * minByteSliceLen
+	minKeyChangeLen      = minByteSliceLen + minMaybeByteSliceLen
 	minProofNodeLen      = minSerializedPathLen + minMaybeByteSliceLen + minVarIntLen
 	minProofLen          = minCodecVersionLen + minProofPathLen + minByteSliceLen
-	minChangeProofLen    = minCodecVersionLen + +boolLen + 2*minProofPathLen + 2*minVarIntLen
-	minRangeProofLen     = minCodecVersionLen + +2*minProofPathLen + minVarIntLen
+	minChangeProofLen    = minCodecVersionLen + boolLen + 2*minProofPathLen + minVarIntLen
+	minRangeProofLen     = minCodecVersionLen + 2*minProofPathLen + minVarIntLen
 	minDBNodeLen         = minCodecVersionLen + minMaybeByteSliceLen + minVarIntLen
 	minHashValuesLen     = minCodecVersionLen + minVarIntLen + minMaybeByteSliceLen + minSerializedPathLen
 	minProofNodeChildLen = minVarIntLen + idLen
@@ -150,20 +150,11 @@ func (c *codecImpl) EncodeChangeProof(version uint16, proof *ChangeProof) ([]byt
 	if err := c.encodeProofPath(buf, proof.EndProof); err != nil {
 		return nil, err
 	}
-	if err := c.encodeInt(buf, len(proof.KeyValues)); err != nil {
-		return nil, err
-	}
-	for _, kv := range proof.KeyValues {
-		if err := c.encodeKeyValue(kv, buf); err != nil {
-			return nil, err
-		}
-	}
-
-	if err := c.encodeInt(buf, len(proof.DeletedKeys)); err != nil {
+	if err := c.encodeInt(buf, len(proof.KeyChanges)); err != nil {
 		return nil, err
 	}
-	for _, key := range proof.DeletedKeys {
-		if err := c.encodeByteSlice(buf, key); err != nil {
+	for _, kv := range proof.KeyChanges {
+		if err := c.encodeKeyChange(kv, buf); err != nil {
 			return nil, err
 		}
 	}
@@ -343,36 +334,19 @@ func (c *codecImpl) DecodeChangeProof(b []byte, proof *ChangeProof) (uint16, err
 		return 0, err
 	}
 
-	numKeyValues, err := c.decodeInt(src)
+	numKeyChanges, err := c.decodeInt(src)
 	if err != nil {
 		return 0, err
 	}
-	if numKeyValues < 0 {
+	if numKeyChanges < 0 {
 		return 0, errNegativeNumKeyValues
 	}
-	if numKeyValues > src.Len()/minKeyValueLen {
+	if numKeyChanges > src.Len()/minKeyChangeLen {
 		return 0, io.ErrUnexpectedEOF
 	}
-	proof.KeyValues = make([]KeyValue, numKeyValues)
-	for i := range proof.KeyValues {
-		if proof.KeyValues[i], err = c.decodeKeyValue(src); err != nil {
-			return 0, err
-		}
-	}
-
-	numDeletedKeys, err := c.decodeInt(src)
-	if err != nil {
-		return 0, err
-	}
-	if numDeletedKeys < 0 {
-		return 0, errNegativeNumKeyValues
-	}
-	if numDeletedKeys > src.Len()/minDeletedKeyLen {
-		return 0, io.ErrUnexpectedEOF
-	}
-	proof.DeletedKeys = make([][]byte, numDeletedKeys)
-	for i := range proof.DeletedKeys {
-		if proof.DeletedKeys[i], err = c.decodeByteSlice(src); err != nil {
+	proof.KeyChanges = make([]KeyChange, numKeyChanges)
+	for i := range proof.KeyChanges {
+		if proof.KeyChanges[i], err = c.decodeKeyChange(src); err != nil {
 			return 0, err
 		}
 	}
@@ -499,6 +473,24 @@ func (c *codecImpl) decodeDBNode(b []byte, n *dbNode) (uint16, error) {
 	return codecVersion, err
 }
 
+func (c *codecImpl) decodeKeyChange(src *bytes.Reader) (KeyChange, error) {
+	if minKeyChangeLen > src.Len() {
+		return KeyChange{}, io.ErrUnexpectedEOF
+	}
+
+	var (
+		result KeyChange
+		err    error
+	)
+	if result.Key, err = c.decodeByteSlice(src); err != nil {
+		return result, err
+	}
+	if result.Value, err = c.decodeMaybeByteSlice(src); err != nil {
+		return result, err
+	}
+	return result, nil
+}
+
 func (c *codecImpl) decodeKeyValue(src *bytes.Reader) (KeyValue, error) {
 	if minKeyValueLen > src.Len() {
 		return KeyValue{}, io.ErrUnexpectedEOF
@@ -517,6 +509,16 @@ func (c *codecImpl) decodeKeyValue(src *bytes.Reader) (KeyValue, error) {
 	return result, nil
 }
 
+func (c *codecImpl) encodeKeyChange(kv KeyChange, dst io.Writer) error {
+	if err := c.encodeByteSlice(dst, kv.Key); err != nil {
+		return err
+	}
+	if err := c.encodeMaybeByteSlice(dst, kv.Value); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *codecImpl) encodeKeyValue(kv KeyValue, dst io.Writer) error {
 	if err := c.encodeByteSlice(dst, kv.Key); err != nil {
 		return err

x/merkledb/codec_test.go

@@ -52,19 +52,22 @@ func newRandomProofNode(r *rand.Rand) ProofNode {
 	}
 }
 
-func newKeyValues(r *rand.Rand, num uint) []KeyValue {
-	keyValues := make([]KeyValue, num)
-	for i := range keyValues {
+func newKeyChanges(r *rand.Rand, num uint) []KeyChange {
+	keyChanges := make([]KeyChange, num)
+	for i := range keyChanges {
 		key := make([]byte, r.Intn(32)) // #nosec G404
 		_, _ = r.Read(key)              // #nosec G404
 		val := make([]byte, r.Intn(32)) // #nosec G404
 		_, _ = r.Read(val)              // #nosec G404
-		keyValues[i] = KeyValue{
+		if len(val) == 0 {
+			val = nil
+		}
+		keyChanges[i] = KeyChange{
 			Key:   key,
-			Value: val,
+			Value: Some(val),
 		}
 	}
-	return keyValues
+	return keyChanges
 }
 
 func nilEmptySlices(dest interface{}) {
@@ -384,6 +387,7 @@ func FuzzCodecChangeProofDeterministic(f *testing.F) {
 			randSeed int,
 			hadRootsInHistory bool,
 			numProofNodes uint,
+			numChangedKeys uint,
 			numDeletedKeys uint,
 		) {
 			require := require.New(t)
@@ -397,18 +401,18 @@ func FuzzCodecChangeProofDeterministic(f *testing.F) {
 				endProofNodes[i] = newRandomProofNode(r)
 			}
 
-			deletedKeys := make([][]byte, numDeletedKeys)
-			for i := range deletedKeys {
-				deletedKeys[i] = make([]byte, r.Intn(32)) // #nosec G404
-				_, _ = r.Read(deletedKeys[i])             // #nosec G404
+			keyChanges := newKeyChanges(r, numChangedKeys)
+			for i := uint(0); i < numDeletedKeys; i++ {
+				keyToDelete := make([]byte, r.Intn(32)) // #nosec G404
+				_, _ = r.Read(keyToDelete)              // #nosec G404
+				keyChanges = append(keyChanges, KeyChange{Key: keyToDelete})
 			}
 
 			proof := ChangeProof{
 				HadRootsInHistory: hadRootsInHistory,
 				StartProof:        startProofNodes,
 				EndProof:          endProofNodes,
-				KeyValues:         newKeyValues(r, numProofNodes),
-				DeletedKeys:       deletedKeys,
+				KeyChanges:        keyChanges,
 			}
 
 			proofBytes, err := Codec.EncodeChangeProof(Version, &proof)
@@ -586,36 +590,19 @@ func TestCodec_DecodeChangeProof(t *testing.T) {
 		HadRootsInHistory: true,
 		StartProof:        nil,
 		EndProof:          nil,
-		KeyValues:         nil,
-		DeletedKeys:       nil,
+		KeyChanges:        nil,
 	}
 
 	proofBytes, err := Codec.EncodeChangeProof(Version, &proof)
 	require.NoError(err)
 
-	// Remove key-values length and deleted keys length (both 0) from end
-	proofBytes = proofBytes[:len(proofBytes)-2*minVarIntLen]
+	// Remove key-values length (0) from end
+	proofBytes = proofBytes[:len(proofBytes)-minVarIntLen]
 
-	// Put key-values length of -1 and deleted keys length of 0
+	// Put key-values length of -1
 	proofBytesBuf := bytes.NewBuffer(proofBytes)
 	err = Codec.(*codecImpl).encodeInt(proofBytesBuf, -1)
 	require.NoError(err)
-	err = Codec.(*codecImpl).encodeInt(proofBytesBuf, 0)
-	require.NoError(err)
-
-	_, err = Codec.DecodeChangeProof(proofBytesBuf.Bytes(), &parsedProof)
-	require.ErrorIs(err, errNegativeNumKeyValues)
-
-	proofBytes = proofBytesBuf.Bytes()
-	proofBytes = proofBytes[:len(proofBytes)-2*minVarIntLen]
-	proofBytesBuf = bytes.NewBuffer(proofBytes)
-
-	// Remove key-values length and deleted keys length from end
-	// Put key-values length of 0 and deleted keys length of -1
-	err = Codec.(*codecImpl).encodeInt(proofBytesBuf, 0)
-	require.NoError(err)
-	err = Codec.(*codecImpl).encodeInt(proofBytesBuf, -1)
-	require.NoError(err)
 
 	_, err = Codec.DecodeChangeProof(proofBytesBuf.Bytes(), &parsedProof)
 	require.ErrorIs(err, errNegativeNumKeyValues)

x/merkledb/db.go

@@ -516,24 +516,23 @@ func (db *Database) GetChangeProof(
 	})
 
 	// TODO: sync.pool these buffers
-	result.KeyValues = make([]KeyValue, 0, len(changedKeys))
-	result.DeletedKeys = make([][]byte, 0, len(changedKeys))
+	result.KeyChanges = make([]KeyChange, 0, len(changedKeys))
 
 	for _, key := range changedKeys {
 		change := changes.values[key]
 		serializedKey := key.Serialize().Value
 
-		if change.after.IsNothing() {
-			result.DeletedKeys = append(result.DeletedKeys, serializedKey)
-		} else {
-			result.KeyValues = append(result.KeyValues, KeyValue{
-				Key: serializedKey,
-				// create a copy so edits of the []byte don't affect the db
-				Value: slices.Clone(change.after.value),
-			})
-		}
+		result.KeyChanges = append(result.KeyChanges, KeyChange{
+			Key: serializedKey,
+			// create a copy so edits of the []byte don't affect the db
+			Value: Clone(change.after),
+		})
+	}
+
+	largestKey := end
+	if len(result.KeyChanges) > 0 {
+		largestKey = result.KeyChanges[len(result.KeyChanges)-1].Key
 	}
-	largestKey := result.getLargestKey(end)
 
 	// Since we hold [db.commitlock] we must still have sufficient
 	// history to recreate the trie at [endRootID].
@@ -1121,20 +1120,18 @@ func (db *Database) prepareBatchView(ops []database.BatchOp) (*trieView, error)
 // inserted and the key/value pairs in [proof.DeletedKeys] removed.
 // Assumes [db.commitLock] is locked.
 func (db *Database) prepareChangeProofView(proof *ChangeProof) (*trieView, error) {
-	view, err := db.newUntrackedView(len(proof.KeyValues))
+	view, err := db.newUntrackedView(len(proof.KeyChanges))
 	if err != nil {
 		return nil, err
 	}
 	// Don't need to lock [view] because nobody else has a reference to it.
 
-	for _, kv := range proof.KeyValues {
-		if err := view.insert(kv.Key, kv.Value); err != nil {
-			return nil, err
-		}
-	}
-
-	for _, keyToDelete := range proof.DeletedKeys {
-		if err := view.remove(keyToDelete); err != nil {
+	for _, kv := range proof.KeyChanges {
+		if kv.Value.IsNothing() {
+			if err := view.remove(kv.Key); err != nil {
+				return nil, err
+			}
+		} else if err := view.insert(kv.Key, kv.Value.value); err != nil {
 			return nil, err
 		}
 	}

x/merkledb/db_test.go

@@ -820,7 +820,7 @@ func runRandDBTest(require *require.Assertions, r *rand.Rand, rt randTest) {
 				root,
 			)
 			require.NoError(err)
-			require.LessOrEqual(len(changeProof.KeyValues)+len(changeProof.DeletedKeys), 100)
+			require.LessOrEqual(len(changeProof.KeyChanges), 100)
 		case opWriteBatch:
 			oldRoot, err := db.GetMerkleRoot(context.Background())
 			require.NoError(err)