Ignore non-existing instances in GetUpdates (#2263)

Currently GetUpdates returns an error if it is queried for an unknown instance.
This is not correct, as instances can disappear. It should either ignore them,
or return a proof of missing instance. Both is possible, depending on the flag
sent to GetUpdates

byzcoin/api_test.go

@@ -351,47 +351,6 @@ func TestClient_SignerCounterDecoder(t *testing.T) {
 	require.Error(t, c.signerCounterDecoder(buf, &reply))
 }
 
-func TestClient_GetUpdates(t *testing.T) {
-	s := newSer(t, 1, testInterval)
-	defer s.local.CloseAll()
-
-	req := &GetUpdatesRequest{
-		Instances:     nil,
-		Flags:         0,
-		LatestBlockID: nil,
-	}
-	_, err := s.service().GetUpdates(req)
-	require.Error(t, err)
-
-	req.Instances = []IDVersion{{ConfigInstanceID, 0}}
-	req.LatestBlockID = s.genesis.SkipChainID()
-	gur, err := s.service().GetUpdates(req)
-	require.NoError(t, err)
-	require.Equal(t, 0, len(gur.Proofs))
-
-	req.Flags = GUFSendVersion0
-	gur, err = s.service().GetUpdates(req)
-	require.NoError(t, err)
-	require.Equal(t, 1, len(gur.Proofs))
-	require.True(t, gur.Proofs[0].Match(ConfigInstanceID[:]))
-
-	ctx, _ := createConfigTxWithCounter(t, time.Millisecond*100, *s.roster,
-		1e5, s, 1)
-	s.sendTxAndWait(t, ctx, 10)
-
-	req.Flags = 0
-	_, err = s.service().GetUpdates(req)
-	require.Error(t, err)
-	latest, err := s.service().db().GetLatest(s.genesis)
-	req.LatestBlockID = latest.Hash
-	require.NoError(t, err)
-
-	gur, err = s.service().GetUpdates(req)
-	require.NoError(t, err)
-	require.Equal(t, 1, len(gur.Proofs))
-	require.True(t, gur.Proofs[0].Match(ConfigInstanceID[:]))
-}
-
 const testServiceName = "TestByzCoin"
 
 type corruptedService struct {

byzcoin/service.go

@@ -624,17 +624,29 @@ func (s *Service) GetUpdates(pr *GetUpdatesRequest) (*GetUpdatesReply, error) {
 	sendVersion0 := pr.Flags&GUFSendVersion0 > 0
 	reply := &GetUpdatesReply{}
 	for _, idv := range pr.Instances {
-		_, ver, _, _, err := st.GetValues(idv.ID[:])
+		proof, err := st.GetProof(idv.ID[:])
 		if err != nil {
-			return nil, fmt.Errorf("couldn't read values of instance: %v", err)
+			return nil, fmt.Errorf("error while looking up proof: %v", err)
 		}
-		if ver <= idv.Version &&
-			!(sendVersion0 && ver == 0) {
+
+		// Only send missing proofs if flag is set
+		if !proof.Match(idv.ID[:]) {
+			if pr.Flags&GUFSendMissingProofs > 0 {
+				reply.Proofs = append(reply.Proofs, *proof)
+			}
 			continue
 		}
-		proof, err := st.GetProof(idv.ID[:])
+
+		// Check if it's a new version
+		var inst StateChangeBody
+		err = protobuf.Decode(proof.Get(idv.ID[:]), &inst)
 		if err != nil {
-			return nil, fmt.Errorf("error while looking up proof: %v", err)
+			return nil, fmt.Errorf("invalid instance stored in trie: %v",
+				err)
+		}
+		if inst.Version <= idv.Version &&
+			!(sendVersion0 && inst.Version == 0) {
+			continue
 		}
 		reply.Proofs = append(reply.Proofs, *proof)
 	}

byzcoin/service_test.go

@@ -2536,6 +2536,59 @@ func TestService_Repair(t *testing.T) {
 	require.Equal(t, finalRoot, newRoot)
 }
 
+func TestService_GetUpdates(t *testing.T) {
+	s := newSer(t, 1, testInterval)
+	defer s.local.CloseAll()
+
+	req := &GetUpdatesRequest{
+		Instances:     nil,
+		Flags:         0,
+		LatestBlockID: nil,
+	}
+	_, err := s.service().GetUpdates(req)
+	require.Error(t, err)
+
+	req.Instances = []IDVersion{{ConfigInstanceID, 0}}
+	req.LatestBlockID = s.genesis.SkipChainID()
+	gur, err := s.service().GetUpdates(req)
+	require.NoError(t, err)
+	require.Equal(t, 0, len(gur.Proofs))
+
+	req.Flags = GUFSendVersion0
+	gur, err = s.service().GetUpdates(req)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(gur.Proofs))
+	require.True(t, gur.Proofs[0].Match(ConfigInstanceID[:]))
+
+	ctx, _ := createConfigTxWithCounter(t, time.Millisecond*100, *s.roster,
+		1e5, s, 1)
+	s.sendTxAndWait(t, ctx, 10)
+
+	req.Flags = 0
+	_, err = s.service().GetUpdates(req)
+	require.Error(t, err)
+	latest, err := s.service().db().GetLatest(s.genesis)
+	req.LatestBlockID = latest.Hash
+	require.NoError(t, err)
+
+	gur, err = s.service().GetUpdates(req)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(gur.Proofs))
+	require.True(t, gur.Proofs[0].Match(ConfigInstanceID[:]))
+
+	invID := sha256.Sum256([]byte("invalid instance"))
+	req.Instances = append(req.Instances, IDVersion{invID, 0})
+	gur, err = s.service().GetUpdates(req)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(gur.Proofs))
+
+	req.Flags = GUFSendMissingProofs
+	gur, err = s.service().GetUpdates(req)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(gur.Proofs))
+	require.False(t, gur.Proofs[1].Match(invID[:]))
+}
+
 func createBadConfigTx(t *testing.T, s *ser, intervalBad, szBad bool) (ClientTransaction, ChainConfig) {
 	switch {
 	case intervalBad:

byzcoin/struct.go

@@ -725,4 +725,7 @@ const (
 	// GUFSendVersion0 will make GetUpdates to send all instances with
 	// version 0, even those that are note updated.
 	GUFSendVersion0 = GetUpdatesFlags(1 << iota)
+	// GUFSendMissingProofs will make GetUpdates send proofs for missing
+	// instances. If not present, missing instances are ignored.
+	GUFSendMissingProofs
 )