multi: update PaymentAddr to use fn.Option

Since it is allowed to not be set and so can lead to nil deref panics if
it is a pointer.
This commit is contained in:
Elle Mouton 2024-09-24 12:36:15 +09:00
parent 84c91f701c
commit 7dc86acb8c
No known key found for this signature in database
GPG Key ID: D7D916376026F177
14 changed files with 87 additions and 70 deletions

View File

@ -23,6 +23,10 @@
propagate mission control and debug level config values to the main LND config
struct so that the GetDebugInfo response is accurate.
* [Fix a bug](https://github.com/lightningnetwork/lnd/pull/9134) that would
cause a nil pointer dereference during the probing of a payment request that
does not contain a payment address.
# New Features
## Functional Enhancements
## RPC Additions
@ -69,5 +73,6 @@
# Contributors (Alphabetical Order)
* CharlieZKSmith
* Elle Mouton
* Pins
* Ziggie

View File

@ -16,6 +16,7 @@ import (
sphinx "github.com/lightningnetwork/lightning-onion"
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/feature"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/htlcswitch"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lntypes"
@ -1011,7 +1012,7 @@ func (r *RouterBackend) extractIntentFromSendRequest(
if len(rpcPayReq.PaymentAddr) > 0 {
var addr [32]byte
copy(addr[:], rpcPayReq.PaymentAddr)
payAddr = &addr
payAddr = fn.Some(addr)
}
} else {
err = payIntent.SetPaymentHash(*payReq.PaymentHash)
@ -1128,7 +1129,7 @@ func (r *RouterBackend) extractIntentFromSendRequest(
} else {
copy(payAddr[:], rpcPayReq.PaymentAddr)
}
payIntent.PaymentAddr = &payAddr
payIntent.PaymentAddr = fn.Some(payAddr)
// Generate random SetID and root share.
var setID [32]byte
@ -1167,7 +1168,7 @@ func (r *RouterBackend) extractIntentFromSendRequest(
var payAddr [32]byte
copy(payAddr[:], rpcPayReq.PaymentAddr)
payIntent.PaymentAddr = &payAddr
payIntent.PaymentAddr = fn.Some(payAddr)
}
}

View File

@ -528,11 +528,16 @@ func (s *Server) probePaymentRequest(ctx context.Context, paymentRequest string,
AmtMsat: amtMsat,
PaymentHash: paymentHash[:],
FeeLimitSat: routeFeeLimitSat,
PaymentAddr: payReq.PaymentAddr[:],
FinalCltvDelta: int32(payReq.MinFinalCLTVExpiry()),
DestFeatures: MarshalFeatures(payReq.Features),
}
// If the payment addresses is specified, then we'll also populate that
// now as well.
payReq.PaymentAddr.WhenSome(func(addr [32]byte) {
copy(probeRequest.PaymentAddr, addr[:])
})
hints := payReq.RouteHints
// If the hints don't indicate an LSP then chances are that our probe
@ -1453,12 +1458,12 @@ func (s *Server) BuildRoute(_ context.Context,
outgoingChan = &req.OutgoingChanId
}
var payAddr *[32]byte
var payAddr fn.Option[[32]byte]
if len(req.PaymentAddr) != 0 {
var backingPayAddr [32]byte
copy(backingPayAddr[:], req.PaymentAddr)
payAddr = &backingPayAddr
payAddr = fn.Some(backingPayAddr)
}
if req.FinalCltvDelta == 0 {

View File

@ -187,7 +187,7 @@ func (c *integratedRoutingContext) testPayment(maxParts uint32,
FinalCLTVDelta: uint16(c.finalExpiry),
FeeLimit: lnwire.MaxMilliSatoshi,
Target: c.target.pubkey,
PaymentAddr: &paymentAddr,
PaymentAddr: fn.Some(paymentAddr),
DestFeatures: lnwire.NewFeatureVector(
baseFeatureBits, lnwire.Features,
),

View File

@ -111,7 +111,7 @@ type finalHopParams struct {
cltvDelta uint16
records record.CustomSet
paymentAddr *[32]byte
paymentAddr fn.Option[[32]byte]
// metadata is additional data that is sent along with the payment to
// the payee.
@ -226,7 +226,7 @@ func newRoute(sourceVertex route.Vertex,
// If we're attaching a payment addr but the receiver
// doesn't support both TLV and payment addrs, fail.
payAddr := supports(lnwire.PaymentAddrOptional)
if !payAddr && finalHop.paymentAddr != nil {
if !payAddr && finalHop.paymentAddr.IsSome() {
return nil, errors.New("cannot attach " +
"payment addr")
}
@ -234,12 +234,9 @@ func newRoute(sourceVertex route.Vertex,
// Otherwise attach the mpp record if it exists.
// TODO(halseth): move this to payment life cycle,
// where AMP options are set.
if finalHop.paymentAddr != nil {
mpp = record.NewMPP(
finalHop.totalAmt,
*finalHop.paymentAddr,
)
}
finalHop.paymentAddr.WhenSome(func(addr [32]byte) {
mpp = record.NewMPP(finalHop.totalAmt, addr)
})
metadata = finalHop.metadata
@ -452,7 +449,7 @@ type RestrictParams struct {
// PaymentAddr is a random 32-byte value generated by the receiver to
// mitigate probing vectors and payment sniping attacks on overpaid
// invoices.
PaymentAddr *[32]byte
PaymentAddr fn.Option[[32]byte]
// Amp signals to the pathfinder that this payment is an AMP payment
// and therefore it needs to account for additional AMP data in the
@ -608,7 +605,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
// checking that it supports the features we need. If the caller has a
// payment address to attach, check that our destination feature vector
// supports them.
if r.PaymentAddr != nil &&
if r.PaymentAddr.IsSome() &&
!features.HasFeature(lnwire.PaymentAddrOptional) {
return nil, 0, errNoPaymentAddr
@ -1435,9 +1432,9 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32,
}
var mpp *record.MPP
if r.PaymentAddr != nil {
mpp = record.NewMPP(amount, *r.PaymentAddr)
}
r.PaymentAddr.WhenSome(func(addr [32]byte) {
mpp = record.NewMPP(amount, addr)
})
var amp *record.AMP
if r.Amp != nil {

View File

@ -1367,7 +1367,7 @@ func TestNewRoute(t *testing.T) {
// overwrite the final hop's feature vector in the graph.
destFeatures *lnwire.FeatureVector
paymentAddr *[32]byte
paymentAddr fn.Option[[32]byte]
// metadata is the payment metadata to attach to the route.
metadata []byte
@ -1446,7 +1446,7 @@ func TestNewRoute(t *testing.T) {
// a fee to receive the payment.
name: "two hop single shot mpp",
destFeatures: tlvPayAddrFeatures,
paymentAddr: &testPaymentAddr,
paymentAddr: fn.Some(testPaymentAddr),
paymentAmount: 100000,
hops: []*models.CachedEdgePolicy{
createHop(0, 1000, 1000000, 10),
@ -1911,7 +1911,7 @@ func runDestPaymentAddr(t *testing.T, useCache bool) {
luoji := ctx.keyFromAlias("luoji")
// Add payment address w/o any invoice features.
ctx.restrictParams.PaymentAddr = &[32]byte{1}
ctx.restrictParams.PaymentAddr = fn.Some([32]byte{1})
// Add empty destination features. This should cause us to fail, since
// this overrides anything in the graph.
@ -2955,7 +2955,7 @@ func runInboundFees(t *testing.T, useCache bool) {
ctx := newPathFindingTestContext(t, useCache, testChannels, "a")
payAddr := [32]byte{1}
ctx.restrictParams.PaymentAddr = &payAddr
ctx.restrictParams.PaymentAddr = fn.Some(payAddr)
ctx.restrictParams.DestFeatures = tlvPayAddrFeatures
const (
@ -2974,7 +2974,7 @@ func runInboundFees(t *testing.T, useCache bool) {
amt: paymentAmt,
cltvDelta: finalHopCLTV,
records: nil,
paymentAddr: &payAddr,
paymentAddr: fn.Some(payAddr),
totalAmt: paymentAmt,
},
nil,
@ -3469,7 +3469,7 @@ func TestLastHopPayloadSize(t *testing.T) {
{
name: "Non blinded final hop",
restrictions: &RestrictParams{
PaymentAddr: paymentAddr,
PaymentAddr: fn.Some(*paymentAddr),
DestCustomRecords: customRecords,
Metadata: metadata,
Amp: ampOptions,
@ -3501,12 +3501,10 @@ func TestLastHopPayloadSize(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
var mpp *record.MPP
if tc.restrictions.PaymentAddr != nil {
mpp = record.NewMPP(
tc.amount, *tc.restrictions.PaymentAddr,
)
}
mpp := fn.MapOptionZ(tc.restrictions.PaymentAddr,
func(addr [32]byte) *record.MPP {
return record.NewMPP(tc.amount, addr)
})
// In case it's an AMP payment we use the max AMP record
// size to estimate the final hop size.

View File

@ -344,7 +344,7 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
// record. If it has a blinded path though, then we
// can split. Split payments to blinded paths won't have
// MPP records.
if p.payment.PaymentAddr == nil &&
if p.payment.PaymentAddr.IsNone() &&
p.payment.BlindedPathSet == nil {
p.log.Debugf("not splitting because payment " +

View File

@ -862,7 +862,7 @@ type LightningPayment struct {
// PaymentAddr is the payment address specified by the receiver. This
// field should be a random 32-byte nonce presented in the receiver's
// invoice to prevent probing of the destination.
PaymentAddr *[32]byte
PaymentAddr fn.Option[[32]byte]
// PaymentRequest is an optional payment request that this payment is
// attempting to complete.
@ -1063,9 +1063,10 @@ func (r *ChannelRouter) PreparePayment(payment *LightningPayment) (
switch {
// If this is an AMP payment, we'll use the AMP shard tracker.
case payment.amp != nil:
addr := payment.PaymentAddr.UnwrapOr([32]byte{})
shardTracker = amp.NewShardTracker(
payment.amp.RootShare, payment.amp.SetID,
*payment.PaymentAddr, payment.Amount,
payment.amp.RootShare, payment.amp.SetID, addr,
payment.Amount,
)
// Otherwise we'll use the simple tracker that will map each attempt to
@ -1367,8 +1368,8 @@ func (e ErrNoChannel) Error() string {
// outgoing channel, use the outgoingChan parameter.
func (r *ChannelRouter) BuildRoute(amt fn.Option[lnwire.MilliSatoshi],
hops []route.Vertex, outgoingChan *uint64, finalCltvDelta int32,
payAddr *[32]byte, firstHopBlob fn.Option[[]byte]) (*route.Route,
error) {
payAddr fn.Option[[32]byte], firstHopBlob fn.Option[[]byte]) (
*route.Route, error) {
log.Tracef("BuildRoute called: hopsCount=%v, amt=%v", len(hops), amt)

View File

@ -1653,14 +1653,14 @@ func TestBuildRoute(t *testing.T) {
// Test that we can't build a route when no hops are given.
hops = []route.Vertex{}
_, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, nil, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.None[[32]byte](), fn.None[[]byte](),
)
require.Error(t, err)
// Create hop list for an unknown destination.
hops := []route.Vertex{ctx.aliases["b"], ctx.aliases["y"]}
_, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, &payAddr, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.Some(payAddr), fn.None[[]byte](),
)
noChanErr := ErrNoChannel{}
require.ErrorAs(t, err, &noChanErr)
@ -1672,7 +1672,8 @@ func TestBuildRoute(t *testing.T) {
// Build the route for the given amount.
rt, err := ctx.router.BuildRoute(
fn.Some(amt), hops, nil, 40, &payAddr, fn.None[[]byte](),
fn.Some(amt), hops, nil, 40, fn.Some(payAddr),
fn.None[[]byte](),
)
require.NoError(t, err)
@ -1684,7 +1685,7 @@ func TestBuildRoute(t *testing.T) {
// Build the route for the minimum amount.
rt, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, &payAddr, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.Some(payAddr), fn.None[[]byte](),
)
require.NoError(t, err)
@ -1702,7 +1703,7 @@ func TestBuildRoute(t *testing.T) {
// There is no amount that can pass through both channel 5 and 4.
hops = []route.Vertex{ctx.aliases["e"], ctx.aliases["c"]}
_, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, nil, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.None[[32]byte](), fn.None[[]byte](),
)
require.Error(t, err)
noChanErr = ErrNoChannel{}
@ -1722,7 +1723,7 @@ func TestBuildRoute(t *testing.T) {
// policy of channel 3.
hops = []route.Vertex{ctx.aliases["b"], ctx.aliases["z"]}
rt, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, &payAddr, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.Some(payAddr), fn.None[[]byte](),
)
require.NoError(t, err)
checkHops(rt, []uint64{1, 8}, payAddr)
@ -1736,7 +1737,8 @@ func TestBuildRoute(t *testing.T) {
hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]}
amt = lnwire.NewMSatFromSatoshis(100)
rt, err = ctx.router.BuildRoute(
fn.Some(amt), hops, nil, 40, &payAddr, fn.None[[]byte](),
fn.Some(amt), hops, nil, 40, fn.Some(payAddr),
fn.None[[]byte](),
)
require.NoError(t, err)
checkHops(rt, []uint64{9, 10}, payAddr)
@ -1752,7 +1754,7 @@ func TestBuildRoute(t *testing.T) {
// is a third pass through newRoute in which this gets corrected to end
hops = []route.Vertex{ctx.aliases["d"], ctx.aliases["f"]}
rt, err = ctx.router.BuildRoute(
noAmt, hops, nil, 40, &payAddr, fn.None[[]byte](),
noAmt, hops, nil, 40, fn.Some(payAddr), fn.None[[]byte](),
)
require.NoError(t, err)
checkHops(rt, []uint64{9, 10}, payAddr)

View File

@ -5321,7 +5321,7 @@ type rpcPaymentIntent struct {
outgoingChannelIDs []uint64
lastHop *route.Vertex
destFeatures *lnwire.FeatureVector
paymentAddr *[32]byte
paymentAddr fn.Option[[32]byte]
payReq []byte
metadata []byte
blindedPathSet *routing.BlindedPaymentPathSet
@ -5530,8 +5530,9 @@ func (r *rpcServer) extractPaymentIntent(rpcPayReq *rpcPaymentRequest) (rpcPayme
// Note that the payment address for the payIntent should be nil if none
// was provided with the rpcPaymentRequest.
if len(rpcPayReq.PaymentAddr) != 0 {
payIntent.paymentAddr = &[32]byte{}
copy(payIntent.paymentAddr[:], rpcPayReq.PaymentAddr)
var addr [32]byte
copy(addr[:], rpcPayReq.PaymentAddr)
payIntent.paymentAddr = fn.Some(addr)
}
// Otherwise, If the payment request field was not specified
@ -7380,10 +7381,7 @@ func (r *rpcServer) DecodePayReq(ctx context.Context,
}
// Extract the payment address from the payment request, if present.
var paymentAddr []byte
if payReq.PaymentAddr != nil {
paymentAddr = payReq.PaymentAddr[:]
}
paymentAddr := payReq.PaymentAddr.UnwrapOr([32]byte{})
dest := payReq.Destination.SerializeCompressed()
return &lnrpc.PayReq{
@ -7399,7 +7397,7 @@ func (r *rpcServer) DecodePayReq(ctx context.Context,
CltvExpiry: int64(payReq.MinFinalCLTVExpiry()),
RouteHints: routeHints,
BlindedPaths: blindedPaymentPaths,
PaymentAddr: paymentAddr,
PaymentAddr: paymentAddr[:],
Features: invoicesrpc.CreateRPCFeatures(payReq.Features),
}, nil
}

View File

@ -14,6 +14,7 @@ import (
"github.com/btcsuite/btcd/btcutil/bech32"
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/lnwire"
)
@ -278,13 +279,19 @@ func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) er
invoice.PaymentHash, err = parse32Bytes(base32Data)
case fieldTypeS:
if invoice.PaymentAddr != nil {
if invoice.PaymentAddr.IsSome() {
// We skip the field if we have already seen a
// supported one.
continue
}
invoice.PaymentAddr, err = parse32Bytes(base32Data)
addr, err := parse32Bytes(base32Data)
if err != nil {
return err
}
if addr != nil {
invoice.PaymentAddr = fn.Some(*addr)
}
case fieldTypeD:
if invoice.Description != nil {

View File

@ -9,6 +9,7 @@ import (
"github.com/btcsuite/btcd/btcutil/bech32"
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcd/chaincfg/chainhash"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/lnwire"
)
@ -301,14 +302,14 @@ func writeTaggedFields(bufferBase32 *bytes.Buffer, invoice *Invoice) error {
return err
}
}
if invoice.PaymentAddr != nil {
err := writeBytes32(
bufferBase32, fieldTypeS, *invoice.PaymentAddr,
)
err := fn.MapOptionZ(invoice.PaymentAddr, func(addr [32]byte) error {
return writeBytes32(bufferBase32, fieldTypeS, addr)
})
if err != nil {
return err
}
}
if invoice.Features.SerializeSize32() > 0 {
var b bytes.Buffer
err := invoice.Features.RawFeatureVector.EncodeBase32(&b)

View File

@ -8,6 +8,7 @@ import (
"github.com/btcsuite/btcd/btcec/v2"
"github.com/btcsuite/btcd/btcutil"
"github.com/btcsuite/btcd/chaincfg"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/lnwire"
)
@ -136,7 +137,7 @@ type Invoice struct {
// PaymentAddr is the payment address to be used by payments to prevent
// probing of the destination.
PaymentAddr *[32]byte
PaymentAddr fn.Option[[32]byte]
// Destination is the public key of the target node. This will always
// be set after decoding, and can optionally be set before encoding to
@ -301,7 +302,7 @@ func Features(features *lnwire.FeatureVector) func(*Invoice) {
// the desired payment address that is advertised on the invoice.
func PaymentAddr(addr [32]byte) func(*Invoice) {
return func(i *Invoice) {
i.PaymentAddr = &addr
i.PaymentAddr = fn.Some(addr)
}
}

View File

@ -17,6 +17,7 @@ import (
"github.com/btcsuite/btcd/chaincfg"
"github.com/btcsuite/btcd/chaincfg/chainhash"
sphinx "github.com/lightningnetwork/lightning-onion"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/stretchr/testify/require"
)
@ -562,7 +563,7 @@ func TestDecodeEncode(t *testing.T) {
MilliSat: &testMillisat25mBTC,
Timestamp: time.Unix(1496314658, 0),
PaymentHash: &testPaymentHash,
PaymentAddr: &specPaymentAddr,
PaymentAddr: fn.Some(specPaymentAddr),
Description: &testCoffeeBeans,
Destination: testPubKey,
Features: lnwire.NewFeatureVector(
@ -589,7 +590,7 @@ func TestDecodeEncode(t *testing.T) {
MilliSat: &testMillisat25mBTC,
Timestamp: time.Unix(1496314658, 0),
PaymentHash: &testPaymentHash,
PaymentAddr: &specPaymentAddr,
PaymentAddr: fn.Some(specPaymentAddr),
Description: &testCoffeeBeans,
Destination: testPubKey,
Features: lnwire.NewFeatureVector(
@ -718,7 +719,7 @@ func TestDecodeEncode(t *testing.T) {
PaymentHash: &testPaymentHash,
Description: &testPaymentMetadata,
Destination: testPubKey,
PaymentAddr: &specPaymentAddr,
PaymentAddr: fn.Some(specPaymentAddr),
Features: lnwire.NewFeatureVector(
lnwire.NewRawFeatureVector(8, 14, 48),
lnwire.Features,
@ -772,7 +773,7 @@ func TestDecodeEncode(t *testing.T) {
MilliSat: &testMillisat25mBTC,
Timestamp: time.Unix(1496314658, 0),
PaymentHash: &testPaymentHash,
PaymentAddr: &specPaymentAddr,
PaymentAddr: fn.Some(specPaymentAddr),
Description: &testCoffeeBeans,
Destination: testPubKey,
Features: lnwire.NewFeatureVector(
@ -803,7 +804,7 @@ func TestDecodeEncode(t *testing.T) {
MilliSat: &testMillisat25mBTC,
Timestamp: time.Unix(1496314658, 0),
PaymentHash: &testPaymentHash,
PaymentAddr: &specPaymentAddr,
PaymentAddr: fn.Some(specPaymentAddr),
Description: &testCoffeeBeans,
Destination: testPubKey,
Features: lnwire.NewFeatureVector(