New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] multi: add new draft sphinx send mode for spontaneous payments #2455

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@Roasbeef
Copy link
Member

Roasbeef commented Jan 11, 2019

In this commit, we add new feature to lncli sendpayment (and the RPC): the ability to send a payment to a destination without first needing to have an invoice. This allows users to start exploring a new set of use cases that benefits from this type of spontaneous payment. The payment can also carry additional application-specific data such as an account ID, API call, etc. This new feature requires the new EOB (extra onion blob parsing) implemented here. The coolest part about this new feature is that it can be used today in the wild as long as both nodes are updated to this branch!

An example run on the cli:

⛰   lncli --network=simnet sendpayment --dest=02788242941915ed5a101511b8dfeb6db81e0fcd7546f6a55ef4dedf590a7d7ff4 --final_cltv_delta=144 --amt=100 --sphinx
{
    "payment_error": "",
    "payment_preimage": "0b1661fb77071fa5efd2217e380c93068bc3dc6e40e302673ff6373ecd8997b5",
    "payment_route": {
        "total_time_lock": 3299,
        "total_amt": 100,
        "hops": [
            {
                "chan_id": 3610796185681920,
                "chan_capacity": 15000000,
                "amt_to_forward": 100,
                "expiry": 3299,
                "amt_to_forward_msat": 100000,
                "pub_key": "02788242941915ed5a101511b8dfeb6db81e0fcd7546f6a55ef4dedf590a7d7ff4"
            }
        ],
        "total_amt_msat": 100000
    }
}

NOTE: This is only a draft implementation and while it works today on mainnet out of the box (if both sides are upgraded) much this will likely change. Notably the modifications in the link are very hacky and will likely be refactored in preparation of AMP and the like.

@Roasbeef Roasbeef changed the title multi: add new draft sphinx send mode for spontaneous payments [WIP] multi: add new draft sphinx send mode for spontaneous payments Jan 11, 2019

@ghanavati27

This comment has been minimized.

Copy link

ghanavati27 commented Jan 12, 2019

This is the best feature that we have in lightning !!
Thanks a lot @Roasbeef

@junderw

This comment has been minimized.

Copy link
Contributor

junderw commented Jan 12, 2019

Looks very VERY cool.

  • I would suggest disabling this by default, and wallets like Zap etc. that link with LND can then silently enable them, or offer to enable them.
  • Also, I guess there would be some way to ping the node and check and make sure it's still currently supporting the feature.
  • Another thing I would recommend is adding an optional password/blinding key of some sort. That way you could make a "Sphinx Address" that is basically your node key, IP, and password all bech32 encoded or something, and anyone who sees that address can send payments willy nilly, and anyone who doesn't know the password (hasn't seen the "address") won't be able to send.
  • Bonus points, the "Sphinx Address" format could encode all those invoice ID etc. things you spoke of.

This would be bullish for exchange adoption.

@junderw

This comment has been minimized.

Copy link
Contributor

junderw commented Jan 12, 2019

Imagine we have 3 users on our exchange:

Make 3 "Sphinx Addresses":

  1. bech32(nodekey = 02... || ip:port = x.x.x.x:yyyy || password = acbdefg... || accountID = 0)
  2. bech32(nodekey = 02... || ip:port = x.x.x.x:yyyy || password = acbdefg... || accountID = 1)
  3. bech32(nodekey = 02... || ip:port = x.x.x.x:yyyy || password = acbdefg... || accountID = 2)

Then we could just fetch our received payment history, check the accountID, and attribute the amount to the user's balance on our DB.

Super awesome. Can't wait. I highly recommend standardizing an "address" format though.

)

needUpdate = true
continue
// An empty EOB means they didn't send su any special

This comment has been minimized.

@JimmyMow
@molxyz

This comment has been minimized.

Copy link

molxyz commented Jan 14, 2019

I'm running two nodes on testnet and spamming myself with tiny amounts (sphinx send ftw!) ... So I'm wondering if there's a way not to allow these spamming txs if the receiving end does not wish to be spammed?

