sync -- change proof request can return range proof

proto/sync/sync.proto

@@ -23,7 +23,7 @@ service DB {
 
   rpc GetProof(GetProofRequest) returns (GetProofResponse);
 
-  rpc GetChangeProof(GetChangeProofRequest) returns (ChangeProof);
+  rpc GetChangeProof(GetChangeProofRequest) returns (GetChangeProofResponse);
   rpc VerifyChangeProof(VerifyChangeProofRequest) returns (VerifyChangeProofResponse);
   rpc CommitChangeProof(CommitChangeProofRequest) returns (google.protobuf.Empty);
 
@@ -75,6 +75,14 @@ message GetChangeProofRequest {
   uint32 key_limit = 5;
 }
 
+message GetChangeProofResponse {
+  oneof response {
+    ChangeProof change_proof = 1;
+    // True iff server errored with merkledb.ErrInsufficientHistory.
+    bool root_not_present = 2;
+  }
+}
+
 message VerifyChangeProofRequest {
   ChangeProof proof = 1;
   bytes start_key = 2;
@@ -118,10 +126,9 @@ message CommitRangeProofRequest {
 }
 
 message ChangeProof {
-  bool had_roots_in_history = 1; // TODO remove
-  repeated ProofNode start_proof = 2;
-  repeated ProofNode end_proof = 3;
-  repeated KeyChange key_changes = 4;
+  repeated ProofNode start_proof = 1;
+  repeated ProofNode end_proof = 2;
+  repeated KeyChange key_changes = 3;
 }
 
 message RangeProof {

x/merkledb/db.go

@@ -57,6 +57,8 @@ type ChangeProofer interface {
 	// GetChangeProof returns a proof for a subset of the key/value changes in key range
 	// [start, end] that occurred between [startRootID] and [endRootID].
 	// Returns at most [maxLength] key/value pairs.
+	// Returns [ErrInsufficientHistory] if this node has insufficient history
+	// to generate the proof.
 	GetChangeProof(
 		ctx context.Context,
 		startRootID ids.ID,
@@ -68,7 +70,7 @@ type ChangeProofer interface {
 
 	// Returns nil iff all of the following hold:
 	//   - [start] <= [end].
-	//   - [proof] is non-empty iff [proof.HadRootsInHistory].
+	//   - [proof] is non-empty.
 	//   - All keys in [proof.KeyValues] and [proof.DeletedKeys] are in [start, end].
 	//     If [start] is empty, all keys are considered > [start].
 	//     If [end] is nothing, all keys are considered < [end].
@@ -77,7 +79,6 @@ type ChangeProofer interface {
 	//   - When the keys in [proof.KeyValues] are added to [db] and the keys in [proof.DeletedKeys]
 	//     are removed from [db], the root ID of [db] is [expectedEndRootID].
 	//
-	// Assumes [db.lock] isn't held.
 	// This is defined on Database instead of ChangeProof because it accesses
 	// database internals.
 	VerifyChangeProof(
@@ -581,14 +582,7 @@ func (db *merkleDB) GetChangeProof(
 		return nil, database.ErrClosed
 	}
 
-	result := &ChangeProof{
-		HadRootsInHistory: true,
-	}
 	changes, err := db.history.getValueChanges(startRootID, endRootID, start, end, maxLength)
-	if err == ErrRootIDNotPresent {
-		result.HadRootsInHistory = false
-		return result, nil
-	}
 	if err != nil {
 		return nil, err
 	}
@@ -599,7 +593,9 @@ func (db *merkleDB) GetChangeProof(
 	changedKeys := maps.Keys(changes.values)
 	utils.Sort(changedKeys)
 
-	result.KeyChanges = make([]KeyChange, 0, len(changedKeys))
+	result := &ChangeProof{
+		KeyChanges: make([]KeyChange, 0, len(changedKeys)),
+	}
 
 	for _, key := range changedKeys {
 		change := changes.values[key]
@@ -988,28 +984,17 @@ func (*merkleDB) CommitToDB(context.Context) error {
 	return nil
 }
 
+// Assumes [db.lock] isn't held.
 func (db *merkleDB) VerifyChangeProof(
 	ctx context.Context,
 	proof *ChangeProof,
 	start []byte,
 	end maybe.Maybe[[]byte],
 	expectedEndRootID ids.ID,
 ) error {
-	if end.HasValue() && bytes.Compare(start, end.Value()) > 0 {
-		return ErrStartAfterEnd
-	}
-
-	if !proof.HadRootsInHistory {
-		// The node we requested the proof from didn't have sufficient
-		// history to fulfill this request.
-		if !proof.Empty() {
-			// cannot have any changes if the root was missing
-			return ErrDataInMissingRootProof
-		}
-		return nil
-	}
-
 	switch {
+	case end.HasValue() && bytes.Compare(start, end.Value()) > 0:
+		return ErrStartAfterEnd
 	case proof.Empty():
 		return ErrNoMerkleProof
 	case end.HasValue() && len(proof.KeyChanges) == 0 && len(proof.EndProof) == 0:

x/merkledb/history.go

@@ -15,10 +15,7 @@ import (
 	"github.com/ava-labs/avalanchego/utils/set"
 )
 
-var (
-	ErrStartRootNotFound = errors.New("start root is not before end root in history")
-	ErrRootIDNotPresent  = errors.New("root id is not present in history")
-)
+var ErrInsufficientHistory = errors.New("insufficient history to generate proof")
 
 // stores previous trie states
 type trieHistory struct {
@@ -74,9 +71,17 @@ func newTrieHistory(maxHistoryLookback int) *trieHistory {
 	}
 }
 
-// Returns up to [maxLength] key-value pair changes with keys in [start, end] that
-// occurred between [startRoot] and [endRoot].
-func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start []byte, end maybe.Maybe[[]byte], maxLength int) (*changeSummary, error) {
+// Returns up to [maxLength] key-value pair changes with keys in
+// [start, end] that occurred between [startRoot] and [endRoot].
+// Returns [ErrInsufficientHistory] if the history is insufficient
+// to generate the proof.
+func (th *trieHistory) getValueChanges(
+	startRoot ids.ID,
+	endRoot ids.ID,
+	start []byte,
+	end maybe.Maybe[[]byte],
+	maxLength int,
+) (*changeSummary, error) {
 	if maxLength <= 0 {
 		return nil, fmt.Errorf("%w but was %d", ErrInvalidMaxLength, maxLength)
 	}
@@ -86,17 +91,21 @@ func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start []byte,
 	}
 
 	// [endRootChanges] is the last change in the history resulting in [endRoot].
+	// TODO when we update to minimum go version 1.20.X, make this return another
+	// wrapped error ErrNoEndRoot. In NetworkServer.HandleChangeProofRequest, if we return
+	// that error, we know we shouldn't try to generate a range proof since we
+	// lack the necessary history.
 	endRootChanges, ok := th.lastChanges[endRoot]
 	if !ok {
-		return nil, ErrRootIDNotPresent
+		return nil, fmt.Errorf("%w: end root %s not found", ErrInsufficientHistory, endRoot)
 	}
 
 	// Confirm there's a change resulting in [startRoot] before
 	// a change resulting in [endRoot] in the history.
 	// [startRootChanges] is the last appearance of [startRoot].
 	startRootChanges, ok := th.lastChanges[startRoot]
 	if !ok {
-		return nil, ErrStartRootNotFound
+		return nil, fmt.Errorf("%w: start root %s not found", ErrInsufficientHistory, startRoot)
 	}
 
 	var (
@@ -131,7 +140,10 @@ func (th *trieHistory) getValueChanges(startRoot, endRoot ids.ID, start []byte,
 			}
 
 			if i == 0 {
-				return nil, ErrStartRootNotFound
+				return nil, fmt.Errorf(
+					"%w: start root %s not found before end root %s",
+					ErrInsufficientHistory, startRoot, endRoot,
+				)
 			}
 		}
 	}
@@ -216,7 +228,7 @@ func (th *trieHistory) getChangesToGetToRoot(rootID ids.ID, start []byte, end ma
 	// [lastRootChange] is the last change in the history resulting in [rootID].
 	lastRootChange, ok := th.lastChanges[rootID]
 	if !ok {
-		return nil, ErrRootIDNotPresent
+		return nil, ErrInsufficientHistory
 	}
 
 	var (

x/merkledb/history_test.go

@@ -192,7 +192,7 @@ func Test_History_Bad_GetValueChanges_Input(t *testing.T) {
 	require.ErrorIs(err, ErrInvalidMaxLength)
 
 	_, err = db.history.getValueChanges(endRoot, startRoot, nil, maybe.Nothing[[]byte](), 1)
-	require.ErrorIs(err, ErrStartRootNotFound)
+	require.ErrorIs(err, ErrInsufficientHistory)
 
 	// trigger the first root to be deleted by exiting the lookback window
 	batch = db.NewBatch()
@@ -201,7 +201,7 @@ func Test_History_Bad_GetValueChanges_Input(t *testing.T) {
 
 	// now this root should no longer be present
 	_, err = db.history.getValueChanges(toBeDeletedRoot, endRoot, nil, maybe.Nothing[[]byte](), 1)
-	require.ErrorIs(err, ErrStartRootNotFound)
+	require.ErrorIs(err, ErrInsufficientHistory)
 
 	// same start/end roots should yield an empty changelist
 	changes, err := db.history.getValueChanges(endRoot, endRoot, nil, maybe.Nothing[[]byte](), 10)
@@ -261,7 +261,7 @@ func Test_History_Trigger_History_Queue_Looping(t *testing.T) {
 
 	// proof from first root shouldn't be generatable since it should have been removed from the history
 	_, err = db.GetRangeProofAtRoot(context.Background(), origRootID, []byte("k"), maybe.Some([]byte("key3")), 10)
-	require.ErrorIs(err, ErrRootIDNotPresent)
+	require.ErrorIs(err, ErrInsufficientHistory)
 }
 
 func Test_History_Values_Lookup_Over_Queue_Break(t *testing.T) {
@@ -661,7 +661,7 @@ func TestHistoryGetChangesToRoot(t *testing.T) {
 		{
 			name:        "unknown root ID",
 			rootID:      ids.GenerateTestID(),
-			expectedErr: ErrRootIDNotPresent,
+			expectedErr: ErrInsufficientHistory,
 		},
 		{
 			name:   "most recent change",

x/merkledb/proof.go

@@ -499,12 +499,6 @@ type ChangeProof struct {
 	// Invariant: At least one of [StartProof], [EndProof], or
 	// [KeyChanges] is non-empty.
 
-	// If false, the node that created this doesn't have
-	// sufficient history to generate a change proof and
-	// all other fields must be empty.
-	// Otherwise at least one other field is non-empty.
-	HadRootsInHistory bool
-
 	// A proof that the smallest key in the requested range does/doesn't
 	// exist in the trie with the requested start root.
 	// Empty if no lower bound on the requested range was given.
@@ -592,10 +586,9 @@ func (proof *ChangeProof) ToProto() *pb.ChangeProof {
 	}
 
 	return &pb.ChangeProof{
-		HadRootsInHistory: proof.HadRootsInHistory,
-		StartProof:        startProof,
-		EndProof:          endProof,
-		KeyChanges:        keyChanges,
+		StartProof: startProof,
+		EndProof:   endProof,
+		KeyChanges: keyChanges,
 	}
 }
 
@@ -604,8 +597,6 @@ func (proof *ChangeProof) UnmarshalProto(pbProof *pb.ChangeProof) error {
 		return ErrNilChangeProof
 	}
 
-	proof.HadRootsInHistory = pbProof.HadRootsInHistory
-
 	proof.StartProof = make([]ProofNode, len(pbProof.StartProof))
 	for i, protoNode := range pbProof.StartProof {
 		if err := proof.StartProof[i].UnmarshalProto(protoNode); err != nil {
@@ -692,6 +683,12 @@ func (proof *ChangeProof) Empty() bool {
 		len(proof.StartProof) == 0 && len(proof.EndProof) == 0
 }
 
+// Exactly one of [ChangeProof] or [RangeProof] is non-nil.
+type ChangeOrRangeProof struct {
+	ChangeProof *ChangeProof
+	RangeProof  *RangeProof
+}
+
 // Returns nil iff both hold:
 // 1. [kvs] is sorted by key in increasing order.
 // 2. All keys in [kvs] are in the range [start, end].