Different error when trying to pay an invoice from another network

[jtimon]

When trying to pay an invoice from another network, I get the error message 'Could not find a route' (code 205).
That's not bad, it has to fail trying to find a route indeed.
But I think it would be nicer if it returned an error explicitly pointing out that the invoice to pay doesn't correspond with the network/chain this node is operating with.

The code I have doesn't seem to be working though, so any advice on this is very welcomed, assuming of course the concept makes sense in the first place.

I'm testing this with jtimon/multi-ln-demo#4
This is where I check the error: https://github.com/jtimon/multi-ln-demo/blob/master/multiln/bin/demo.py#L114

I don't expect people to run that, but I'm basically using the method pay() from contrib/pylightning.
I'm confused because pay seems to be a plugin and not have the normal cmd argument with which to call get_chainparams().

Again, any tips welcomed.

[ZmnSCPxj]

In theory, there may exist some node that bridges different networks and (if the networks use different assets) offer cross-asset swaps, so we cannot rule out the possibility that we can, in fact, pay the invoice of another network from our own network.

Though I suppose in a practical sense, since it is not practically doable yet, we could indeed just do this check.

pay currently uses the bolt11 library directly to decode the invoice, maybe it should use decodepay command instead? What do you think @cdecker @niftynei @rustyrussell ?

[jtimon]

In theory, there may exist some node that bridges different networks and...

Yeah, my goal is actually to code a demo for multi-chain payments.
I was looking for a check like this to modify it for my purposes, but since it wasn't there, I thought it would be a good way to start doing something and to get to know the code better.

The idea for the first demo is quite simple:

1. allow users to configure "dealer nodes" that manage both networks (so they're actually 1 agent with 2 nodes, one per chain)
2. when a node in chain A tries to pay an invoice in chain B, it checks its list of "dealer nodes" before returning this error, if there's one dealing A -> B, it sends that node the invoice in chain B, which will return an invoice in chain A for the paying node to pay. The payment secret could be shared so that Bob only pays the invoice in chain B if Alice pays the invoice in chain A.

EDIT: this is kind of similar to "Trampoline Payments" from the little a know about them.
Of course there's many more possibilities, but I wanted to start with something simple as a proof of concept.

pay currently uses the bolt11 library directly to decode the invoice, maybe it should use decodepay command instead? What do you think @cdecker @niftynei @rustyrussell ?

Thanks, I guess that's another possibility. I was thinking on just doing the same check there if I can somehow call get_blockchains from there.
But I'm going to try this to see if I can make it work. I mean, if people think the check or calling decodepay from pay instead is not a good idea for whatever reason, it's totally find to just keep it on my local branch for my proof of concept. But this feedback already helps, thanks.

[ZmnSCPxj]

Technically, we are doing this wrong. What is encoded on the struct bolt11 is the network (struct chainparams *chain), but note that network does not mean token. BOLT11 itself encodes the "currency code". In principle you could pay someone asking for BTC using LBTC tokens, for example --- if there is a good reason for people not to accept LBTC tokens, that implies a lack of trust in the idea that an LBTC is indeed claimable for BTC 100% of the time. But note that a network might support multiple token types too, in which case our chainparams structure will need some kind of modification.

A counterargument to doing this in decodepay is that decodepay should be token-agnostic --- you are just asking to decode an invoice, not asking for judgment on whether this invoice is something you can pay (e.g. decodepay does not check your outgoing capacity and error if you cannot pay the decoded invoice).

Perhaps a better way would be to expose the supported currencies by adding a supported_currencies array to getinfo instead. In current code it would contain only a 1-entry array of the only currency supported (bc, tb for test bitcoin, bcrt for regtest bitcoin). Then your proof-of-concept can extend this array for all exchange types supported. Though how to route...

[rustyrussell]

We should still allow decodepay to decode the invoice, even if for a different network. However, for simplicity, we should simply reject the invoice if the currency does not match our current one.

@ZmnSCPxj makes a valid point, but we'd need to revisit this entirely if we were to allow such transparent cross-chain swaps.

[rustyrussell]

ie. the change is a one-liner in json_pay in plugins/pay.c:

if (b11->chain != chainparams)
        return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "Invoice is for another network %s", b11->chain->network_name);

