From fdbdcf1560f690f95905b7b8a38344e7b3aa65d5 Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Mon, 19 Feb 2018 10:18:16 -0500 Subject: [PATCH 1/3] zpay32: handle segwit prefixes > 2 chars This change fixes a bug when an invoice is decoded for a network whose bech32 segwit prefix is longer than 2 characters. The length of the Bech32HRPSegwit network parameter is used to determine where in the human-readable portion of the invoice the amount begins, rather than assuming it begins after the first four characters. Decode() now throws an error when the encoded invoice does not match the active network. Changes the minimum hrp length check to >= 3 instead of >= 4. Also removes a redundant "if ...; err != nil check" that was raising a warning in invoice.go. --- zpay32/invoice.go | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/zpay32/invoice.go b/zpay32/invoice.go index 0856d3ea..4dbbfeec 100644 --- a/zpay32/invoice.go +++ b/zpay32/invoice.go @@ -265,9 +265,9 @@ func NewInvoice(net *chaincfg.Params, paymentHash [32]byte, return invoice, nil } -// Decode parses the provided encoded invoice, and returns a decoded Invoice in -// case it is valid by BOLT-0011. -func Decode(invoice string) (*Invoice, error) { +// Decode parses the provided encoded invoice and returns a decoded Invoice if +// it is valid by BOLT-0011 and matches the provided active network. +func Decode(invoice string, net *chaincfg.Params) (*Invoice, error) { decodedInvoice := Invoice{} // Decode the invoice using the modified bech32 decoder. @@ -276,9 +276,9 @@ func Decode(invoice string) (*Invoice, error) { return nil, err } - // We expect the human-readable part to at least have ln + two chars + // We expect the human-readable part to at least have ln + one char // encoding the network. - if len(hrp) < 4 { + if len(hrp) < 3 { return nil, fmt.Errorf("hrp too short") } @@ -288,24 +288,17 @@ func Decode(invoice string) (*Invoice, error) { } // The next characters should be a valid prefix for a segwit BIP173 - // address. This will also determine which network this invoice is - // meant for. - var net *chaincfg.Params - if strings.HasPrefix(hrp[2:], chaincfg.MainNetParams.Bech32HRPSegwit) { - net = &chaincfg.MainNetParams - } else if strings.HasPrefix(hrp[2:], chaincfg.TestNet3Params.Bech32HRPSegwit) { - net = &chaincfg.TestNet3Params - } else if strings.HasPrefix(hrp[2:], chaincfg.SimNetParams.Bech32HRPSegwit) { - net = &chaincfg.SimNetParams - } else { + // address that match the active network. + if !strings.HasPrefix(hrp[2:], net.Bech32HRPSegwit) { return nil, fmt.Errorf("unknown network") } decodedInvoice.Net = net - // Optionally, if there's anything left of the HRP, it encodes the - // payment amount. - if len(hrp) > 4 { - amount, err := decodeAmount(hrp[4:]) + // Optionally, if there's anything left of the HRP after ln + the segwit + // prefix, we try to decode this as the payment amount. + var netPrefixLength = len(net.Bech32HRPSegwit) + 2 + if len(hrp) > netPrefixLength { + amount, err := decodeAmount(hrp[netPrefixLength:]) if err != nil { return nil, err } @@ -573,11 +566,7 @@ func parseData(invoice *Invoice, data []byte, net *chaincfg.Params) error { // The rest are tagged parts. tagData := data[7:] - if err := parseTaggedFields(invoice, tagData, net); err != nil { - return err - } - - return nil + return parseTaggedFields(invoice, tagData, net) } // parseTimestamp converts a 35-bit timestamp (encoded in base32) to uint64. From 6e5477e378b38a2ad2e2e9c3c70238663d2e5bed Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Mon, 19 Feb 2018 10:19:24 -0500 Subject: [PATCH 2/3] zpay32 test: litecoin decode tests New tests are added for creating, decoding, and re-encoding litecoin invoices for both mainnet and testnet, as well as a test that expects an error when the active network mismatches the invoice. --- zpay32/invoice_test.go | 109 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 97 insertions(+), 12 deletions(-) diff --git a/zpay32/invoice_test.go b/zpay32/invoice_test.go index a5891d2c..64826ac6 100644 --- a/zpay32/invoice_test.go +++ b/zpay32/invoice_test.go @@ -15,7 +15,10 @@ import ( "github.com/roasbeef/btcd/btcec" "github.com/roasbeef/btcd/chaincfg" "github.com/roasbeef/btcd/chaincfg/chainhash" + "github.com/roasbeef/btcd/wire" "github.com/roasbeef/btcutil" + + litecoinCfg "github.com/ltcsuite/ltcd/chaincfg" ) var ( @@ -90,11 +93,24 @@ var ( // Must be initialized in init(). testPaymentHash [32]byte testDescriptionHash [32]byte + + ltcTestNetParams chaincfg.Params + ltcMainNetParams chaincfg.Params ) func init() { copy(testPaymentHash[:], testPaymentHashSlice[:]) copy(testDescriptionHash[:], testDescriptionHashSlice[:]) + + // Initialize litecoin testnet and mainnet params by applying key fields + // to copies of bitcoin params. + // TODO(sangaman): create an interface for chaincfg.params + ltcTestNetParams = chaincfg.TestNet3Params + ltcTestNetParams.Net = wire.BitcoinNet(litecoinCfg.TestNet4Params.Net) + ltcTestNetParams.Bech32HRPSegwit = litecoinCfg.TestNet4Params.Bech32HRPSegwit + ltcMainNetParams = chaincfg.MainNetParams + ltcMainNetParams.Net = wire.BitcoinNet(litecoinCfg.MainNetParams.Net) + ltcMainNetParams.Bech32HRPSegwit = litecoinCfg.MainNetParams.Bech32HRPSegwit } // TestDecodeEncode tests that an encoded invoice gets decoded into the expected @@ -123,7 +139,7 @@ func TestDecodeEncode(t *testing.T) { valid: false, }, { - encodedInvoice: "lnb1asdsaddnv4wudz", // hrp too short + encodedInvoice: "ln1asdsaddnv4wudz", // hrp too short valid: false, }, { @@ -515,10 +531,64 @@ func TestDecodeEncode(t *testing.T) { return i }, }, + { + // Decode a mainnet invoice while expecting active net to be testnet + encodedInvoice: "lnbc241pveeq09pp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdqqnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66jd3m5klcwhq68vdsmx2rjgxeay5v0tkt2v5sjaky4eqahe4fx3k9sqavvce3capfuwv8rvjng57jrtfajn5dkpqv8yelsewtljwmmycq62k443", + valid: false, + decodedInvoice: func() *Invoice { + return &Invoice{ + Net: &chaincfg.TestNet3Params, + MilliSat: &testMillisat24BTC, + Timestamp: time.Unix(1503429093, 0), + PaymentHash: &testPaymentHash, + Destination: testPubKey, + Description: &testEmptyString, + } + }, + skipEncoding: true, // Skip encoding since we were given the wrong net + }, + { + // Decode a litecoin testnet invoice + encodedInvoice: "lntltc241pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66m2eq2fx9uctzkmj30meaghyskkgsd6geap5qg9j2ae444z24a4p8xg3a6g73p8l7d689vtrlgzj0wyx2h6atq8dfty7wmkt4frx9g9sp730h5a", + valid: true, + decodedInvoice: func() *Invoice { + return &Invoice{ + // TODO(sangaman): create an interface for chaincfg.params + Net: <cTestNetParams, + MilliSat: &testMillisat24BTC, + Timestamp: time.Unix(1496314658, 0), + PaymentHash: &testPaymentHash, + DescriptionHash: &testDescriptionHash, + Destination: testPubKey, + } + }, + }, + { + // Decode a litecoin mainnet invoice + encodedInvoice: "lnltc241pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66859t2d55efrxdlgqg9hdqskfstdmyssdw4fjc8qdl522ct885pqk7acn2aczh0jeht0xhuhnkmm3h0qsrxedlwm9x86787zzn4qwwwcpjkl3t2", + valid: true, + decodedInvoice: func() *Invoice { + return &Invoice{ + Net: <cMainNetParams, + MilliSat: &testMillisat24BTC, + Timestamp: time.Unix(1496314658, 0), + PaymentHash: &testPaymentHash, + DescriptionHash: &testDescriptionHash, + Destination: testPubKey, + } + }, + }, } for i, test := range tests { - invoice, err := Decode(test.encodedInvoice) + var decodedInvoice *Invoice + net := &chaincfg.MainNetParams + if test.decodedInvoice != nil { + decodedInvoice = test.decodedInvoice() + net = decodedInvoice.Net + } + + invoice, err := Decode(test.encodedInvoice, net) if (err == nil) != test.valid { t.Errorf("Decoding test %d failed: %v", i, err) return @@ -535,11 +605,6 @@ func TestDecodeEncode(t *testing.T) { continue } - var decodedInvoice *Invoice - if test.decodedInvoice != nil { - decodedInvoice = test.decodedInvoice() - } - if test.beforeEncoding != nil { test.beforeEncoding(decodedInvoice) } @@ -621,6 +686,30 @@ func TestNewInvoice(t *testing.T) { valid: true, encodedInvoice: "lnbc20m1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsfpp3qjmp7lwpagxun9pygexvgpjdc4jdj85fr9yq20q82gphp2nflc7jtzrcazrra7wwgzxqc8u7754cdlpfrmccae92qgzqvzq2ps8pqqqqqqpqqqqq9qqqvpeuqafqxu92d8lr6fvg0r5gv0heeeqgcrqlnm6jhphu9y00rrhy4grqszsvpcgpy9qqqqqqgqqqqq7qqzqj9n4evl6mr5aj9f58zp6fyjzup6ywn3x6sk8akg5v4tgn2q8g4fhx05wf6juaxu9760yp46454gpg5mtzgerlzezqcqvjnhjh8z3g2qqdhhwkj", }, + { + // Create a litecoin testnet invoice + newInvoice: func() (*Invoice, error) { + return NewInvoice(<cTestNetParams, + testPaymentHash, time.Unix(1496314658, 0), + Amount(testMillisat24BTC), + DescriptionHash(testDescriptionHash), + Destination(testPubKey)) + }, + valid: true, + encodedInvoice: "lntltc241pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66m2eq2fx9uctzkmj30meaghyskkgsd6geap5qg9j2ae444z24a4p8xg3a6g73p8l7d689vtrlgzj0wyx2h6atq8dfty7wmkt4frx9g9sp730h5a", + }, + { + // Create a litecoin mainnet invoice + newInvoice: func() (*Invoice, error) { + return NewInvoice(<cMainNetParams, + testPaymentHash, time.Unix(1496314658, 0), + Amount(testMillisat24BTC), + DescriptionHash(testDescriptionHash), + Destination(testPubKey)) + }, + valid: true, + encodedInvoice: "lnltc241pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqsnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66859t2d55efrxdlgqg9hdqskfstdmyssdw4fjc8qdl522ct885pqk7acn2aczh0jeht0xhuhnkmm3h0qsrxedlwm9x86787zzn4qwwwcpjkl3t2", + }, } for i, test := range tests { @@ -689,11 +778,7 @@ func compareInvoices(expected, actual *Invoice) error { expected.FallbackAddr, actual.FallbackAddr) } - if err := compareRoutingInfos(expected.RoutingInfo, actual.RoutingInfo); err != nil { - return err - } - - return nil + return compareRoutingInfos(expected.RoutingInfo, actual.RoutingInfo) } func comparePubkeys(a, b *btcec.PublicKey) bool { From 56c01ebcfe226839900f7cd55defc37e0f7954bf Mon Sep 17 00:00:00 2001 From: Daniel McNally Date: Mon, 19 Feb 2018 10:20:54 -0500 Subject: [PATCH 3/3] rpcserver: passing active net to zpay32.Decode --- rpcserver.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/rpcserver.go b/rpcserver.go index ac74c932..2a864419 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -1693,7 +1693,8 @@ func (r *rpcServer) SendPayment(paymentStream lnrpc.Lightning_SendPaymentServer) // attempt to decode it, populating the // payment accordingly. if nextPayment.PaymentRequest != "" { - payReq, err := zpay32.Decode(nextPayment.PaymentRequest) + payReq, err := zpay32.Decode(nextPayment.PaymentRequest, + activeNetParams.Params) if err != nil { select { case errChan <- err: @@ -1882,7 +1883,8 @@ func (r *rpcServer) SendPaymentSync(ctx context.Context, // If the proto request has an encoded payment request, then we we'll // use that solely to dispatch the payment. if nextPayment.PaymentRequest != "" { - payReq, err := zpay32.Decode(nextPayment.PaymentRequest) + payReq, err := zpay32.Decode(nextPayment.PaymentRequest, + activeNetParams.Params) if err != nil { return nil, err } @@ -2150,7 +2152,7 @@ func (r *rpcServer) AddInvoice(ctx context.Context, // createRPCInvoice creates an *lnrpc.Invoice from the *channeldb.Invoice. func createRPCInvoice(invoice *channeldb.Invoice) (*lnrpc.Invoice, error) { paymentRequest := string(invoice.PaymentRequest) - decoded, err := zpay32.Decode(paymentRequest) + decoded, err := zpay32.Decode(paymentRequest, activeNetParams.Params) if err != nil { return nil, fmt.Errorf("unable to decode payment request: %v", err) @@ -2978,7 +2980,7 @@ func (r *rpcServer) DecodePayReq(ctx context.Context, // Fist we'll attempt to decode the payment request string, if the // request is invalid or the checksum doesn't match, then we'll exit // here with an error. - payReq, err := zpay32.Decode(req.PayReq) + payReq, err := zpay32.Decode(req.PayReq, activeNetParams.Params) if err != nil { return nil, err }