Second question: Before making a Sphinx send should there be a question like You are about to send X amount without an invoice. Are you sure you want to do this? [yes/no]

@prusnak

This comment has been minimized.

Copy link

prusnak commented Jan 15, 2019

So I'm wondering if there's a way not to allow these spamming txs if the receiving end does not wish to be spammed?

Is there a reason why I should not want these payments although very small? Does this somehow corrupt my channel's fitness? If possible (D)DoS is the reason, there might be a better way how to perform it on a LN node.

@junderw

This comment has been minimized.

Copy link
Contributor

junderw commented Jan 15, 2019

@Roasbeef are you writing up a BOLT for this? I'd like to collaborate on some functionality that exchanges would like. Starting with the points I made above.

@molxyz

This comment has been minimized.

Copy link

molxyz commented Jan 15, 2019

@prusnak Hi,

Is there a reason why I should not want these payments although very small? Does this somehow corrupt my channel's fitness? If possible (D)DoS is the reason, there might be a better way how to perform it on a LN node.

In the past we tested this on testnet and my node's constant stream of sending payments for several hours did crash another tester's node. I haven't tested this with Sphinx send yet, but I'm planning to test it, you can help too if you want to. (I've been only spamming myself with about 30 txs at a time. To test crashing maybe should do a few thousands of txs or more overnight... lol)

However, even with no crashing I am just wondering if someone might not like it (just wondering never know), so for caution I'm just raising the question. As for me, I would not mind to receive streams of satoshis though, the more the merrier! Haha ... :D

@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Jan 15, 2019

. So I'm wondering if there's a way not to allow these spamming txs if the receiving end does not wish to be spammed?

Nodes can reject this at will. You can already spam a node by sending them an HTLC with an unknown payment hash. If someone is spamming you by giving you free money then well....🤑

are you writing up a BOLT for this? I'd like to collaborate on some functionality that exchanges would like. Starting with the points I made above.

The main point of synchronization would be finalizing the EOB format (see the PR linked in the description). From there, we'd reserve a type, and then star to specify the behavior/encoding around that type.

@molxyz

This comment has been minimized.

Copy link

molxyz commented Jan 16, 2019

@Roasbeef First crash report!
My node crashed when I tried to Sphinx send to a node that didn't have this PR compiled. This node is running on Tor: https://paste.ee/p/rjlQZ#i9Z26p8QL9f5ZSb6MCdy0AAusFKRL8nl

Strange that my node didn't crash before when I tried the same with a non-sphinx-send node that wasn't running on Tor.

EDIT: The other node got the PR compiled after, then tried to Sphinx send to my node (we both didn't know my node crashed at the time), so he tried a sendpayment and his node crashed too, with same error.

@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Jan 16, 2019

Yeah that's a known bug due to a change in the way I handle pubkeys, easy fix, remember that this PR isn't final at all.

Roasbeef added some commits Jan 11, 2019

router: convert Route.ToHopPayloads() to Route.ToSphinxPath()
In this commit, we update the process that we use to generate a sphinx
packet to send our onion routed HTLC. Due to recent changes in the
`sphinx` package we use, we now need to use a new PaymentPath struct. As
a result, it no longer makes sense to split up the nodes in a route and
their per hop paylods as they're now in the same struct. All tests have
been updated accordingly.

@Roasbeef Roasbeef force-pushed the Roasbeef:new-eob-sphinx-send branch from 5300038 to 552f239 Jan 18, 2019

@Roasbeef

This comment has been minimized.

Copy link
Member

Roasbeef commented Jan 18, 2019

Latest diff patches that issue.

@molxyz

This comment has been minimized.

Copy link

molxyz commented Jan 20, 2019

@Roasbeef It's fixed, Thank you!

Suggestion: Could we have an option to set the minimum amount that we would accept? Something like minchansize=. Maybe minsphinxsize=, or minautopayaccept=, or minsphinxaccept=, etc.. What do you think?

EDIT: And the default is 1 sat like it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment