multi: add new strict-verify CLI option to control invoice usage

In this commit, we add a new CLI argument that allows a user to control
if we use strict verification or not. Strict verification relies on
checking the actual invoice state against lnd, and requires more state
for the Aperture server.

When strict verification isn't on, we rely only on the preimage payment
hash relationship. Namely that the only way a user can obtain the
preimage is to pay the invoice, and as we check the HMAC on the
macaroon, we know that we created it with an invoice obtained from lnd.

Author: Roasbeef

aperture.go

@@ -337,7 +337,7 @@ func (a *Aperture) Start(errChan chan error) error {
 
 			a.challenger, err = challenger.NewLNCChallenger(
 				session, lncStore, a.cfg.InvoiceBatchSize,
-				genInvoiceReq, errChan,
+				genInvoiceReq, errChan, a.cfg.StrictVerify,
 			)
 			if err != nil {
 				return fmt.Errorf("unable to start lnc "+
@@ -361,7 +361,7 @@ func (a *Aperture) Start(errChan chan error) error {
 
 			a.challenger, err = challenger.NewLndChallenger(
 				client, a.cfg.InvoiceBatchSize, genInvoiceReq,
-				context.Background, errChan,
+				context.Background, errChan, a.cfg.StrictVerify,
 			)
 			if err != nil {
 				return err

challenger/lnc.go

@@ -20,7 +20,7 @@ type LNCChallenger struct {
 // connect to an lnd backend to create payment challenges.
 func NewLNCChallenger(session *lnc.Session, lncStore lnc.Store,
 	invoiceBatchSize int, genInvoiceReq InvoiceRequestGenerator,
-	errChan chan<- error) (*LNCChallenger, error) {
+	errChan chan<- error, strictVerify bool) (*LNCChallenger, error) {
 
 	nodeConn, err := lnc.NewNodeConn(session, lncStore)
 	if err != nil {
@@ -35,7 +35,7 @@ func NewLNCChallenger(session *lnc.Session, lncStore lnc.Store,
 
 	lndChallenger, err := NewLndChallenger(
 		client, invoiceBatchSize, genInvoiceReq, nodeConn.CtxFunc,
-		errChan,
+		errChan, strictVerify,
 	)
 	if err != nil {
 		return nil, err

challenger/lnd.go

@@ -25,6 +25,10 @@ type LndChallenger struct {
 	invoicesCancel func()
 	invoicesCond   *sync.Cond
 
+	// strictVerify indicates whether we should verify the invoice states,
+	// or rely on the higher level preimage verification.
+	strictVerify bool
+
 	errChan chan<- error
 
 	quit chan struct{}
@@ -38,9 +42,8 @@ var _ Challenger = (*LndChallenger)(nil)
 // NewLndChallenger creates a new challenger that uses the given connection to
 // an lnd backend to create payment challenges.
 func NewLndChallenger(client InvoiceClient, batchSize int,
-	genInvoiceReq InvoiceRequestGenerator,
-	ctxFunc func() context.Context,
-	errChan chan<- error) (*LndChallenger, error) {
+	genInvoiceReq InvoiceRequestGenerator, ctxFunc func() context.Context,
+	errChan chan<- error, strictVerification bool) (*LndChallenger, error) {
 
 	// Make sure we have a valid context function. This will be called to
 	// create a new context for each call to the lnd client.
@@ -63,6 +66,7 @@ func NewLndChallenger(client InvoiceClient, batchSize int,
 		invoicesCond:  sync.NewCond(invoicesMtx),
 		quit:          make(chan struct{}),
 		errChan:       errChan,
+		strictVerify:  strictVerification,
 	}
 
 	err := challenger.Start()
@@ -78,6 +82,14 @@ func NewLndChallenger(client InvoiceClient, batchSize int,
 // invoices on startup and a subscription to all subsequent invoice updates
 // is created.
 func (l *LndChallenger) Start() error {
+	// If we aren't doing strict verification, then we can just exit here as
+	// we don't need the invoice state.
+	if !l.strictVerify {
+		log.Infof("Skipping invoice state tracking strict_verify=%v",
+			l.strictVerify)
+		return nil
+	}
+
 	// These are the default values for the subscription. In case there are
 	// no invoices yet, this will instruct lnd to just send us all updates.
 	// If there are existing invoices, these indices will be updated to
@@ -303,6 +315,12 @@ func (l *LndChallenger) NewChallenge(price int64) (string, lntypes.Hash,
 func (l *LndChallenger) VerifyInvoiceStatus(hash lntypes.Hash,
 	state lnrpc.Invoice_InvoiceState, timeout time.Duration) error {
 
+	// If we're not doing strict verification, we can skip this check.
+	if !l.strictVerify {
+		log.Tracef("Skipping invoice state check, pay_hash=%v", hash)
+		return nil
+	}
+
 	// Prevent the challenger to be shut down while we're still waiting for
 	// status updates.
 	l.wg.Add(1)

challenger/lnd_test.go

@@ -120,6 +120,7 @@ func newChallenger() (*LndChallenger, *mockInvoiceClient, chan error) {
 		invoicesMtx:   invoicesMtx,
 		invoicesCond:  sync.NewCond(invoicesMtx),
 		errChan:       mainErrChan,
+		strictVerify:  true,
 	}, mockClient, mainErrChan
 }
 
@@ -142,7 +143,7 @@ func TestLndChallenger(t *testing.T) {
 	// First of all, test that the NewLndChallenger doesn't allow a nil
 	// invoice generator function.
 	errChan := make(chan error)
-	_, err := NewLndChallenger(nil, 1, nil, nil, errChan)
+	_, err := NewLndChallenger(nil, 1, nil, nil, errChan, true)
 	require.Error(t, err)
 
 	// Now mock the lnd backend and create a challenger instance that we can

config.go

@@ -20,6 +20,7 @@ var (
 	defaultLogLevel         = "info"
 	defaultLogFilename      = "aperture.log"
 	defaultInvoiceBatchSize = 100000
+	defaultStrictVerify     = false
 
 	defaultSqliteDatabaseFileName = "aperture.db"
 
@@ -224,6 +225,11 @@ type Config struct {
 	// request.
 	InvoiceBatchSize int `long:"invoicebatchsize" description:"The number of invoices to fetch in a single request."`
 
+	// StrictVerify is a flag that indicates whether we should verify the
+	// invoice status strictly or not. If set to true, then this requires
+	// all invoices to be read from disk at start up.
+	StrictVerify bool `long:"strictverify" description:"Whether to verify the invoice status strictly or not."`
+
 	// Logging controls various aspects of aperture logging.
 	Logging *build.LogConfig `group:"logging" namespace:"logging"`
 
@@ -274,5 +280,6 @@ func NewConfig() *Config {
 		InvoiceBatchSize: defaultInvoiceBatchSize,
 		Logging:          build.DefaultLogConfig(),
 		Blocklist:        []string{},
+		StrictVerify:     defaultStrictVerify,
 	}
 }

sample-conf.yaml

@@ -19,6 +19,10 @@ debuglevel: "debug"
 autocert: false
 servername: aperture.example.com
 
+# Whether we should verify the invoice status strictly or not. If set to true,
+# then this requires all invoices to be read from disk at start up.
+strictverify: false
+
 # The port on which the pprof profile will be served. If no port is provided,
 # the profile will not be served.
 profile: 9999