From 459a5728db8e1dc4b93caaaedc089b6d9f8530fa Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 12 May 2025 16:49:31 -0700 Subject: [PATCH] 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. --- aperture.go | 4 ++-- challenger/lnc.go | 4 ++-- challenger/lnd.go | 24 +++++++++++++++++++++--- challenger/lnd_test.go | 3 ++- config.go | 7 +++++++ sample-conf.yaml | 4 ++++ 6 files changed, 38 insertions(+), 8 deletions(-) diff --git a/aperture.go b/aperture.go index b19599f..7d7d558 100644 --- a/aperture.go +++ b/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 diff --git a/challenger/lnc.go b/challenger/lnc.go index aea3672..0931be2 100644 --- a/challenger/lnc.go +++ b/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 diff --git a/challenger/lnd.go b/challenger/lnd.go index e2533f1..cbe13d3 100644 --- a/challenger/lnd.go +++ b/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) diff --git a/challenger/lnd_test.go b/challenger/lnd_test.go index 62bcc45..34d4138 100644 --- a/challenger/lnd_test.go +++ b/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 diff --git a/config.go b/config.go index cba9d38..a24437d 100644 --- a/config.go +++ b/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, } } diff --git a/sample-conf.yaml b/sample-conf.yaml index b5e1502..0b7bdb8 100644 --- a/sample-conf.yaml +++ b/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