Use custom mocks instead of mocks.StorageAuthority (#7494)

Replace "mocks.StorageAuthority" with "sapb.StorageAuthorityClient" in
our test mocks. The improves them by removing implementations of the
methods the tests don't actually need, instead of inheriting lots of
extraneous methods from the huge and cumbersome mocks.StorageAuthority.

This reduces our usage of mocks.StorageAuthority to only the WFE tests
(which create one in the frequently-used setup() function), which will
make refactoring those mocks in the pursuit of
https://github.com/letsencrypt/boulder/issues/7476 much easier.

Part of https://github.com/letsencrypt/boulder/issues/7476

cmd/admin/key_test.go

@@ -86,6 +86,18 @@ func (msa *mockSARecordingBlocks) reset() {
 	msa.blockRequests = nil
 }
 
+type mockSARO struct {
+	sapb.StorageAuthorityReadOnlyClient
+}
+
+func (sa *mockSARO) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByKeyClient, error) {
+	return &mocks.ServerStreamClient[sapb.Serial]{}, nil
+}
+
+func (sa *mockSARO) KeyBlocked(ctx context.Context, req *sapb.SPKIHash, _ ...grpc.CallOption) (*sapb.Exists, error) {
+	return &sapb.Exists{Exists: false}, nil
+}
+
 func TestBlockSPKIHash(t *testing.T) {
 	fc := clock.NewFake()
 	fc.Set(time.Now())
@@ -97,7 +109,7 @@ func TestBlockSPKIHash(t *testing.T) {
 	keyHash, err := core.KeyDigest(privKey.Public())
 	test.AssertNotError(t, err, "computing test SPKI hash")
 
-	a := admin{saroc: &mocks.StorageAuthorityReadOnly{}, sac: &msa, clk: fc, log: log}
+	a := admin{saroc: &mockSARO{}, sac: &msa, clk: fc, log: log}
 	u := &user.User{}
 
 	// A full run should result in one request with the right fields.

crl/updater/updater_test.go

@@ -12,16 +12,16 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/jmhodges/clock"
+	"github.com/prometheus/client_golang/prometheus"
+
 	capb "github.com/letsencrypt/boulder/ca/proto"
 	corepb "github.com/letsencrypt/boulder/core/proto"
 	cspb "github.com/letsencrypt/boulder/crl/storer/proto"
 	"github.com/letsencrypt/boulder/issuance"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
-	"github.com/letsencrypt/boulder/mocks"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 	"github.com/letsencrypt/boulder/test"
-	"github.com/prometheus/client_golang/prometheus"
 )
 
 // fakeGRCC is a fake sapb.StorageAuthority_GetRevokedCertsClient which can be
@@ -50,7 +50,7 @@ func (f *fakeGRCC) Recv() (*corepb.CRLEntry, error) {
 // fakeGRCC to be used as the return value for calls to GetRevokedCerts, and a
 // fake timestamp to serve as the database's maximum notAfter value.
 type fakeSAC struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 	grcc        fakeGRCC
 	maxNotAfter time.Time
 	leaseError  error

mocks/grpc.go

@@ -0,0 +1,19 @@
+package mocks
+
+import (
+	"io"
+
+	"google.golang.org/grpc"
+)
+
+// ServerStreamClient is a mock which satisfies the grpc.ClientStream interface,
+// allowing it to be returned by methods where the server returns a stream of
+// results. This simple mock will always return zero results.
+type ServerStreamClient[T any] struct {
+	grpc.ClientStream
+}
+
+// Recv immediately returns the EOF error, indicating that the stream is done.
+func (c *ServerStreamClient[T]) Recv() (*T, error) {
+	return nil, io.EOF
+}

mocks/mailer.go

@@ -0,0 +1,60 @@
+package mocks
+
+import (
+	"sync"
+
+	"github.com/letsencrypt/boulder/mail"
+)
+
+// Mailer is a mock
+type Mailer struct {
+	sync.Mutex
+	Messages []MailerMessage
+}
+
+var _ mail.Mailer = &Mailer{}
+
+// mockMailerConn is a mock that satisfies the mail.Conn interface
+type mockMailerConn struct {
+	parent *Mailer
+}
+
+var _ mail.Conn = &mockMailerConn{}
+
+// MailerMessage holds the captured emails from SendMail()
+type MailerMessage struct {
+	To      string
+	Subject string
+	Body    string
+}
+
+// Clear removes any previously recorded messages
+func (m *Mailer) Clear() {
+	m.Lock()
+	defer m.Unlock()
+	m.Messages = nil
+}
+
+// SendMail is a mock
+func (m *mockMailerConn) SendMail(to []string, subject, msg string) error {
+	m.parent.Lock()
+	defer m.parent.Unlock()
+	for _, rcpt := range to {
+		m.parent.Messages = append(m.parent.Messages, MailerMessage{
+			To:      rcpt,
+			Subject: subject,
+			Body:    msg,
+		})
+	}
+	return nil
+}
+
+// Close is a mock
+func (m *mockMailerConn) Close() error {
+	return nil
+}
+
+// Connect is a mock
+func (m *Mailer) Connect() (mail.Conn, error) {
+	return &mockMailerConn{parent: m}, nil
+}

mocks/publisher.go

@@ -0,0 +1,19 @@
+package mocks
+
+import (
+	"context"
+
+	"google.golang.org/grpc"
+
+	pubpb "github.com/letsencrypt/boulder/publisher/proto"
+)
+
+// PublisherClient is a mock
+type PublisherClient struct {
+	// empty
+}
+
+// SubmitToSingleCTWithResult is a mock
+func (*PublisherClient) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request, _ ...grpc.CallOption) (*pubpb.Result, error) {
+	return &pubpb.Result{}, nil
+}

mocks/sa.go

@@ -6,11 +6,9 @@ import (
 	"crypto/x509"
 	"errors"
 	"fmt"
-	"io"
 	"math/rand"
 	"net"
 	"os"
-	"sync"
 	"time"
 
 	"github.com/go-jose/go-jose/v4"
@@ -26,8 +24,6 @@ import (
 	berrors "github.com/letsencrypt/boulder/errors"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	"github.com/letsencrypt/boulder/identifier"
-	"github.com/letsencrypt/boulder/mail"
-	pubpb "github.com/letsencrypt/boulder/publisher/proto"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
 )
 
@@ -53,18 +49,6 @@ func NewStorageAuthority(clk clock.Clock) *StorageAuthority {
 	return &StorageAuthority{StorageAuthorityReadOnly{clk}}
 }
 
-// serverStreamClient is a mock which satisfies the grpc.ClientStream interface,
-// allowing it to be returned by methods where the server returns a stream of
-// results. This simple mock will always return zero results.
-type serverStreamClient[T any] struct {
-	grpc.ClientStream
-}
-
-// Recv immediately returns the EOF error, indicating that the stream is done.
-func (c *serverStreamClient[T]) Recv() (*T, error) {
-	return nil, io.EOF
-}
-
 const (
 	test1KeyPublicJSON  = `{"kty":"RSA","n":"yNWVhtYEKJR21y9xsHV-PD_bYwbXSeNuFal46xYxVfRL5mqha7vttvjB_vc7Xg2RvgCxHPCqoxgMPTzHrZT75LjCwIW2K_klBYN8oYvTwwmeSkAz6ut7ZxPv-nZaT5TJhGk0NT2kh_zSpdriEJ_3vW-mqxYbbBmpvHqsa1_zx9fSuHYctAZJWzxzUZXykbWMWQZpEiE0J4ajj51fInEzVn7VxV-mzfMyboQjujPh7aNJxAWSq4oQEJJDgWwSh9leyoJoPpONHxh5nEE5AjE01FkGICSxjpZsF-w8hOTI3XXohUdu29Se26k2B0PolDSuj0GIQU6-W9TdLXSjBb2SpQ","e":"AQAB"}`
 	test2KeyPublicJSON  = `{"kty":"RSA","n":"qnARLrT7Xz4gRcKyLdydmCr-ey9OuPImX4X40thk3on26FkMznR3fRjs66eLK7mmPcBZ6uOJseURU6wAaZNmemoYx1dMvqvWWIyiQleHSD7Q8vBrhR6uIoO4jAzJZR-ChzZuSDt7iHN-3xUVspu5XGwXU_MVJZshTwp4TaFx5elHIT_ObnTvTOU3Xhish07AbgZKmWsVbXh5s-CrIicU4OexJPgunWZ_YJJueOKmTvnLlTV4MzKR2oZlBKZ27S0-SfdV_QDx_ydle5oMAyKVtlAV35cyPMIsYNwgUGBCdY_2Uzi5eX0lTc7MPRwz6qR1kip-i59VcGcUQgqHV6Fyqw","e":"AQAB"}`
@@ -252,22 +236,22 @@ func (sa *StorageAuthorityReadOnly) GetRevocationStatus(_ context.Context, req *
 
 // SerialsForIncident is a mock
 func (sa *StorageAuthorityReadOnly) SerialsForIncident(ctx context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_SerialsForIncidentClient, error) {
-	return &serverStreamClient[sapb.IncidentSerial]{}, nil
+	return &ServerStreamClient[sapb.IncidentSerial]{}, nil
 }
 
 // SerialsForIncident is a mock
 func (sa *StorageAuthority) SerialsForIncident(ctx context.Context, _ *sapb.SerialsForIncidentRequest, _ ...grpc.CallOption) (sapb.StorageAuthority_SerialsForIncidentClient, error) {
-	return &serverStreamClient[sapb.IncidentSerial]{}, nil
+	return &ServerStreamClient[sapb.IncidentSerial]{}, nil
 }
 
 // GetRevokedCerts is a mock
 func (sa *StorageAuthorityReadOnly) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetRevokedCertsClient, error) {
-	return &serverStreamClient[corepb.CRLEntry]{}, nil
+	return &ServerStreamClient[corepb.CRLEntry]{}, nil
 }
 
 // GetRevokedCerts is a mock
 func (sa *StorageAuthority) GetRevokedCerts(ctx context.Context, _ *sapb.GetRevokedCertsRequest, _ ...grpc.CallOption) (sapb.StorageAuthority_GetRevokedCertsClient, error) {
-	return &serverStreamClient[corepb.CRLEntry]{}, nil
+	return &ServerStreamClient[corepb.CRLEntry]{}, nil
 }
 
 // GetMaxExpiration is a mock
@@ -559,22 +543,22 @@ func (sa *StorageAuthorityReadOnly) GetAuthorization2(ctx context.Context, id *s
 
 // GetSerialsByKey is a mock
 func (sa *StorageAuthorityReadOnly) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByKeyClient, error) {
-	return &serverStreamClient[sapb.Serial]{}, nil
+	return &ServerStreamClient[sapb.Serial]{}, nil
 }
 
 // GetSerialsByKey is a mock
 func (sa *StorageAuthority) GetSerialsByKey(ctx context.Context, _ *sapb.SPKIHash, _ ...grpc.CallOption) (sapb.StorageAuthority_GetSerialsByKeyClient, error) {
-	return &serverStreamClient[sapb.Serial]{}, nil
+	return &ServerStreamClient[sapb.Serial]{}, nil
 }
 
 // GetSerialsByAccount is a mock
 func (sa *StorageAuthorityReadOnly) GetSerialsByAccount(ctx context.Context, _ *sapb.RegistrationID, _ ...grpc.CallOption) (sapb.StorageAuthorityReadOnly_GetSerialsByAccountClient, error) {
-	return &serverStreamClient[sapb.Serial]{}, nil
+	return &ServerStreamClient[sapb.Serial]{}, nil
 }
 
 // GetSerialsByAccount is a mock
 func (sa *StorageAuthority) GetSerialsByAccount(ctx context.Context, _ *sapb.RegistrationID, _ ...grpc.CallOption) (sapb.StorageAuthority_GetSerialsByAccountClient, error) {
-	return &serverStreamClient[sapb.Serial]{}, nil
+	return &ServerStreamClient[sapb.Serial]{}, nil
 }
 
 // RevokeCertificate is a mock
@@ -616,66 +600,3 @@ func (sa *StorageAuthority) UpdateCRLShard(ctx context.Context, req *sapb.Update
 func (sa *StorageAuthorityReadOnly) ReplacementOrderExists(ctx context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.Exists, error) {
 	return nil, nil
 }
-
-// PublisherClient is a mock
-type PublisherClient struct {
-	// empty
-}
-
-// SubmitToSingleCTWithResult is a mock
-func (*PublisherClient) SubmitToSingleCTWithResult(_ context.Context, _ *pubpb.Request, _ ...grpc.CallOption) (*pubpb.Result, error) {
-	return &pubpb.Result{}, nil
-}
-
-// Mailer is a mock
-type Mailer struct {
-	sync.Mutex
-	Messages []MailerMessage
-}
-
-var _ mail.Mailer = &Mailer{}
-
-// mockMailerConn is a mock that satisfies the mail.Conn interface
-type mockMailerConn struct {
-	parent *Mailer
-}
-
-var _ mail.Conn = &mockMailerConn{}
-
-// MailerMessage holds the captured emails from SendMail()
-type MailerMessage struct {
-	To      string
-	Subject string
-	Body    string
-}
-
-// Clear removes any previously recorded messages
-func (m *Mailer) Clear() {
-	m.Lock()
-	defer m.Unlock()
-	m.Messages = nil
-}
-
-// SendMail is a mock
-func (m *mockMailerConn) SendMail(to []string, subject, msg string) error {
-	m.parent.Lock()
-	defer m.parent.Unlock()
-	for _, rcpt := range to {
-		m.parent.Messages = append(m.parent.Messages, MailerMessage{
-			To:      rcpt,
-			Subject: subject,
-			Body:    msg,
-		})
-	}
-	return nil
-}
-
-// Close is a mock
-func (m *mockMailerConn) Close() error {
-	return nil
-}
-
-// Connect is a mock
-func (m *Mailer) Connect() (mail.Conn, error) {
-	return &mockMailerConn{parent: m}, nil
-}

ocsp/responder/redis/checked_redis_source_test.go

@@ -18,7 +18,6 @@ import (
 	berrors "github.com/letsencrypt/boulder/errors"
 	blog "github.com/letsencrypt/boulder/log"
 	"github.com/letsencrypt/boulder/metrics"
-	"github.com/letsencrypt/boulder/mocks"
 	"github.com/letsencrypt/boulder/ocsp/responder"
 	ocsp_test "github.com/letsencrypt/boulder/ocsp/test"
 	"github.com/letsencrypt/boulder/sa"
@@ -99,7 +98,7 @@ func (s notFoundSelector) SelectOne(_ context.Context, _ interface{}, _ string,
 
 // echoSA always returns the given revocation status.
 type echoSA struct {
-	mocks.StorageAuthorityReadOnly
+	sapb.StorageAuthorityReadOnlyClient
 	status *sapb.RevocationStatus
 }
 
@@ -109,7 +108,7 @@ func (s *echoSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...g
 
 // errorSA always returns an error.
 type errorSA struct {
-	mocks.StorageAuthorityReadOnly
+	sapb.StorageAuthorityReadOnlyClient
 }
 
 func (s *errorSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {
@@ -118,7 +117,7 @@ func (s *errorSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...
 
 // notFoundSA always returns a NotFound error.
 type notFoundSA struct {
-	mocks.StorageAuthorityReadOnly
+	sapb.StorageAuthorityReadOnlyClient
 }
 
 func (s *notFoundSA) GetRevocationStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.RevocationStatus, error) {

ra/mock_test.go

@@ -1,81 +0,0 @@
-package ra
-
-import (
-	"context"
-	"time"
-
-	grpc "google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-	emptypb "google.golang.org/protobuf/types/known/emptypb"
-	"google.golang.org/protobuf/types/known/timestamppb"
-
-	"github.com/letsencrypt/boulder/core"
-	corepb "github.com/letsencrypt/boulder/core/proto"
-	"github.com/letsencrypt/boulder/mocks"
-	sapb "github.com/letsencrypt/boulder/sa/proto"
-)
-
-type mockInvalidAuthorizationsAuthority struct {
-	mocks.StorageAuthority
-	domainWithFailures string
-}
-
-// SetCertificateStatusReady implements proto.StorageAuthorityClient
-func (*mockInvalidAuthorizationsAuthority) SetCertificateStatusReady(ctx context.Context, in *sapb.Serial, opts ...grpc.CallOption) (*emptypb.Empty, error) {
-	return nil, status.Error(codes.Unimplemented, "unimplemented mock")
-}
-
-func (sa *mockInvalidAuthorizationsAuthority) CountOrders(_ context.Context, _ *sapb.CountOrdersRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
-	return &sapb.Count{}, nil
-}
-
-func (sa *mockInvalidAuthorizationsAuthority) CountInvalidAuthorizations2(ctx context.Context, req *sapb.CountInvalidAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
-	if req.Hostname == sa.domainWithFailures {
-		return &sapb.Count{Count: 1}, nil
-	} else {
-		return &sapb.Count{}, nil
-	}
-}
-
-// An authority that returns nonzero failures for CountInvalidAuthorizations2,
-// and also returns existing authzs for the same domain from GetAuthorizations2
-type mockInvalidPlusValidAuthzAuthority struct {
-	mockInvalidAuthorizationsAuthority
-}
-
-func (sa *mockInvalidPlusValidAuthzAuthority) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) {
-	return &sapb.Authorizations{
-		Authz: []*sapb.Authorizations_MapElement{
-			{
-				Domain: sa.domainWithFailures, Authz: &corepb.Authorization{
-					Id:             "1234",
-					Status:         "valid",
-					Identifier:     sa.domainWithFailures,
-					RegistrationID: 1234,
-					Expires:        timestamppb.New(time.Date(2101, 12, 3, 0, 0, 0, 0, time.UTC)),
-				},
-			},
-		},
-	}, nil
-}
-
-// An authority that returns an error from NewOrderAndAuthzs if the
-// "ReplacesSerial" field of the request is empty.
-type mockNewOrderMustBeReplacementAuthority struct {
-	mocks.StorageAuthority
-}
-
-func (sa *mockNewOrderMustBeReplacementAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb.NewOrderAndAuthzsRequest, _ ...grpc.CallOption) (*corepb.Order, error) {
-	if req.NewOrder.ReplacesSerial == "" {
-		return nil, status.Error(codes.InvalidArgument, "NewOrder is not a replacement")
-	}
-	return &corepb.Order{
-		Id:             1,
-		RegistrationID: req.NewOrder.RegistrationID,
-		Expires:        req.NewOrder.Expires,
-		Status:         string(core.StatusPending),
-		Created:        timestamppb.New(time.Now()),
-		Names:          req.NewOrder.Names,
-	}, nil
-}

ra/ra.go

@@ -2670,8 +2670,8 @@ func (ra *RegistrationAuthorityImpl) NewOrder(ctx context.Context, req *rapb.New
 	if err != nil {
 		return nil, err
 	}
-	// TODO(#7153): Check each value via core.IsAnyNilOrZero
+
-	if storedOrder.Id == 0 || storedOrder.Status == "" || storedOrder.RegistrationID == 0 || len(storedOrder.Names) == 0 || core.IsAnyNilOrZero(storedOrder.Created, storedOrder.Expires) {
+	if core.IsAnyNilOrZero(storedOrder.Id, storedOrder.Status, storedOrder.RegistrationID, storedOrder.Names, storedOrder.Created, storedOrder.Expires) {
 		return nil, errIncompleteGRPCResponse
 	}
 	ra.orderAges.WithLabelValues("NewOrder").Observe(0)

ra/ra_test.go

@@ -34,6 +34,8 @@ import (
 	"github.com/weppos/publicsuffix-go/publicsuffix"
 	"golang.org/x/crypto/ocsp"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/emptypb"
 	"google.golang.org/protobuf/types/known/timestamppb"
 
@@ -583,7 +585,7 @@ func TestNewRegistrationContactsPresent(t *testing.T) {
 }
 
 type mockSAFailsNewRegistration struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 }
 
 func (sa *mockSAFailsNewRegistration) NewRegistration(_ context.Context, _ *corepb.Registration, _ ...grpc.CallOption) (*corepb.Registration, error) {
@@ -769,7 +771,7 @@ func TestRegistrationsPerIPOverrideUsage(t *testing.T) {
 }
 
 type NoUpdateSA struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 }
 
 func (sa NoUpdateSA) UpdateRegistration(_ context.Context, _ *corepb.Registration, _ ...grpc.CallOption) (*emptypb.Empty, error) {
@@ -1185,6 +1187,21 @@ func TestEarlyOrderRateLimiting(t *testing.T) {
 	test.AssertEquals(t, bErr.Error(), expected)
 }
 
+// mockInvalidAuthorizationsAuthority is a mock which claims that the given
+// domain has one invalid authorization.
+type mockInvalidAuthorizationsAuthority struct {
+	sapb.StorageAuthorityClient
+	domainWithFailures string
+}
+
+func (sa *mockInvalidAuthorizationsAuthority) CountInvalidAuthorizations2(ctx context.Context, req *sapb.CountInvalidAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
+	if req.Hostname == sa.domainWithFailures {
+		return &sapb.Count{Count: 1}, nil
+	} else {
+		return &sapb.Count{}, nil
+	}
+}
+
 func TestAuthzFailedRateLimitingNewOrder(t *testing.T) {
 	_, _, ra, _, cleanUp := initAuthorities(t)
 	defer cleanUp()
@@ -1196,26 +1213,22 @@ func TestAuthzFailedRateLimitingNewOrder(t *testing.T) {
 		},
 	}
 
-	testcase := func() {
+	limit := ra.rlPolicies.InvalidAuthorizationsPerAccount()
-		limit := ra.rlPolicies.InvalidAuthorizationsPerAccount()
+	ra.SA = &mockInvalidAuthorizationsAuthority{domainWithFailures: "all.i.do.is.lose.com"}
-		ra.SA = &mockInvalidAuthorizationsAuthority{domainWithFailures: "all.i.do.is.lose.com"}
+	err := ra.checkInvalidAuthorizationLimits(ctx, Registration.Id,
-		err := ra.checkInvalidAuthorizationLimits(ctx, Registration.Id,
+		[]string{"charlie.brown.com", "all.i.do.is.lose.com"}, limit)
-			[]string{"charlie.brown.com", "all.i.do.is.lose.com"}, limit)
+	test.AssertError(t, err, "checkInvalidAuthorizationLimits did not encounter expected rate limit error")
-		test.AssertError(t, err, "checkInvalidAuthorizationLimits did not encounter expected rate limit error")
+	test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/failed-validation-limit/")
-		test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/failed-validation-limit/")
-	}
-
-	testcase()
 }
 
 type mockSAWithNameCounts struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 	nameCounts *sapb.CountByNames
 	t          *testing.T
 	clk        clock.FakeClock
 }
 
-func (m mockSAWithNameCounts) CountCertificatesByNames(ctx context.Context, req *sapb.CountCertificatesByNamesRequest, _ ...grpc.CallOption) (*sapb.CountByNames, error) {
+func (m *mockSAWithNameCounts) CountCertificatesByNames(ctx context.Context, req *sapb.CountCertificatesByNamesRequest, _ ...grpc.CallOption) (*sapb.CountByNames, error) {
 	expectedLatest := m.clk.Now()
 	if req.Range.Latest.AsTime() != expectedLatest {
 		m.t.Errorf("incorrect latest: got '%v', expected '%v'", req.Range.Latest.AsTime(), expectedLatest)
@@ -1233,6 +1246,12 @@ func (m mockSAWithNameCounts) CountCertificatesByNames(ctx context.Context, req
 	return &sapb.CountByNames{Counts: counts}, nil
 }
 
+// FQDNSetExists is a mock which always returns false, so the test requests
+// aren't considered to be renewals.
+func (m *mockSAWithNameCounts) FQDNSetExists(ctx context.Context, req *sapb.FQDNSetExistsRequest, _ ...grpc.CallOption) (*sapb.Exists, error) {
+	return &sapb.Exists{Exists: false}, nil
+}
+
 func TestCheckCertificatesPerNameLimit(t *testing.T) {
 	_, _, ra, fc, cleanUp := initAuthorities(t)
 	defer cleanUp()
@@ -1514,7 +1533,7 @@ func TestRegistrationKeyUpdate(t *testing.T) {
 // CountCertificatesByName as well as FQDNSetExists. This allows testing
 // checkCertificatesPerNameRateLimit's FQDN exemption logic.
 type mockSAWithFQDNSet struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 	fqdnSet            map[string]bool
 	issuanceTimestamps map[string]*sapb.Timestamps
 
@@ -2248,7 +2267,7 @@ func TestNewOrderReuseInvalidAuthz(t *testing.T) {
 // mockSACountPendingFails has a CountPendingAuthorizations2 implementation
 // that always returns error
 type mockSACountPendingFails struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 }
 
 func (mock *mockSACountPendingFails) CountPendingAuthorizations2(ctx context.Context, req *sapb.RegistrationID, _ ...grpc.CallOption) (*sapb.Count, error) {
@@ -2278,9 +2297,24 @@ func TestPendingAuthorizationsUnlimited(t *testing.T) {
 	test.AssertNotError(t, err, "checking pending authorization limit")
 }
 
+// An authority that returns nonzero failures for CountInvalidAuthorizations2,
+// and also returns existing authzs for the same domain from GetAuthorizations2
+type mockInvalidPlusValidAuthzAuthority struct {
+	mockSAWithAuthzs
+	domainWithFailures string
+}
+
+func (sa *mockInvalidPlusValidAuthzAuthority) CountInvalidAuthorizations2(ctx context.Context, req *sapb.CountInvalidAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Count, error) {
+	if req.Hostname == sa.domainWithFailures {
+		return &sapb.Count{Count: 1}, nil
+	} else {
+		return &sapb.Count{}, nil
+	}
+}
+
 // Test that the failed authorizations limit is checked before authz reuse.
 func TestNewOrderCheckFailedAuthorizationsFirst(t *testing.T) {
-	_, _, ra, _, cleanUp := initAuthorities(t)
+	_, _, ra, clk, cleanUp := initAuthorities(t)
 	defer cleanUp()
 
 	// Create an order (and thus a pending authz) for example.com
@@ -2293,8 +2327,29 @@ func TestNewOrderCheckFailedAuthorizationsFirst(t *testing.T) {
 	test.AssertNotNil(t, order.Id, "initial order had a nil ID")
 	test.AssertEquals(t, numAuthorizations(order), 1)
 
-	// Now treat example.com as if it had a recent failure.
+	// Now treat example.com as if it had a recent failure, but also a valid authz.
-	ra.SA = &mockInvalidPlusValidAuthzAuthority{mockInvalidAuthorizationsAuthority{domainWithFailures: "example.com"}}
+	expires := clk.Now().Add(24 * time.Hour)
+	ra.SA = &mockInvalidPlusValidAuthzAuthority{
+		mockSAWithAuthzs: mockSAWithAuthzs{
+			authzs: map[string]*core.Authorization{
+				"example.com": {
+					ID:             "1",
+					Identifier:     identifier.DNSIdentifier("example.com"),
+					RegistrationID: Registration.Id,
+					Expires:        &expires,
+					Status:         "valid",
+					Challenges: []core.Challenge{
+						{
+							Type:   core.ChallengeTypeHTTP01,
+							Status: core.StatusValid,
+						},
+					},
+				},
+			},
+		},
+		domainWithFailures: "example.com",
+	}
+
 	// Set a very restrictive police for invalid authorizations - one failure
 	// and you're done for a day.
 	ra.rlPolicies = &dummyRateLimitConfig{
@@ -2315,15 +2370,28 @@ func TestNewOrderCheckFailedAuthorizationsFirst(t *testing.T) {
 	test.AssertEquals(t, err.Error(), "too many failed authorizations recently: see https://letsencrypt.org/docs/failed-validation-limit/")
 }
 
-// mockSAUnsafeAuthzReuse has a GetAuthorizations implementation that returns
+// mockSAWithAuthzs has a GetAuthorizations2 method that returns the protobuf
-// an HTTP-01 validated wildcard authz.
+// version of its authzs struct member. It also has a fake GetOrderForNames
-type mockSAUnsafeAuthzReuse struct {
+// which always fails, and a fake NewOrderAndAuthzs which always succeeds, to
-	mocks.StorageAuthority
+// facilitate the full execution of RA.NewOrder.
+type mockSAWithAuthzs struct {
+	sapb.StorageAuthorityClient
+	authzs map[string]*core.Authorization
 }
 
-func authzMapToPB(m map[string]*core.Authorization) (*sapb.Authorizations, error) {
+// GetOrderForNames is a mock which always returns NotFound so that NewOrder
+// proceeds to attempt authz reuse instead of wholesale order reuse.
+func (msa *mockSAWithAuthzs) GetOrderForNames(ctx context.Context, req *sapb.GetOrderForNamesRequest, _ ...grpc.CallOption) (*corepb.Order, error) {
+	return nil, berrors.NotFoundError("no such order")
+}
+
+// GetAuthorizations2 returns a _bizarre_ authorization for "*.zombo.com" that
+// was validated by HTTP-01. This should never happen in real life since the
+// name is a wildcard. We use this mock to test that we reject this bizarre
+// situation correctly.
+func (msa *mockSAWithAuthzs) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) {
 	resp := &sapb.Authorizations{}
-	for k, v := range m {
+	for k, v := range msa.authzs {
 		authzPB, err := bgrpc.AuthzToPB(*v)
 		if err != nil {
 			return nil, err
@@ -2333,65 +2401,27 @@ func authzMapToPB(m map[string]*core.Authorization) (*sapb.Authorizations, error
 	return resp, nil
 }
 
-// GetAuthorizations2 returns a _bizarre_ authorization for "*.zombo.com" that
+// NewOrderAndAuthzs is a mock which just reflects the incoming request back,
-// was validated by HTTP-01. This should never happen in real life since the
+// pretending to have created new db rows for the requested newAuthzs.
-// name is a wildcard. We use this mock to test that we reject this bizarre
+func (msa *mockSAWithAuthzs) NewOrderAndAuthzs(ctx context.Context, req *sapb.NewOrderAndAuthzsRequest, _ ...grpc.CallOption) (*corepb.Order, error) {
-// situation correctly.
+	authzIDs := req.NewOrder.V2Authorizations
-func (msa *mockSAUnsafeAuthzReuse) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) {
-	expires := time.Now()
-	authzs := map[string]*core.Authorization{
-		"*.zombo.com": {
-			// A static fake ID we can check for in a unit test
-			ID:             "1",
-			Identifier:     identifier.DNSIdentifier("*.zombo.com"),
-			RegistrationID: req.RegistrationID,
-			// Authz is valid
-			Status:  "valid",
-			Expires: &expires,
-			Challenges: []core.Challenge{
-				// HTTP-01 challenge is valid
-				{
-					Type:   core.ChallengeTypeHTTP01, // The dreaded HTTP-01! X__X
-					Status: core.StatusValid,
-				},
-				// DNS-01 challenge is pending
-				{
-					Type:   core.ChallengeTypeDNS01,
-					Status: core.StatusPending,
-				},
-			},
-		},
-		"zombo.com": {
-			// A static fake ID we can check for in a unit test
-			ID:             "2",
-			Identifier:     identifier.DNSIdentifier("zombo.com"),
-			RegistrationID: req.RegistrationID,
-			// Authz is valid
-			Status:  "valid",
-			Expires: &expires,
-			Challenges: []core.Challenge{
-				// HTTP-01 challenge is valid
-				{
-					Type:   core.ChallengeTypeHTTP01,
-					Status: core.StatusValid,
-				},
-				// DNS-01 challenge is pending
-				{
-					Type:   core.ChallengeTypeDNS01,
-					Status: core.StatusPending,
-				},
-			},
-		},
-	}
-	return authzMapToPB(authzs)
-
-}
-
-func (msa *mockSAUnsafeAuthzReuse) NewOrderAndAuthzs(ctx context.Context, req *sapb.NewOrderAndAuthzsRequest, _ ...grpc.CallOption) (*corepb.Order, error) {
 	for range req.NewAuthzs {
-		req.NewOrder.V2Authorizations = append(req.NewOrder.V2Authorizations, mrand.Int63())
+		authzIDs = append(authzIDs, mrand.Int63())
 	}
-	return msa.StorageAuthority.NewOrderAndAuthzs(ctx, req)
+	return &corepb.Order{
+		// Fields from the input new order request.
+		RegistrationID:         req.NewOrder.RegistrationID,
+		Expires:                req.NewOrder.Expires,
+		Names:                  req.NewOrder.Names,
+		V2Authorizations:       authzIDs,
+		CertificateProfileName: req.NewOrder.CertificateProfileName,
+		// Mock new fields generated by the database transaction.
+		Id:      mrand.Int63(),
+		Created: timestamppb.Now(),
+		// A new order is never processing because it can't have been finalized yet.
+		BeganProcessing: false,
+		Status:          string(core.StatusPending),
+	}, nil
 }
 
 // TestNewOrderAuthzReuseSafety checks that the RA's safety check for reusing an
@@ -2409,7 +2439,53 @@ func TestNewOrderAuthzReuseSafety(t *testing.T) {
 
 	// Use a mock SA that always returns a valid HTTP-01 authz for the name
 	// "zombo.com"
-	ra.SA = &mockSAUnsafeAuthzReuse{}
+	expires := time.Now()
+	ra.SA = &mockSAWithAuthzs{
+		authzs: map[string]*core.Authorization{
+			"*.zombo.com": {
+				// A static fake ID we can check for in a unit test
+				ID:             "1",
+				Identifier:     identifier.DNSIdentifier("*.zombo.com"),
+				RegistrationID: Registration.Id,
+				// Authz is valid
+				Status:  "valid",
+				Expires: &expires,
+				Challenges: []core.Challenge{
+					// HTTP-01 challenge is valid
+					{
+						Type:   core.ChallengeTypeHTTP01, // The dreaded HTTP-01! X__X
+						Status: core.StatusValid,
+					},
+					// DNS-01 challenge is pending
+					{
+						Type:   core.ChallengeTypeDNS01,
+						Status: core.StatusPending,
+					},
+				},
+			},
+			"zombo.com": {
+				// A static fake ID we can check for in a unit test
+				ID:             "2",
+				Identifier:     identifier.DNSIdentifier("zombo.com"),
+				RegistrationID: Registration.Id,
+				// Authz is valid
+				Status:  "valid",
+				Expires: &expires,
+				Challenges: []core.Challenge{
+					// HTTP-01 challenge is valid
+					{
+						Type:   core.ChallengeTypeHTTP01,
+						Status: core.StatusValid,
+					},
+					// DNS-01 challenge is pending
+					{
+						Type:   core.ChallengeTypeDNS01,
+						Status: core.StatusPending,
+					},
+				},
+			},
+		},
+	}
 
 	// Create an initial request with regA and names
 	orderReq := &rapb.NewOrderRequest{
@@ -2591,35 +2667,6 @@ func TestNewOrderWildcard(t *testing.T) {
 	test.AssertEquals(t, dupeOrder.V2Authorizations[0], order.V2Authorizations[0])
 }
 
-// mockSANearExpiredAuthz is a mock SA that always returns an authz near expiry
-// to test orders expiry calculations
-type mockSANearExpiredAuthz struct {
-	mocks.StorageAuthority
-	expiry time.Time
-}
-
-// GetAuthorizations2 is a mock that always returns a valid authorization for
-// "zombo.com" very near to expiry
-func (msa *mockSANearExpiredAuthz) GetAuthorizations2(ctx context.Context, req *sapb.GetAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) {
-	authzs := map[string]*core.Authorization{
-		"zombo.com": {
-			// A static fake ID we can check for in a unit test
-			ID:             "1",
-			Identifier:     identifier.DNSIdentifier("zombo.com"),
-			RegistrationID: req.RegistrationID,
-			Expires:        &msa.expiry,
-			Status:         "valid",
-			Challenges: []core.Challenge{
-				{
-					Type:   core.ChallengeTypeHTTP01,
-					Status: core.StatusValid,
-				},
-			},
-		},
-	}
-	return authzMapToPB(authzs)
-}
-
 func TestNewOrderExpiry(t *testing.T) {
 	_, _, ra, clk, cleanUp := initAuthorities(t)
 	defer cleanUp()
@@ -2636,7 +2683,24 @@ func TestNewOrderExpiry(t *testing.T) {
 
 	// Use a mock SA that always returns a soon-to-be-expired valid authz for
 	// "zombo.com".
-	ra.SA = &mockSANearExpiredAuthz{expiry: fakeAuthzExpires}
+	ra.SA = &mockSAWithAuthzs{
+		authzs: map[string]*core.Authorization{
+			"zombo.com": {
+				// A static fake ID we can check for in a unit test
+				ID:             "1",
+				Identifier:     identifier.DNSIdentifier("zombo.com"),
+				RegistrationID: Registration.Id,
+				Expires:        &fakeAuthzExpires,
+				Status:         "valid",
+				Challenges: []core.Challenge{
+					{
+						Type:   core.ChallengeTypeHTTP01,
+						Status: core.StatusValid,
+					},
+				},
+			},
+		},
+	}
 
 	// Create an initial request with regA and names
 	orderReq := &rapb.NewOrderRequest{
@@ -3668,6 +3732,14 @@ func (ca *MockCARecordingProfile) IssueCertificateForPrecertificate(ctx context.
 	return ca.inner.IssueCertificateForPrecertificate(ctx, req)
 }
 
+type mockSAWithFinalize struct {
+	sapb.StorageAuthorityClient
+}
+
+func (sa *mockSAWithFinalize) FinalizeOrder(ctx context.Context, req *sapb.FinalizeOrderRequest, _ ...grpc.CallOption) (*emptypb.Empty, error) {
+	return &emptypb.Empty{}, nil
+}
+
 func TestIssueCertificateInnerWithProfile(t *testing.T) {
 	_, _, ra, fc, cleanup := initAuthorities(t)
 	defer cleanup()
@@ -3694,9 +3766,7 @@ func TestIssueCertificateInnerWithProfile(t *testing.T) {
 	mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}}
 	ra.CA = &mockCA
 
-	// The basic mocks.StorageAuthority always succeeds on FinalizeOrder, which is
+	ra.SA = &mockSAWithFinalize{}
-	// the only SA call that issueCertificateInner makes.
-	ra.SA = &mocks.StorageAuthority{}
 
 	// Call issueCertificateInner with the CSR generated above and the profile
 	// name "default", which will cause the mockCA to return a specific hash.
@@ -3753,9 +3823,7 @@ func TestIssueCertificateOuter(t *testing.T) {
 	mockCA := MockCARecordingProfile{inner: &mocks.MockCA{PEM: certPEM}}
 	ra.CA = &mockCA
 
-	// The basic mocks.StorageAuthority always succeeds on FinalizeOrder, which is
+	ra.SA = &mockSAWithFinalize{}
-	// the only SA call that issueCertificateInner makes.
-	ra.SA = &mocks.StorageAuthority{}
 
 	_, err = ra.issueCertificateOuter(context.Background(), order, csr, certificateRequestEvent{})
 	test.AssertNotError(t, err, "Could not issue certificate")
@@ -3834,20 +3902,24 @@ rA==
 -----END CERTIFICATE-----
 `)
 
+// mockSARevocation is a fake which includes all of the SA methods called in the
+// course of a revocation. Its behavior can be customized by providing sets of
+// issued (known) certs, already-revoked certs, and already-blocked keys. It
+// also updates the sets of revoked certs and blocked keys when certain methods
+// are called, to allow for more complex test logic.
 type mockSARevocation struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 
 	known   map[string]*x509.Certificate
 	revoked map[string]*corepb.CertificateStatus
 	blocked []*sapb.AddBlockedKeyRequest
 }
 
-func newMockSARevocation(known *x509.Certificate, clk clock.Clock) *mockSARevocation {
+func newMockSARevocation(known *x509.Certificate) *mockSARevocation {
 	return &mockSARevocation{
-		StorageAuthority: *mocks.NewStorageAuthority(clk),
+		known:   map[string]*x509.Certificate{core.SerialToString(known.SerialNumber): known},
-		known:            map[string]*x509.Certificate{core.SerialToString(known.SerialNumber): known},
+		revoked: make(map[string]*corepb.CertificateStatus),
-		revoked:          make(map[string]*corepb.CertificateStatus),
+		blocked: make([]*sapb.AddBlockedKeyRequest, 0),
-		blocked:          make([]*sapb.AddBlockedKeyRequest, 0),
 	}
 }
 
@@ -3861,6 +3933,18 @@ func (msar *mockSARevocation) AddBlockedKey(_ context.Context, req *sapb.AddBloc
 	return &emptypb.Empty{}, nil
 }
 
+func (msar *mockSARevocation) GetSerialMetadata(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*sapb.SerialMetadata, error) {
+	if cert, present := msar.known[req.Serial]; present {
+		return &sapb.SerialMetadata{
+			Serial:         req.Serial,
+			RegistrationID: 1,
+			Created:        timestamppb.New(cert.NotBefore),
+			Expires:        timestamppb.New(cert.NotAfter),
+		}, nil
+	}
+	return nil, berrors.UnknownSerialError()
+}
+
 func (msar *mockSARevocation) GetLintPrecertificate(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.Certificate, error) {
 	if cert, present := msar.known[req.Serial]; present {
 		return &corepb.Certificate{Der: cert.Raw}, nil
@@ -3929,7 +4013,7 @@ func (mp *mockPurger) Purge(context.Context, *akamaipb.PurgeRequest, ...grpc.Cal
 
 // mockSAGenerateOCSP is a mock SA that always returns a good OCSP response, with a constant NotAfter.
 type mockSAGenerateOCSP struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 	expiration time.Time
 }
 
@@ -3968,7 +4052,7 @@ func TestGenerateOCSP(t *testing.T) {
 // removed from the certificateStatus table), but returns success to GetSerialMetadata (simulating
 // a serial number staying in the `serials` table indefinitely).
 type mockSALongExpiredSerial struct {
-	mocks.StorageAuthority
+	sapb.StorageAuthorityClient
 }
 
 func (msgo *mockSALongExpiredSerial) GetCertificateStatus(_ context.Context, req *sapb.Serial, _ ...grpc.CallOption) (*corepb.CertificateStatus, error) {
@@ -4042,7 +4126,7 @@ func TestRevokeCertByApplicant_Subscriber(t *testing.T) {
 	ra.issuersByNameID = map[issuance.NameID]*issuance.Certificate{
 		ic.NameID(): ic,
 	}
-	ra.SA = newMockSARevocation(cert, clk)
+	ra.SA = newMockSARevocation(cert)
 
 	// Revoking without a regID should fail.
 	_, err = ra.RevokeCertByApplicant(context.Background(), &rapb.RevokeCertByApplicantRequest{
@@ -4080,6 +4164,33 @@ func TestRevokeCertByApplicant_Subscriber(t *testing.T) {
 	test.AssertContains(t, err.Error(), "already revoked")
 }
 
+// mockSARevocationWithAuthzs embeds a mockSARevocation and so inherits all its
+// methods, but also adds GetValidAuthorizations2 so that it can pretend to
+// either be authorized or not for all of the names in the to-be-revoked cert.
+type mockSARevocationWithAuthzs struct {
+	*mockSARevocation
+	authorized bool
+}
+
+func (msa *mockSARevocationWithAuthzs) GetValidAuthorizations2(ctx context.Context, req *sapb.GetValidAuthorizationsRequest, _ ...grpc.CallOption) (*sapb.Authorizations, error) {
+	authzs := &sapb.Authorizations{}
+
+	if !msa.authorized {
+		return authzs, nil
+	}
+
+	for _, name := range req.Domains {
+		authzs.Authz = append(authzs.Authz, &sapb.Authorizations_MapElement{
+			Domain: name,
+			Authz: &corepb.Authorization{
+				Identifier: name,
+			},
+		})
+	}
+
+	return authzs, nil
+}
+
 func TestRevokeCertByApplicant_Controller(t *testing.T) {
 	_, _, ra, clk, cleanUp := initAuthorities(t)
 	defer cleanUp()
@@ -4095,10 +4206,12 @@ func TestRevokeCertByApplicant_Controller(t *testing.T) {
 	ra.issuersByNameID = map[issuance.NameID]*issuance.Certificate{
 		ic.NameID(): ic,
 	}
-	mockSA := newMockSARevocation(cert, clk)
+	mockSA := newMockSARevocation(cert)
-	ra.SA = mockSA
 
-	// Revoking with the wrong regID should fail.
+	// Revoking when the account doesn't have valid authzs for the name should fail.
+	// We use RegID 2 here and below because the mockSARevocation believes regID 1
+	// is the original issuer.
+	ra.SA = &mockSARevocationWithAuthzs{mockSA, false}
 	_, err = ra.RevokeCertByApplicant(context.Background(), &rapb.RevokeCertByApplicantRequest{
 		Cert:  cert.Raw,
 		Code:  ocsp.Unspecified,
@@ -4107,12 +4220,13 @@ func TestRevokeCertByApplicant_Controller(t *testing.T) {
 	test.AssertError(t, err, "should have failed with wrong RegID")
 	test.AssertContains(t, err.Error(), "requester does not control all names")
 
-	// Revoking with a different RegID that has valid authorizations should succeed,
+	// Revoking when the account does have valid authzs for the name should succeed,
 	// but override the revocation reason to cessationOfOperation.
+	ra.SA = &mockSARevocationWithAuthzs{mockSA, true}
 	_, err = ra.RevokeCertByApplicant(context.Background(), &rapb.RevokeCertByApplicantRequest{
 		Cert:  cert.Raw,
 		Code:  ocsp.Unspecified,
-		RegID: 5,
+		RegID: 2,
 	})
 	test.AssertNotError(t, err, "should have succeeded")
 	test.AssertEquals(t, mockSA.revoked[core.SerialToString(cert.SerialNumber)].RevokedReason, int64(ocsp.CessationOfOperation))
@@ -4135,7 +4249,7 @@ func TestRevokeCertByKey(t *testing.T) {
 	ra.issuersByNameID = map[issuance.NameID]*issuance.Certificate{
 		ic.NameID(): ic,
 	}
-	mockSA := newMockSARevocation(cert, clk)
+	mockSA := newMockSARevocation(cert)
 	ra.SA = mockSA
 
 	// Revoking should work, but override the requested reason and block the key.
@@ -4187,7 +4301,7 @@ func TestAdministrativelyRevokeCertificate(t *testing.T) {
 	ra.issuersByNameID = map[issuance.NameID]*issuance.Certificate{
 		ic.NameID(): ic,
 	}
-	mockSA := newMockSARevocation(cert, clk)
+	mockSA := newMockSARevocation(cert)
 	ra.SA = mockSA
 
 	// Revoking with an empty request should fail immediately.
@@ -4369,10 +4483,10 @@ func TestNewOrderFailedAuthzRateLimitingExempt(t *testing.T) {
 	test.AssertNotNil(t, order.Id, "initial order had a nil ID")
 	test.AssertEquals(t, numAuthorizations(order), 1)
 
-	// Mock SA that fails all authorizations for "example.com".
+	// Mock SA that has a failed authorization for "example.com".
 	ra.SA = &mockInvalidPlusValidAuthzAuthority{
-		mockInvalidAuthorizationsAuthority{
+		mockSAWithAuthzs{authzs: map[string]*core.Authorization{}},
-			domainWithFailures: "example.com"},
+		"example.com",
 	}
 
 	// Set up a rate limit policy that allows 1 order every 24 hours.
@@ -4394,6 +4508,26 @@ func TestNewOrderFailedAuthzRateLimitingExempt(t *testing.T) {
 	test.AssertNotError(t, err, "limit exempt order should have succeeded")
 }
 
+// An authority that returns an error from NewOrderAndAuthzs if the
+// "ReplacesSerial" field of the request is empty.
+type mockNewOrderMustBeReplacementAuthority struct {
+	mockSAWithAuthzs
+}
+
+func (sa *mockNewOrderMustBeReplacementAuthority) NewOrderAndAuthzs(ctx context.Context, req *sapb.NewOrderAndAuthzsRequest, _ ...grpc.CallOption) (*corepb.Order, error) {
+	if req.NewOrder.ReplacesSerial == "" {
+		return nil, status.Error(codes.InvalidArgument, "NewOrder is not a replacement")
+	}
+	return &corepb.Order{
+		Id:             1,
+		RegistrationID: req.NewOrder.RegistrationID,
+		Expires:        req.NewOrder.Expires,
+		Status:         string(core.StatusPending),
+		Created:        timestamppb.New(time.Now()),
+		Names:          req.NewOrder.Names,
+	}, nil
+}
+
 func TestNewOrderReplacesSerialCarriesThroughToSA(t *testing.T) {
 	_, _, ra, _, cleanUp := initAuthorities(t)
 	defer cleanUp()
@@ -4406,7 +4540,7 @@ func TestNewOrderReplacesSerialCarriesThroughToSA(t *testing.T) {
 
 	// Mock SA that returns an error from NewOrderAndAuthzs if the
 	// "ReplacesSerial" field of the request is empty.
-	ra.SA = &mockNewOrderMustBeReplacementAuthority{}
+	ra.SA = &mockNewOrderMustBeReplacementAuthority{mockSAWithAuthzs{}}
 
 	_, err := ra.NewOrder(ctx, exampleOrder)
 	test.AssertNotError(t, err, "order with ReplacesSerial should have succeeded")

wfe2/verify_test.go

@@ -20,7 +20,6 @@ import (
 	"github.com/letsencrypt/boulder/goodkey"
 	bgrpc "github.com/letsencrypt/boulder/grpc"
 	"github.com/letsencrypt/boulder/grpc/noncebalancer"
-	"github.com/letsencrypt/boulder/mocks"
 	noncepb "github.com/letsencrypt/boulder/nonce/proto"
 	"github.com/letsencrypt/boulder/probs"
 	sapb "github.com/letsencrypt/boulder/sa/proto"
@@ -1633,8 +1632,8 @@ func (sa mockSADifferentStoredKey) GetRegistration(_ context.Context, _ *sapb.Re
 }
 
 func TestValidPOSTForAccountSwappedKey(t *testing.T) {
-	wfe, fc, signer := setupWFE(t)
+	wfe, _, signer := setupWFE(t)
-	wfe.sa = &mockSADifferentStoredKey{mocks.NewStorageAuthorityReadOnly(fc)}
+	wfe.sa = &mockSADifferentStoredKey{}
 	wfe.accountGetter = wfe.sa
 	event := newRequestEvent()
 

wfe2/wfe_test.go

@@ -1673,7 +1673,7 @@ func TestAuthorization500(t *testing.T) {
 // a `GetAuthorization` implementation that can return authorizations with
 // failed challenges.
 type SAWithFailedChallenges struct {
-	mocks.StorageAuthorityReadOnly
+	sapb.StorageAuthorityReadOnlyClient
 	Clk clock.FakeClock
 }