diff --git a/log/sequencer.go b/log/sequencer.go index 713cced894..d924412ba3 100644 --- a/log/sequencer.go +++ b/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) diff --git a/log/sequencer_test.go b/log/sequencer_test.go index a30a76988c..ad1525932f 100644 --- a/log/sequencer_test.go +++ b/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) } diff --git a/server/sequencer_manager.go b/server/sequencer_manager.go index a7fd7ad99d..17d317bade 100644 --- a/server/sequencer_manager.go +++ b/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) diff --git a/server/sequencer_manager_test.go b/server/sequencer_manager_test.go index f24da13d32..9b9ce08435 100644 --- a/server/sequencer_manager_test.go +++ b/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()