Remove the code for signing log roots if the last one is too old. (#266)

* Remove the code for signing log roots if the last one is too old.

This is leftover from when we thought app layers wouldn't have their own
storage. I don't think it's being used and probably doesn't do what we
need anyway.

* Create a root in sequencing if none already exists.

Could be better to move this to log create API when we write it. Then we
can assume a root exists and raise an error if it doesn't. TODO added.

Author: Martin2112

log/sequencer.go

@@ -40,11 +40,6 @@ type Sequencer struct {
 //           the subtrees.
 const maxTreeDepth = 64
 
-// CurrentRootExpiredFunc examines a signed log root and decides if it has expired with respect
-// to a max age duration and a given time source
-// TODO(Martin2112): This is all likely to go away when we switch to application STHs
-type CurrentRootExpiredFunc func(trillian.SignedLogRoot) bool
-
 // NewSequencer creates a new Sequencer instance for the specified inputs.
 func NewSequencer(hasher merkle.TreeHasher, timeSource util.TimeSource, logStorage storage.LogStorage, km crypto.KeyManager) *Sequencer {
 	return &Sequencer{hasher: hasher, timeSource: timeSource, logStorage: logStorage, keyManager: km}
@@ -156,7 +151,7 @@ func (s Sequencer) createRootSignature(ctx context.Context, root trillian.Signed
 // TODO(Martin2112): Can possibly improve by deferring a function that attempts to rollback,
 // which will fail if the tx was committed. Should only do this if we can hide the details of
 // the underlying storage transactions and it doesn't create other problems.
-func (s Sequencer) SequenceBatch(ctx context.Context, limit int, expiryFunc CurrentRootExpiredFunc) (int, error) {
+func (s Sequencer) SequenceBatch(ctx context.Context, limit int) (int, error) {
 	tx, err := s.logStorage.Begin()
 
 	if err != nil {
@@ -184,22 +179,17 @@ func (s Sequencer) SequenceBatch(ctx context.Context, limit int, expiryFunc Curr
 	}
 
 	// TODO(al): Have a better detection mechanism for there being no stored root.
+	// TODO(mhs): Might be better to create empty root in provisioning API when it exists
 	if currentRoot.RootHash == nil {
 		glog.Warningf("%s: Fresh log - no previous TreeHeads exist.", util.LogIDPrefix(ctx))
+		return 0, s.SignRoot(ctx)
 	}
 
 	// There might be no work to be done. But we possibly still need to create an STH if the
 	// current one is too old. If there's work to be done then we'll be creating a root anyway.
 	if len(leaves) == 0 {
 		// We have nothing to integrate into the tree
-		tx.Commit()
-		if expiryFunc(currentRoot) {
-			// Current root is too old, sign one. Will use a new TX, safe as we have no writes
-			// pending in this one.
-			glog.V(2).Infof("%s: Current root is too old, create new STH", util.LogIDPrefix(ctx))
-			return 0, s.SignRoot(ctx)
-		}
-		return 0, nil
+		return 0, tx.Commit()
 	}
 
 	merkleTree, err := s.initMerkleTreeFromStorage(ctx, currentRoot, tx)

log/sequencer_test.go

@@ -17,12 +17,6 @@ import (
 	"github.com/google/trillian/util"
 )
 
-// Func that says root never expires, so we can be sure tests will only sign / sequence when we
-// expect them to
-func rootNeverExpiresFunc(trillian.SignedLogRoot) bool {
-	return false
-}
-
 var treeHasher = merkle.NewRFC6962TreeHasher(crypto.NewSHA256())
 
 // These can be shared between tests as they're never modified
@@ -230,7 +224,7 @@ func TestBeginTXFails(t *testing.T) {
 	params := testParameters{beginFails: true, skipDequeue: true, skipStoreSignedRoot: true}
 	c, ctx := createTestContext(ctrl, params)
 
-	leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leaves, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leaves != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leaves)
 	}
@@ -245,7 +239,7 @@ func TestSequenceWithNothingQueued(t *testing.T) {
 
 	c, ctx := createTestContext(ctrl, params)
 
-	leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leaves, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leaves != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leaves)
 	}
@@ -268,7 +262,7 @@ func TestGuardWindowPassthrough(t *testing.T) {
 	c, ctx := createTestContext(ctrl, params)
 	c.sequencer.SetGuardWindow(guardInterval)
 
-	leaves, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leaves, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leaves != 0 {
 		t.Fatalf("Expected no leaves sequenced when in guard interval but got: %d", leaves)
 	}
@@ -285,7 +279,7 @@ func TestDequeueError(t *testing.T) {
 	params := testParameters{dequeueLimit: 1, shouldRollback: true, dequeuedError: errors.New("dequeue")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	testonly.EnsureErrorContains(t, err, "dequeue")
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
@@ -301,7 +295,7 @@ func TestLatestRootError(t *testing.T) {
 		latestSignedRoot: &testRoot16, latestSignedRootError: errors.New("root")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -319,7 +313,7 @@ func TestUpdateSequencedLeavesError(t *testing.T) {
 		updatedLeavesError: errors.New("unsequenced")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -337,7 +331,7 @@ func TestSetMerkleNodesError(t *testing.T) {
 		merkleNodesSetError: errors.New("setmerklenodes")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -358,7 +352,7 @@ func TestStoreSignedRootError(t *testing.T) {
 		signingResult: []byte("signed")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -378,7 +372,7 @@ func TestStoreSignedRootKeyManagerFails(t *testing.T) {
 		keyManagerError: errors.New("keymanagerfailed")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -399,7 +393,7 @@ func TestStoreSignedRootSignerFails(t *testing.T) {
 		signingError:    errors.New("signerfailed")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -421,7 +415,7 @@ func TestCommitFails(t *testing.T) {
 		signingResult: []byte("signed")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if leafCount != 0 {
 		t.Fatalf("Unexpectedly sequenced %d leaves on error", leafCount)
 	}
@@ -444,7 +438,7 @@ func TestSequenceBatch(t *testing.T) {
 		signingResult: []byte("signed")}
 	c, ctx := createTestContext(ctrl, params)
 
-	leafCount, err := c.sequencer.SequenceBatch(ctx, 1, rootNeverExpiresFunc)
+	leafCount, err := c.sequencer.SequenceBatch(ctx, 1)
 	if err != nil {
 		t.Fatalf("Expected sequencing to succeed, but got err: %v", err)
 	}

server/sequencer_manager.go

@@ -4,7 +4,6 @@ import (
 	"time"
 
 	"github.com/golang/glog"
-	"github.com/google/trillian"
 	"github.com/google/trillian/crypto"
 	"github.com/google/trillian/log"
 	"github.com/google/trillian/merkle"
@@ -18,15 +17,6 @@ type SequencerManager struct {
 	cachedProvider *cachedLogStorageProvider
 }
 
-func isRootTooOld(ts util.TimeSource, maxAge time.Duration) log.CurrentRootExpiredFunc {
-	return func(root trillian.SignedLogRoot) bool {
-		rootTime := time.Unix(0, root.TimestampNanos)
-		rootAge := ts.Now().Sub(rootTime)
-
-		return rootAge > maxAge
-	}
-}
-
 // NewSequencerManager creates a new SequencerManager instance based on the provided KeyManager instance
 // and guard window.
 func NewSequencerManager(km crypto.KeyManager, p LogStorageProviderFunc, gw time.Duration) *SequencerManager {
@@ -71,7 +61,7 @@ func (s SequencerManager) ExecutePass(logIDs []int64, logctx LogOperationManager
 		sequencer := log.NewSequencer(merkle.NewRFC6962TreeHasher(crypto.NewSHA256()), logctx.timeSource, storage, s.keyManager)
 		sequencer.SetGuardWindow(s.guardWindow)
 
-		leaves, err := sequencer.SequenceBatch(ctx, logctx.batchSize, isRootTooOld(logctx.timeSource, logctx.signInterval))
+		leaves, err := sequencer.SequenceBatch(ctx, logctx.batchSize)
 
 		if err != nil {
 			glog.Warningf("%s: Error trying to sequence batch for: %v", util.LogIDPrefix(ctx), err)

server/sequencer_manager_test.go

@@ -137,40 +137,6 @@ func TestSequencerManagerSingleLogOneLeaf(t *testing.T) {
 	sm.ExecutePass([]int64{logID}, createTestContext(provider))
 }
 
-// Tests that a new root is signed if it's due even when there is no work to sequence.
-// The various failure cases of SignRoot() are tested in the sequencer tests. This is
-// an interaction test.
-func TestSignsIfNoWorkAndRootExpired(t *testing.T) {
-	mockCtrl := gomock.NewController(t)
-	defer mockCtrl.Finish()
-
-	mockStorage := storage.NewMockLogStorage(mockCtrl)
-	mockTx := storage.NewMockLogTX(mockCtrl)
-	mockKeyManager := crypto.NewMockKeyManager(mockCtrl)
-	mockKeyManager.EXPECT().SignatureAlgorithm().AnyTimes().Return(trillian.SignatureAlgorithm_ECDSA)
-	logID := int64(1)
-	hasher := crypto.NewSHA256()
-
-	mockStorage.EXPECT().Begin().AnyTimes().Return(mockTx, nil)
-	mockTx.EXPECT().WriteRevision().AnyTimes().Return(writeRev)
-	mockTx.EXPECT().Commit().AnyTimes().Return(nil)
-	mockTx.EXPECT().LatestSignedLogRoot().AnyTimes().Return(testRoot0, nil)
-	mockTx.EXPECT().DequeueLeaves(50, fakeTime).Return([]trillian.LogLeaf{}, nil)
-	mockTx.EXPECT().StoreSignedLogRoot(updatedRootSignOnly).AnyTimes().Return(nil)
-
-	mockSigner := crypto.NewMockSigner(mockCtrl)
-	mockSigner.EXPECT().Sign(gomock.Any(), []byte{0xeb, 0x7d, 0xa1, 0x4f, 0x1e, 0x60, 0x91, 0x24, 0xa, 0xf7, 0x1c, 0xcd, 0xdb, 0xd4, 0xca, 0x38, 0x4b, 0x12, 0xe4, 0xa3, 0xcf, 0x80, 0x5, 0x55, 0x17, 0x71, 0x35, 0xaf, 0x80, 0x11, 0xa, 0x87}, hasher).Return([]byte("signed"), nil)
-	mockKeyManager.EXPECT().Signer().Return(mockSigner, nil)
-
-	provider := mockStorageProviderForSequencer(mockStorage)
-	sm := NewSequencerManager(mockKeyManager, provider, zeroDuration)
-
-	tc := createTestContext(provider)
-	// Lower the expiry so we can trigger a signing for a root older than 5 seconds
-	tc.signInterval = time.Second * 5
-	sm.ExecutePass([]int64{logID}, tc)
-}
-
 func TestSequencerManagerGuardWindow(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	defer mockCtrl.Finish()