Lnd: 🐞 Settting Litecoin fallback address

[juscamarena]

Background

Describe your issue here.

Your environment

• version of lnd
• which operating system (uname -a on *Nix)
• version of btcd, bitcoind, or other backend
• any other relevant environment details

Steps to reproduce

Tell us how to reproduce this issue. Please provide stacktraces and links to code in question.

Attach a litecoin address as a fallback address when creating a litecoin lightning invoice

Expected behaviour

Being able to set litecoin addresses as fallback when chain is set to litecoin
Tell us what should happen

Actual behaviour

Bitcoin addresses can be set as fallback for litecoin invoices and litecoin addresses fail validation.

Code seems to only do validation for bitcoin addresses.

lncli --chain litecoin addinvoice 1 --fallback_addr 3CowtmRNBiJqpdB6w1Y2h2dogHp6NYddaQ
{
"pay_req": "lnltc10n1pd66vespp5hvq6lp76hdvdsztr2kau843akag6wvslcasss0lwxev87tz70f6sdqqcqzjqfppj08uwu32aglvha7aeu03smtag96smw44gw9qy9njgklwl3av0nssdu7y5vpkjd4xw7adkwm9ugpr2gwmts0gz7dphdyxcylsjffdz735vxvwljjxq9stel4ajheudgj4af6s7daqq4gmr6n",
}

lncli --chain litecoin addinvoice 1 --fallback_addr ltc1q60v0a8tnwwv48c8asflr8a8dt6qax8qwxxe47t
[lncli] rpc error: code = Unknown desc = invalid fallback address: decoded address is of unknown format

[githubsands]

Can you guys assign me to this issue?

[halseth]

@githubsands Assigned! 🕺

(not github assigned, but in spirit)

[mlerner]

@halseth: It looks like this isn't currently being worked on. I could pick this up.

[Roasbeef]

@mlerner it's all yours!

[mlerner]

After looking into this, the problem appears to be that the fallback address is always decoded with the btcutil.DecodeAddress method, regardless of which chain is active: https://github.com/lightningnetwork/lnd/blob/f6ea91af713546174bf04a79302116aa455d89c8/rpcserver.go#L2484

btcutil.DecodeAddress uses chaincfg.IsBech32SegwitPrefix to parse bech32 addresses and this method uses the bech32SegwitPrefixes map. Because this map in btcd only contains configuration for bitcoin bech32 prefixes, bech32 Litecoin addresses are parsed unsuccessfully.

Litecoin addresses would be recognized by https://github.com/ltcsuite/ltcutil/blob/master/address.go#L145). As a side note, chaincfg.Params is duplicated in both btcd and ltcd, and there is currently some manipulation that happens to make the ltcd chaincfg.Params work in lnd.

[wpaulino]

@mlerner cool! Looks like we can just check what the active chain is and use the correct DecodeAddress method.

[mlerner]

@wpaulino: PR incoming!

[ecurrencyhodler]

@mlerner Did you ever submit a PR? This is a larger problem than just with fallbackaddresses. lnd does not recognize LTC address formats when sending an on-chain address as well.

[dannypaz]

This issue also related to https://github.com/lightningnetwork/lnd/issues/2085

[dannypaz]

We've actually investigated this issue and had to make a patch to btcutil to use the activeNetParams instead of the default params that are set by btcd/chaincfg.

You can find the patch here: https://github.com/sparkswap/btcutil/blob/bfad32d2f123951fd51eb9d54622aa12a87192be/address.go#L199-L205

THE ISSUE
The issue occurs because btcutil is using btc chain settings which is initialized when the btcd/chaincfg package is referenced

https://github.com/btcsuite/btcd/blob/master/chaincfg/params.go#L696-L702

DecodeAddress in btcutil is called to check the address type in sendcoins and addinvoice. This method makes calls to chaincfg which uses BTC configuration instead of defaultNet which would be litecoin.

isP2PKH := chaincfg.IsPubKeyHashAddrID(netID)
isP2SH := chaincfg.IsScriptHashAddrID(netID)

It looks like btcutil should be modified for this change (since they use defaultNet in other areas of DecodeAddress), but would like validation from the Lightning Labs team if this is the correct way to resolve the issue

[mlerner]

@dannypaz @ecurrencyhodler: you can go ahead and submit your patch for this issue if you would like. I haven't finished up mine yet.

[dannypaz]

@juscamarena can you add a note that this is affecting mainnet

[dannypaz]

@wpaulino @Roasbeef is the correct fix here to updated btcutil? (as long as the PR is made generic and doesn't reference litecoin)

[dannypaz]

As a proactive measure, I've opened up a PR for a proposed solution here: https://github.com/btcsuite/btcutil/pull/133

[dannypaz]

https://github.com/btcsuite/btcutil/pull/133 has been approved but is waiting to merge. Once that PR is in, we can update LND's version of btcutil and this issue will be resolved