[jtimon]

Well, the confidence will be reflected in the BTC:LBTC exchange rate. Perfect confidence would mean I guess 1:1, but it could deviate from that for other reasons. But I think that's out of scope for this. I think invoices in liquid should use some other bip173 prefix simply because it is another chain, otherwise the chain would need

Yes, also there's chains like those created with elements that can have multiple assets.
One option would be for asset ids to be integrated in the lightning protocol somehow.
The software currently assumes a single network/chain and a single asset for that chain.
I think the bip173 prefix should change per chain but a 256 bit asset id doesn't seem something you would add to the bip173 prefix. Perhaps it's fine.
Again, this seems out of scope for this PR.
The goal of this is simply not to try to find routes if we know the bip173 prefix is for another chain different from the one supported by the running node.

A counterargument to doing this in decodepay is that decodepay should be token-agnostic --- you are just asking to decode an invoice, not asking for judgment on whether this invoice is something you can pay (e.g. decodepay does not check your outgoing capacity and error if you cannot pay the decoded invoice).

That make sense. I don't know what place would be best to check whether the invoice is for the chain supported by this node or not, before trying to route anything.

...(bc, tb for test bitcoin, bcrt for regtest bitcoin)...

or other prefixes for other chains, they're not currencies, they're chains, and the prefixes are already in: https://github.com/ElementsProject/lightning/blob/master/bitcoin/chainparams.c

I mean, if there's any other PR on supporting multiple currencies, even if it is within a single chain, by all means ping me for review and testing.
But this is not it. This is really just acknowledging more explicitly that the current software only supports a single chain simultaneously (although the same binary can be run in parallel for multiple chains) and save the costs of trying to route something that can't possibly route with this software by returning an error faster.

[jtimon]

Sorry, @rustyrussell for some reason I didn't see your comments.
Yeah, that one-liner is indeed what I wanted, but I didn't realize I could directly use "chainparams" there, thanks.

Moved the check from json_decodepay in invoice.c to plugins/pay.c as originally intended and suggested by rusty.

[cdecker]

Notice that b11->chain may be NULL if we don't recognize the network name.

lightning/bitcoin/chainparams.c

Lines 233 to 241 in 0985c6e

	const struct chainparams *chainparams_by_bip173(const char *bip173_name)
	{
	for (size_t i = 0; i < ARRAY_SIZE(networks); i++) {
	if (streq(bip173_name, networks[i].bip173_name)) {
	return &networks[i];
	}
	}
	return NULL;
	}

As is a user could be tricked into parsing an invoice for an unknown network and then crash. Please use the bip173 prefix extracted from the invoice instead.

[jtimon]

Oh, right, that's true.
But that would require parsing the bolt11 twice and if the chain is unknown, there's really nothing else to do.
I'm going to simply check if it's null a return an unknown network error if so, that seems simpler.

[jtimon]

Updated fixing @cdecker 's nit, but in a different way to what he suggested. Hopefully this is fine.
To test this only with a bitcoind, I think it would be nice to have something like bitcoin/bitcoin#17037 merged on bitcoin core.
If it gets merged, I will also PR something like jtimon@3333a5f here unless you guys think it is a bad idea.

[cdecker]

ACK 5008c9b

[darosior]

ACK 5008c9b

To test this only with a bitcoind, I think it would be nice to have something like bitcoin/bitcoin#17037 merged on bitcoin core.

Or bitcoin/bitcoin#17032 :-)

[niftynei]

spelling correction only

[jtimon]

Fixed spelling error.

[niftynei]

ACK 0a69c56