Skip to content

Commit

Permalink
Ignore non-existing instances in GetUpdates
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ineiti committed May 5, 2020
1 parent 0bd647f commit 5d632d8
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 47 deletions.
41 changes: 0 additions & 41 deletions byzcoin/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 18 additions & 6 deletions byzcoin/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
53 changes: 53 additions & 0 deletions byzcoin/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions byzcoin/struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

0 comments on commit 5d632d8

Please sign in to comment.