From 91664be80bc061cf28c2db6eb65d98010ea96799 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Wed, 7 Dec 2022 12:23:11 +0100 Subject: [PATCH] htlcswitch: use fat errors as a sender --- channeldb/payments.go | 13 ++++ htlcswitch/failure.go | 63 +++++++++++++++++-- htlcswitch/failure_test.go | 2 +- htlcswitch/switch_test.go | 4 +- itest/lnd_multi-hop-error-propagation_test.go | 34 +++++++++- lnrpc/routerrpc/config.go | 1 + lnrpc/routerrpc/routing_config.go | 4 ++ routing/pathfind.go | 41 +++++++++++- routing/pathfind_test.go | 5 ++ routing/payment_lifecycle.go | 14 +++-- routing/payment_session.go | 1 + routing/route/route.go | 14 +++++ routing/router.go | 2 + sample-lnd.conf | 3 + server.go | 1 + 15 files changed, 186 insertions(+), 16 deletions(-) diff --git a/channeldb/payments.go b/channeldb/payments.go index 93abe4fb16..387ae968be 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -1176,6 +1176,12 @@ func serializeHop(w io.Writer, h *route.Hop) error { records = append(records, record.NewMetadataRecord(&h.Metadata)) } + // Signal attributable errors. + if h.AttrError { + attrError := record.NewAttributableError() + records = append(records, attrError.Record()) + } + // Final sanity check to absolutely rule out custom records that are not // custom and write into the standard range. if err := h.CustomRecords.Validate(); err != nil { @@ -1297,6 +1303,13 @@ func deserializeHop(r io.Reader) (*route.Hop, error) { h.Metadata = metadata } + attributableErrorType := uint64(record.AttributableErrorOnionType) + if _, ok := tlvMap[attributableErrorType]; ok { + delete(tlvMap, attributableErrorType) + + h.AttrError = true + } + h.CustomRecords = tlvMap return h, nil diff --git a/htlcswitch/failure.go b/htlcswitch/failure.go index 373263381f..e51746bc51 100644 --- a/htlcswitch/failure.go +++ b/htlcswitch/failure.go @@ -2,13 +2,17 @@ package htlcswitch import ( "bytes" + "encoding/binary" "fmt" + "strings" sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/htlcswitch/hop" "github.com/lightningnetwork/lnd/lnwire" ) +var byteOrder = binary.BigEndian + // ClearTextError is an interface which is implemented by errors that occur // when we know the underlying wire failure message. These errors are the // opposite to opaque errors which are onion-encrypted blobs only understandable @@ -166,7 +170,25 @@ type OnionErrorDecrypter interface { // SphinxErrorDecrypter wraps the sphinx data SphinxErrorDecrypter and maps the // returned errors to concrete lnwire.FailureMessage instances. type SphinxErrorDecrypter struct { - OnionErrorDecrypter + decrypter interface{} +} + +// NewSphinxErrorDecrypter instantiates a new error decryptor. +func NewSphinxErrorDecrypter(circuit *sphinx.Circuit, + attrError bool) *SphinxErrorDecrypter { + + var decrypter interface{} + if !attrError { + decrypter = sphinx.NewOnionErrorDecrypter(circuit) + } else { + decrypter = sphinx.NewOnionAttrErrorDecrypter( + circuit, hop.AttrErrorStruct, + ) + } + + return &SphinxErrorDecrypter{ + decrypter: decrypter, + } } // DecryptError peels off each layer of onion encryption from the first hop, to @@ -177,9 +199,42 @@ type SphinxErrorDecrypter struct { func (s *SphinxErrorDecrypter) DecryptError(reason lnwire.OpaqueReason) ( *ForwardingError, error) { - failure, err := s.OnionErrorDecrypter.DecryptError(reason) - if err != nil { - return nil, err + var failure *sphinx.DecryptedError + + switch decrypter := s.decrypter.(type) { + case OnionErrorDecrypter: + legacyError, err := decrypter.DecryptError(reason) + if err != nil { + return nil, err + } + + failure = legacyError + + case *sphinx.OnionAttrErrorDecrypter: + attributableError, err := decrypter.DecryptError(reason) + if err != nil { + return nil, err + } + + // Log hold times. + // + // TODO: Use to penalize nodes. + var holdTimes []string + for _, payload := range attributableError.Payloads { + // Read hold time. + holdTimeMs := byteOrder.Uint32(payload) + + holdTimes = append( + holdTimes, + fmt.Sprintf("%v", holdTimeMs), + ) + } + log.Debugf("Hold times: %v", strings.Join(holdTimes, "/")) + + failure = &attributableError.DecryptedError + + default: + panic("unexpected decrypter type") } // Decode the failure. If an error occurs, we leave the failure message diff --git a/htlcswitch/failure_test.go b/htlcswitch/failure_test.go index 48ebc66821..fe44b47e4f 100644 --- a/htlcswitch/failure_test.go +++ b/htlcswitch/failure_test.go @@ -52,7 +52,7 @@ func TestLongFailureMessage(t *testing.T) { } errorDecryptor := &SphinxErrorDecrypter{ - OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit), + decrypter: sphinx.NewOnionErrorDecrypter(circuit), } // Assert that the failure message can still be extracted. diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 28f3d9e2c8..b2317365cc 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -3253,7 +3253,7 @@ func TestInvalidFailure(t *testing.T) { // Get payment result from switch. We expect an unreadable failure // message error. deobfuscator := SphinxErrorDecrypter{ - OnionErrorDecrypter: &mockOnionErrorDecryptor{ + decrypter: &mockOnionErrorDecryptor{ err: ErrUnreadableFailureMessage, }, } @@ -3278,7 +3278,7 @@ func TestInvalidFailure(t *testing.T) { // Modify the decryption to simulate that decryption went alright, but // the failure cannot be decoded. deobfuscator = SphinxErrorDecrypter{ - OnionErrorDecrypter: &mockOnionErrorDecryptor{ + decrypter: &mockOnionErrorDecryptor{ sourceIdx: 2, message: []byte{200}, }, diff --git a/itest/lnd_multi-hop-error-propagation_test.go b/itest/lnd_multi-hop-error-propagation_test.go index 3941accb0b..ac2e1955c3 100644 --- a/itest/lnd_multi-hop-error-propagation_test.go +++ b/itest/lnd_multi-hop-error-propagation_test.go @@ -2,23 +2,53 @@ package itest import ( "math" + "testing" + "github.com/btcsuite/btcd/btcutil" "github.com/lightningnetwork/lnd/funding" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/lntest/node" "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" ) func testHtlcErrorPropagation(ht *lntest.HarnessTest) { + ht.Run("legacy error", func(tt *testing.T) { + st := ht.Subtest(tt) + st.EnsureConnected(st.Alice, st.Bob) + + // Test legacy errors using the standby node. + testHtlcErrorPropagationWithNode(st, st.Alice) + }) + + ht.Run("attr error", func(tt *testing.T) { + st := ht.Subtest(tt) + + // Create a different Alice node with attributable + // errors enabled. Alice will signal to Bob and Carol to + // return attributable errors to her. + alice := st.NewNode("Alice", []string{"--routerrpc.attrerrors"}) + st.FundCoins(btcutil.SatoshiPerBitcoin, alice) + + st.ConnectNodes(alice, st.Bob) + + testHtlcErrorPropagationWithNode(st, alice) + + st.Shutdown(alice) + }) +} + +func testHtlcErrorPropagationWithNode(ht *lntest.HarnessTest, + alice *node.HarnessNode) { + // In this test we wish to exercise the daemon's correct parsing, // handling, and propagation of errors that occur while processing a // multi-hop payment. const chanAmt = funding.MaxBtcFundingAmount - alice, bob := ht.Alice, ht.Bob - + bob := ht.Bob // Since we'd like to test some multi-hop failure scenarios, we'll // introduce another node into our test network: Carol. carol := ht.NewNode("Carol", nil) diff --git a/lnrpc/routerrpc/config.go b/lnrpc/routerrpc/config.go index adcc84e800..b2340126a6 100644 --- a/lnrpc/routerrpc/config.go +++ b/lnrpc/routerrpc/config.go @@ -87,5 +87,6 @@ func GetRoutingConfig(cfg *Config) *RoutingConfig { NodeWeight: cfg.BimodalConfig.NodeWeight, DecayTime: cfg.BimodalConfig.DecayTime, }, + AttrErrors: cfg.AttrErrors, } } diff --git a/lnrpc/routerrpc/routing_config.go b/lnrpc/routerrpc/routing_config.go index 31494d5464..1e0084a3ca 100644 --- a/lnrpc/routerrpc/routing_config.go +++ b/lnrpc/routerrpc/routing_config.go @@ -41,6 +41,10 @@ type RoutingConfig struct { // BimodalConfig defines parameters for the bimodal probability. BimodalConfig *BimodalConfig `group:"bimodal" namespace:"bimodal" description:"configuration for the bimodal pathfinding probability estimator"` + + // AttrErrors indicates whether attributable errors should be requested + // if the whole route supports it. + AttrErrors bool `long:"attrerrors" description:"request attributable errors if the whole route supports it"` } // AprioriConfig defines parameters for the apriori probability. diff --git a/routing/pathfind.go b/routing/pathfind.go index e293d320de..cf5845fb2f 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -11,6 +11,7 @@ import ( sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/feature" + "github.com/lightningnetwork/lnd/htlcswitch/hop" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" "github.com/lightningnetwork/lnd/routing/route" @@ -105,6 +106,36 @@ type finalHopParams struct { metadata []byte } +// useAttrErrors returns true if the path can use attributable errors. +func useAttrErrors(pathEdges []*channeldb.CachedEdgePolicy) bool { + // Use legacy errors if the route length exceeds the maximum number of + // hops for attributable errors. + if len(pathEdges) > hop.AttrErrorStruct.HopCount() { + return false + } + + // Every node along the path must signal support for attributable + // errors. + for _, edge := range pathEdges { + // Get the node features. + toFeat := edge.ToNodeFeatures + + // If there are no features known, assume the node cannot handle + // attributable errors. + if toFeat == nil { + return false + } + + // If the node does not signal support for attributable errors, + // do not use them. + if !toFeat.HasFeature(lnwire.AttributableErrorsOptional) { + return false + } + } + + return true +} + // newRoute constructs a route using the provided path and final hop constraints. // Any destination specific fields from the final hop params will be attached // assuming the destination's feature vector signals support, otherwise this @@ -117,7 +148,7 @@ type finalHopParams struct { // dependencies. func newRoute(sourceVertex route.Vertex, pathEdges []*channeldb.CachedEdgePolicy, currentHeight uint32, - finalHop finalHopParams) (*route.Route, error) { + finalHop finalHopParams, attrErrors bool) (*route.Route, error) { var ( hops []*route.Hop @@ -134,6 +165,9 @@ func newRoute(sourceVertex route.Vertex, nextIncomingAmount lnwire.MilliSatoshi ) + // Use attributable errors if enabled and supported by the route. + attributableErrors := attrErrors && useAttrErrors(pathEdges) + pathLength := len(pathEdges) for i := pathLength - 1; i >= 0; i-- { // Now we'll start to calculate the items within the per-hop @@ -250,6 +284,7 @@ func newRoute(sourceVertex route.Vertex, CustomRecords: customRecords, MPP: mpp, Metadata: metadata, + AttrError: attributableErrors, } hops = append([]*route.Hop{currentHop}, hops...) @@ -371,6 +406,10 @@ type PathFindingConfig struct { // MinProbability defines the minimum success probability of the // returned route. MinProbability float64 + + // AttrErrors indicates whether we should use the new attributable + // errors if the nodes on the route allow it. + AttrErrors bool } // getOutgoingBalance returns the maximum available balance in any of the diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 4224a4f638..ed69f9b7d5 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -959,6 +959,7 @@ func runFindLowestFeePath(t *testing.T, useCache bool) { cltvDelta: finalHopCLTV, records: nil, }, + false, ) require.NoError(t, err, "unable to create path") @@ -1101,6 +1102,7 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc cltvDelta: finalHopCLTV, records: nil, }, + false, ) require.NoError(t, err, "unable to create path") @@ -1639,6 +1641,7 @@ func TestNewRoute(t *testing.T) { paymentAddr: testCase.paymentAddr, metadata: testCase.metadata, }, + false, ) if testCase.expectError { @@ -2641,6 +2644,7 @@ func testCltvLimit(t *testing.T, useCache bool, limit uint32, cltvDelta: finalHopCLTV, records: nil, }, + false, ) require.NoError(t, err, "unable to create path") @@ -2964,6 +2968,7 @@ func runNoCycle(t *testing.T, useCache bool) { cltvDelta: finalHopCLTV, records: nil, }, + false, ) require.NoError(t, err, "unable to create path") diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index c6bd7e08e8..e0b25f3b2a 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -7,7 +7,6 @@ import ( "github.com/btcsuite/btcd/btcec/v2" "github.com/davecgh/go-spew/spew" - sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/lntypes" @@ -556,11 +555,14 @@ func (p *shardHandler) collectResult(attempt *channeldb.HTLCAttemptInfo) ( } // Using the created circuit, initialize the error decrypter so we can - // parse+decode any failures incurred by this payment within the - // switch. - errorDecryptor := &htlcswitch.SphinxErrorDecrypter{ - OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit), - } + // parse+decode any failures incurred by this payment within the switch. + // + // The resolution format to use for the decryption is based on the + // instruction that we gave to the first hop. + attrError := attempt.Route.Hops[0].AttrError + errorDecryptor := htlcswitch.NewSphinxErrorDecrypter( + circuit, attrError, + ) // Now ask the switch to return the result of the payment when // available. diff --git a/routing/payment_session.go b/routing/payment_session.go index 8b181a04d9..9228f4b9a8 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -391,6 +391,7 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, paymentAddr: p.payment.PaymentAddr, metadata: p.payment.Metadata, }, + p.pathFindingConfig.AttrErrors, ) if err != nil { return nil, err diff --git a/routing/route/route.go b/routing/route/route.go index 5992bd4046..f76b2e6c23 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -131,6 +131,9 @@ type Hop struct { // Metadata is additional data that is sent along with the payment to // the payee. Metadata []byte + + // AttrError signals that the sender wants to use attributable errors. + AttrError bool } // Copy returns a deep copy of the Hop. @@ -176,6 +179,12 @@ func (h *Hop) PackHopPayload(w io.Writer, nextChanID uint64) error { record.NewLockTimeRecord(&h.OutgoingTimeLock), ) + // Add attributable error structure if used. + if h.AttrError { + attrErrorRecord := record.NewAttributableError() + records = append(records, attrErrorRecord.Record()) + } + // BOLT 04 says the next_hop_id should be omitted for the final hop, // but present for all others. // @@ -255,6 +264,11 @@ func (h *Hop) PayloadSize(nextChanID uint64) uint64 { tlv.SizeTUint64(uint64(h.OutgoingTimeLock)), ) + // Add attributable error structure. + if h.AttrError { + addRecord(record.AttributableErrorOnionType, 0) + } + // Add next hop if present. if nextChanID != 0 { addRecord(record.NextHopOnionType, 8) diff --git a/routing/router.go b/routing/router.go index a38980bbaf..2c8164eb27 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1805,6 +1805,7 @@ func (r *ChannelRouter) FindRoute(source, target route.Vertex, cltvDelta: finalExpiry, records: destCustomRecords, }, + r.cfg.PathFindingConfig.AttrErrors, ) if err != nil { return nil, 0, err @@ -2798,6 +2799,7 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, records: nil, paymentAddr: payAddr, }, + r.cfg.PathFindingConfig.AttrErrors, ) } diff --git a/sample-lnd.conf b/sample-lnd.conf index bf738cadbe..7d050367f7 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1393,6 +1393,9 @@ ; failures in channels. ; routerrpc.bimodal.decaytime=168h +; Indicates whether attributable errors should be requested if the whole route +; supports it. (default=false) +; routerrpc.attrerrors=false [workers] diff --git a/server.go b/server.go index 28b35b31a7..641a95a030 100644 --- a/server.go +++ b/server.go @@ -940,6 +940,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, ), AttemptCostPPM: routingConfig.AttemptCostPPM, MinProbability: routingConfig.MinRouteProbability, + AttrErrors: routingConfig.AttrErrors, } sourceNode, err := chanGraph.SourceNode()