ACINQ / eclair-mobile

An Android wallet for the Lightning Network
Apache License 2.0
258 stars 43 forks source link

LND compatibility issue? #256

Closed C-Otto closed 3 years ago

C-Otto commented 3 years ago

I have a channel to my LND node, and this now gives "incompatible features, disconnecting" when I look at the logs. According to LND it's just a "connection reset by peer". The channel was opened about a year ago, and I keep LND mostly up to date. The app auto-updates according to Google Play, I think.

Which features are incompatible? When did this happen? Can I work around this to re-enable the channel?

In my app logs:

using features=0000001000000010
peer is using features=000000101001001010100001
t-bast commented 3 years ago

Your lnd node requires the option_staticremotekey feature, which eclair-mobile doesn't support (because some bech32 work needs to be done before it can happen). I believe you should set it to optional on your lnd node. Closing this issue as we can't do anything on the eclair-mobile side to fix this.

C-Otto commented 3 years ago

@t-bast thanks, could you please point me to the issue where "some bech32 work" is further defined, so that I can track the progress?

C-Otto commented 3 years ago

See https://github.com/lightningnetwork/lnd/pull/4751

t-bast commented 3 years ago

Sure, it's linked to that PR: https://github.com/ACINQ/eclair/pull/1170 The painful part is how we handle migration of existing wallets: there's no perfect solution there.

Note that eclair-mobile doesn't really need static_remotekey: we had found a somewhat clever way of deriving the keys that achieved the same results as static_remotekey before it was introduced to the specification. This is why it was not a priority for us to add this to eclair-mobile.

Roasbeef commented 3 years ago

Why's bech32 required for static remote key @t-bast?

Roasbeef commented 3 years ago

FWIW, the next release of lnd will start to require it, given it's been so long since it was introduced, and it really improves security for end users since in failure/loss scenarios since coins just go directly to the wallet.

Roasbeef commented 3 years ago

Also how are y'all planning on rolling out support for anchor outputs since it implicitly requires static key?

C-Otto commented 3 years ago

I'm running "the next release", and had to force-close my channel because of this :/

t-bast commented 3 years ago

I'm running "the next release", and had to force-close my channel because of this :/

Why can't you simply set this feature to optional? You should be able to configure your node features to your pleasure, regardless of lnd's default settings, right?

As long as you don't use anchor outputs, if you don't lose/corrupt your state backup (which should not happen if you're a serious enough node) you don't really need static_remotekey.

it really improves security

I don't think it improves security. It only improves recovery in case you lose/break your backup data (and it saves on on-chain fees since it goes directly to your wallet). TBH that's something that should be extremely rare (we never had a user running a server node complaining about that). Anchor outputs improves security though, so that one is a good argument to upgrade (but it will take a while before it comes to eclair-mobile, especially since some fee scenarios are still unclear).

Why's bech32 required for static remote key @t-bast?

That's a very good question, IIRC it was linked to an electrum limitation. I'll dig into it and will report back.

t-bast commented 3 years ago

Here is the reason it's linked to bech32 work.

Currently eclair-mobile uses a p2sh-of-p2wpkh wallet instead of bech32 (because it was released before bech32 and we didn't upgrade it). To use option_staticremotekey that needs to change, p2sh-of-p2wpkh won't work (unless I'm missing something).

The upgrade story to bech32 is painful. There are multiple solutions, but none of those hit the sweet spot where it's not cumbersome for users and not too many code changes. For example if we internally have two wallets (one bech32 and one p2sh-of-p2wpkh), in case users need to restore they'll need to restore two wallets which is a horrible UX. If we fully migrate to bech32, what do we do with previous users/channels? It's going to be a painful update so we've currently postponed it.

Roasbeef commented 3 years ago

if you don't lose/corrupt your state backup (which should not happen if you're a serious enough node) you don't really need static_remotekey.

That's a pretty big if...I think you underestimate the wide range of users that are running nodes given how easy projects like the rasp pi blitz have made it.

I don't think it improves security. It only improves recovery in case you lose/break your backup data (and it saves on on-chain fees since it goes directly to your wallet). TBH that's something that should be extremely rare (we never had a user running a server node complaining about that).

It definitely does! We added it to remove the "get random point from peer" dependency when a user loses data. I think you underestimate how often this actually happens in practice due to your sampling bias, in that I assume most of your users are running the mobile client which AFAIK uses cloud storage to sync the user's state.

We've processed likely hundreds (prob an underestimate) (just a quick search, likely missing a lot) of instances where a user's rasp-pi blows up for w/e reason, requiring them to use our "SCB" mechanism to get their funds back. All the instances we've seen has caused us to continually harden the logic to make sure users are able to recover their funds. It's the case empirically, that any channels that aren't created with the static remote key is less safe than those using the safety feature.

Roasbeef commented 3 years ago

Currently eclair-mobile uses a p2sh-of-p2wpkh wallet instead of bech32 (because it was released before bech32 and we didn't upgrade it). To use option_staticremotekey that needs to change, p2sh-of-p2wpkh won't work (unless I'm missing something).

Not sure how y'all's wallet works internally, but for lnd, we actually just give it a raw key, then manually handle sweeping that just like any other output (CSV delay, HTLCs, etc).

For example if we internally have two wallets (one bech32 and one p2sh-of-p2wpkh), in case users need to restore they'll need to restore two wallets which is a horrible UX. If we fully migrate to bech32, what do we do with previous users/channels? It's going to be a painful update so we've currently postponed it.

Again just speaking from a perspective of how we do things in lnd, but we also support recovery from multiple accounts/bip-44-purposes, so a user puts in a single seed, and on the wallet backend we scan for all those scopes (native segwit, nested, legacy, etc, etc). We've even added some over time, and all users needed to do was upgrade to be able to start using the new scope. But of course, I understand that y'all have have a different set up.

t-bast commented 3 years ago

I think you underestimate the wide range of users that are running nodes given how easy projects like the rasp pi blitz have made it.

That's true, I can understand that nodes on raspberry pis may have this kind of issue a lot... TBH I've always thought running nodes on things like raspberry pis is a very bad idea and asking for trouble, but well that's just my unpopular opinion (and if users want it, I'm in no position to judge against it).

Not sure how y'all's wallet works internally, but for lnd, we actually just give it a raw key, then manually handle sweeping that just like any other output (CSV delay, HTLCs, etc).

We may go in that direction in the future, but we wanted to avoid as much as possible re-implementing wallet software that already existed, that's why we try to use Electrum on mobile and Bitcoin Core on servers as much as possible. It definitely has limits and has been frustrating at times, but we simply didn't have enough resources to properly develop and maintain these additional components.

cfromknecht commented 3 years ago

In the original protocol (tweaked), how does eclair-mobile generate the payment_basepoint? There shouldn't be any change between that and the non-tweaked version. I can see how it might be an issue if one wants to provide a payment_basepoint controlled by an onchain wallet, but idt it's at all required for static_remote_key. All of the sweeping logic for a non wallet-controlled pubkey should already be implemented from the original protocol as well IIUC?