Optimize chain optimization

The current chain optimization is badly defined, as you need to give it a block, but for some blocks the optimization will always fail, as the nodes will not accept the new forward-links. This PR re-defines the chain optimization: - if given a skipchain-ID it optimizes the route from the genesis to the latest block - if given a random skipblock-ID, it optimizes that block only This will allow to crawl through all blocks and ask an optimization for every block that is lacking forward-links.
dedis · Dec 8, 2020 · 5f8d8a4 · 5f8d8a4
1 parent 9e1c4c8
commit 5f8d8a4
Show file tree

Hide file tree

Showing 5 changed files with 248 additions and 49 deletions.
diff --git a/scmgr/app.go b/scmgr/app.go
@@ -480,16 +480,12 @@ func scOptimize(c *cli.Context) error {
 
 	// If a genesis block is provided, we optimize the entire chain
 	if sb.Index == 0 {
-		reply, err := cl.GetUpdateChain(roster, id)
-		if err != nil {
-			return fmt.Errorf("couldn't get the latest block: %v", err)
-		}
-
-		sb = reply.Update[len(reply.Update)-1]
+		log.Infof("Optimizing chain %x", sb.SkipChainID())
+	} else {
+		log.Infof("Optimizing block with index %d in chain %x",
+			sb.Index, sb.SkipChainID())
 	}
 
-	log.Infof("Optimizing chain %x for block at index %d...", sb.SkipChainID(), sb.Index)
-
 	reply, err := cl.OptimizeProof(roster, sb.Hash)
 	if err != nil {
 		return fmt.Errorf("couldn't optimize the proof: %v", err)

diff --git a/skipchain/msgs.go b/skipchain/msgs.go
@@ -88,9 +88,14 @@ type StoreSkipBlockReply struct {
 	Latest   *SkipBlock
 }
 
-// OptimizeProofRequest is request to create missing forward links
+// OptimizeProofRequest is request to create missing forward links.
+// If the ID is the skipchain-ID,
+// the proofs from the genesis block to the latest block are optimized.
+// If the ID is a block in the chain,
+// the proofs from that block only are optimized.
 type OptimizeProofRequest struct {
-	ID     SkipBlockID
+	ID SkipBlockID
+	// Deprecated: will not be used in the new call to OptimizeProof
 	Roster *onet.Roster
 }
 

diff --git a/skipchain/skipchain.go b/skipchain/skipchain.go
@@ -15,6 +15,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"math"
 	"reflect"
 	"runtime"
 	"strconv"
@@ -423,18 +424,115 @@ func sendForwardLinkRequest(ro *onet.Roster, req *ForwardSignature, reply *Forwa
 	return err
 }
 
-// OptimizeProof creates missing forward links to optimize the proof of the block
-// at the given index.
+// OptimizeProof creates missing forward links to optimize the proof either
+//   - from the genesis block to the latest block of the given skipchain-ID
+//   - for the given block
 func (s *Service) OptimizeProof(req *OptimizeProofRequest) (*OptimizeProofReply, error) {
-	pr, err := s.db.GetProofForID(req.ID)
+	sb := s.db.GetByID(req.ID)
+	if sb == nil {
+		return nil, xerrors.New("didn't find that block-id in our DB")
+	}
+	if sb.Index == 0 {
+		return s.optimizeChain(req.ID)
+	}
+	return s.optimizeBlock(sb)
+}
+
+// optimizeBlock adds missing forward-links to one block only.
+func (s *Service) optimizeBlock(sb *SkipBlock) (*OptimizeProofReply, error) {
+	latest, err := s.db.GetLatestByID(sb.SkipChainID())
+	if err != nil {
+		return nil, xerrors.Errorf("couldn't get latest block: %v", err)
+	}
+
+	maxHeight, _ := sb.pathForIndex(latest.Index)
+	if sb.GetForwardLen() > maxHeight {
+		return nil, xerrors.New("nothing to optimize for this block")
+	}
+
+	optimized := false
+	for height := sb.GetForwardLen(); height <= maxHeight; height++ {
+		indexTo := sb.Index +
+			int(math.Pow(float64(sb.BaseHeight), float64(height)))
+		if indexTo > latest.Index {
+			break
+		}
+		_, sbs, err := s.db.GetFullProof(sb.SkipChainID(), indexTo)
+		if err != nil {
+			return nil, xerrors.Errorf("couldn't get block at index %d: %v",
+				indexTo, err)
+		}
+		to := sbs[len(sbs)-1]
+
+		log.Lvlf2("Adding forward-link level %d to block %d", height, sb.Index)
+		if err := s.addForwardLink(height, sb, to); err != nil {
+			log.Warnf("Couldn't add a forwardlink from %d to %d "+
+				"with height %d: %v",
+				sb.Index, to.Index, height, err)
+		} else {
+			optimized = true
+		}
+	}
+
+	if !optimized {
+		return nil, xerrors.New("couldn't optimize this block")
+	}
+
+	pr, err := s.db.GetProofForID(sb.Hash)
+	if err != nil {
+		return nil, xerrors.Errorf("couldn't get proof for latest block: %v",
+			err)
+	}
+	if err := s.optimizeSend(pr); err != nil {
+		return nil, xerrors.Errorf("couldn't propagate proof: %v", err)
+	}
+	return &OptimizeProofReply{Proof: pr}, nil
+}
+
+// addForwardLink asks the roster of the block to create the forward-links,
+// but does not propagate those links.
+func (s *Service) addForwardLink(targetHeight int, from, to *SkipBlock) error {
+	req := &ForwardSignature{
+		TargetHeight: targetHeight,
+		Previous:     from.Hash,
+		Newest:       to,
+	}
+	reply := &ForwardSignatureReply{}
+
+	log.Lvlf2("requesting missing forward-link from index %d to"+
+		" %d with height %d", from.Index, to.Index, targetHeight)
+	// The signature must be created by the roster of the block
+	err := sendForwardLinkRequest(from.Roster, req, reply)
+
+	if err != nil {
+		return xerrors.Errorf("could not create a missing forward link: %v",
+			err)
+	}
+
+	// save the new forward link
+	err = from.AddForwardLink(reply.Link, targetHeight)
+	if err != nil {
+		return xerrors.Errorf("could not store the missing forward-link"+
+			": %v", err)
+	}
+	return nil
+}
+
+// optimizeChain optimizes all forward-links from the genesis to the latest
+// block.
+func (s *Service) optimizeChain(scID SkipBlockID) (
+	*OptimizeProofReply, error) {
+	sbs, err := s.db.GetProof(scID)
 	if err != nil {
 		return nil, err
 	}
+	pr := Proof(sbs)
 
 	target := pr[len(pr)-1]
 	index := 0
 	h := 0
 	newProof := Proof{}
+	optimized := false
 
 	for _, sb := range pr[:len(pr)-1] {
 		if sb.Index < index {
@@ -443,47 +541,62 @@ func (s *Service) OptimizeProof(req *OptimizeProofRequest) (*OptimizeProofReply,
 		}
 
 		h, index = sb.pathForIndex(target.Index)
-
 		if h > 0 && len(sb.ForwardLink) <= h {
 			to := pr.Search(index)
 			if to == nil {
 				return nil, fmt.Errorf("chain is inconsistent: block at index %d not found", index)
 			}
 
-			req := &ForwardSignature{
-				TargetHeight: h,
-				Previous:     sb.Hash,
-				Newest:       to,
-			}
-			reply := &ForwardSignatureReply{}
-
-			log.Lvlf2("requesting missing forward-link at index %d with height %d / %d", sb.Index, h, index)
-			// The signature must be created by the roster of the block
-			err := sendForwardLinkRequest(sb.Roster, req, reply)
-
+			err := s.addForwardLink(h, sb, to)
 			if err != nil {
-				log.Error("could not create a missing forward link:", err)
-				// reset the index to try to create lower levels
+				log.Errorf("Couldn't create forward-link: %v", err)
 				index = sb.Index
 			} else {
-				// save the new forward link
-				err = sb.AddForwardLink(reply.Link, h)
-				if err != nil {
-					log.Error("could not store the missing forward-link:", err)
-					index = sb.Index
-				}
+				optimized = true
 			}
 		}
 
+		// sb is already updated, as we're only using a pointer.
 		newProof = append(newProof, sb)
 	}
 
 	newProof = append(newProof, target)
 
-	// Propagate the optimized proof to the given roster
-	err = s.startPropagation(s.propagateProof, req.Roster, &PropagateProof{newProof})
+	if optimized {
+		if err := s.optimizeSend(newProof); err != nil {
+			return nil, xerrors.Errorf("couldn't propagate proof: %v", err)
+		}
+	}
+
+	return &OptimizeProofReply{newProof}, nil
+}
+
+// optimizeSend propagates the new blocks to all nodes.
+func (s *Service) optimizeSend(newProof Proof) error {
+	log.Lvl2("Done creating forwardlinks, propagating new proofs")
+	var nodes []*network.ServerIdentity
+	for i, pr := range newProof {
+		for _, si := range pr.Roster.List {
+			exists := false
+			for _, siExist := range nodes {
+				exists = exists || siExist.Equal(si)
+			}
+			if !exists {
+				nodes = append(nodes, si)
+			}
+		}
+		if pr.GetForwardLen() > 0 {
+			to := pr.ForwardLink[pr.GetForwardLen()-1].To
+			log.Lvlf3("%d: Block %d / %x with fl-len %d pointing to %x",
+				i, pr.Index, pr.Hash[:], pr.GetForwardLen(), to[:])
+		}
+	}
 
-	return &OptimizeProofReply{newProof}, err
+	// Propagate the optimized proof to all nodes that were defined in any of
+	// the blocks.
+	roster := onet.NewRoster(nodes)
+	return s.startPropagation(s.propagateProof, roster,
+		&PropagateProof{newProof})
 }
 
 // GetUpdateChain returns a slice of SkipBlocks which describe the part of the

diff --git a/skipchain/skipchain_test.go b/skipchain/skipchain_test.go
@@ -764,7 +764,7 @@ func TestService_ForgedPropagationMessage(t *testing.T) {
 	// checks that the targets have to match
 	err = service.propagateProofHandler(&PropagateProof{append(p1[:2], p2[2], p1[3])})
 	require.NotNil(t, err)
-	require.Contains(t, err.Error(), "Wrong targets")
+	require.Contains(t, err.Error(), "invalid or missing forward-links")
 
 	// checks that the signature must match
 	forgedBlock := NewSkipBlock()
@@ -1627,7 +1627,8 @@ func testOptimizeProof(t *testing.T, numBlock, base, max, expected int) {
 	sk5 := srvs[4].Service(ServiceName).(*Service)
 	sk6 := srvs[5].Service(ServiceName).(*Service)
 
-	sbRoot, err := makeGenesisRosterArgs(sk1, roster, nil, VerificationStandard, base, max)
+	sbRoot, err := makeGenesisRosterArgs(sk1, ro, nil, VerificationStandard,
+		base, max)
 	require.NoError(t, err)
 
 	sb := NewSkipBlock()
@@ -1646,12 +1647,79 @@ func testOptimizeProof(t *testing.T, numBlock, base, max, expected int) {
 	log.Lvl1("Request to optimize the proof")
 
 	// Ask host 5 to optimize (not the leader)
-	opr, err := sk5.OptimizeProof(&OptimizeProofRequest{Roster: ro, ID: reply.Latest.Hash})
+	opr, err := sk5.OptimizeProof(&OptimizeProofRequest{ID: sbRoot.SkipChainID()})
 	require.NoError(t, err)
 	require.Equal(t, expected, len(opr.Proof))
 
-	// And verify the proof is propagated to the roster we asked for
-	sbs, err := sk6.db.GetProofForID(reply.Latest.Hash)
+	// If base == 1, there is nothing to optimize,
+	// so the blocks won't be sent to the other nodes.
+	if base > 1 {
+		// And verify the proof is propagated to the roster we asked for
+		sbs, err := sk6.db.GetProofForID(reply.Latest.Hash)
+		require.NoError(t, err)
+		require.Equal(t, expected, len(sbs))
+	}
+}
+
+// This tests what happens when a block in the middle of the chain is
+// optimized. One corner-case was when a block is optimized while a previous
+// block points further. Given a skipchain with base-height 2 and the
+// following blocks with [block-index, max-height, current/height]:
+//   0 4 4
+//   1 1 1
+//   2 2 1
+// Now if block #2 is optimized, the proof that will be sent includes block
+// 0 and 2, but block 0 points further than 2.
+// This needs to be handled correctly.
+func TestOptimizeMiddle(t *testing.T) {
+	numBlock := 9
+
+	local := onet.NewLocalTest(cothority.Suite)
+	defer local.CloseAll()
+	srvs, roster, _ := local.MakeSRS(cothority.Suite, 3, skipchainSID)
+
+	sks := make([]*Service, len(srvs))
+	for i, srv := range srvs {
+		sks[i] = srv.Service(ServiceName).(*Service)
+	}
+	leader := sks[0]
+
+	blocks := make([]*SkipBlock, numBlock)
+	var err error
+	blocks[0], err = makeGenesisRosterArgs(leader, roster, nil,
+		VerificationStandard, 2, 10)
+	require.NoError(t, err)
+
+	leader.disableForwardLink = true
+
+	for i := 1; i < numBlock; i++ {
+		sb := NewSkipBlock()
+		sb.Roster = roster
+		reply, err := leader.StoreSkipBlock(
+			&StoreSkipBlock{TargetSkipChainID: blocks[0].Hash, NewBlock: sb})
+		require.NoError(t, err)
+		blocks[i] = reply.Latest
+	}
+
+	leader.disableForwardLink = false
+
+	// Optimize the genesis-block,
+	// will create an empty forward-link at index 1,
+	// plus a valid forward-link at index 2.
+	log.Lvl1("Request to optimize the proof for the chain")
+	opr, err := sks[2].OptimizeProof(&OptimizeProofRequest{ID: blocks[0].Hash})
+	require.NoError(t, err)
+	require.Equal(t, 4, opr.Proof[0].GetForwardLen())
+
+	log.Lvl1("Request to optimize the proof for block index 2")
+	opr, err = sks[2].OptimizeProof(&OptimizeProofRequest{ID: blocks[2].Hash})
+	require.NoError(t, err)
+	require.Equal(t, 3, len(opr.Proof))
+	require.Equal(t, 2, opr.Proof[2].GetForwardLen())
+
+	log.Lvl1("Request to optimize the proof for block index 4")
+	opr, err = sks[2].OptimizeProof(&OptimizeProofRequest{ID: blocks[4].Hash})
 	require.NoError(t, err)
-	require.Equal(t, expected, len(sbs))
+	require.Equal(t, 4, len(opr.Proof))
+	require.Equal(t, 3, opr.Proof[3].GetForwardLen())
 }
diff --git a/skipchain/struct.go b/skipchain/struct.go
@@ -623,19 +623,36 @@ func (sbs Proof) verifyChain() error {
 				return errors.New("Missing backlink")
 			}
 		}
+
+		// Check if there is a forward link to the next block - this might
+		// not be the highest link,
+		// as the block to be optimized might be in nthe middle of the chain.
 		if i < len(sbs)-1 {
-			// Check if there is a forward link to the next block
 			if len(sb.ForwardLink) == 0 {
 				return errors.New("Missing forward links")
 			}
 
-			fl := sb.ForwardLink[len(sb.ForwardLink)-1]
-			if err := fl.VerifyWithScheme(suite, sb.Roster.ServicePublics(ServiceName), sb.SignatureScheme); err != nil {
-				return err
-			}
+			flOK := false
+			for flIndex := len(sb.ForwardLink) - 1; flIndex >= 0; flIndex-- {
+				fl := sb.ForwardLink[flIndex]
+				if fl.IsEmpty() {
+					continue
+				}
 
-			if !sbs[i+1].Hash.Equal(fl.To) || !fl.From.Equal(sb.Hash) {
-				return errors.New("Wrong targets for the forward link")
+				if err := fl.VerifyWithScheme(suite,
+					sb.Roster.ServicePublics(ServiceName), sb.SignatureScheme); err != nil {
+					return err
+				}
+
+				if !sbs[i+1].Hash.Equal(fl.To) || !fl.From.Equal(sb.Hash) {
+					continue
+				}
+				flOK = true
+				break
+			}
+			if !flOK {
+				return xerrors.New("encountered invalid or missing forward" +
+					"-links in proof")
 			}
 		}
 	}