Skip to content
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

chainparams: Hardcode 2 new custom regtests #3337

Closed
wants to merge 1 commit into from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Dec 12, 2019

This adds 5 new regtest-like chains to chainparams.

This could be useful for testing any experimental use case involving lightning and several chains/networks.
For example, I think it could be useful to more conveniently test multi-chain use cases related to #3260

This depends on bitcoin/bitcoin#17037 , so I completely understand if this is not wanted because of that, or not wanted unless that gets merged or until that happens if it ever happens. For the alleged "convenience" is nullified if it's not compatible with bitcoin core releases, which is an inconvenience.
I can close or re-open as asked.

It can also be a different number other than 5 chains (6 including regular regtest), of course.
I guess the right number of regtest chains depend on the test cases.

I will try to describe one, please don't judge the PR only by the example only (nor viceversa).

If I understood #3260 correctly, which I probably didn't at my first try, my most wanted test case looks something like this:

Alice --chain_1_ln--> (TBCH1(Bob) -- htlc_accepted[hook] -->plugin(Bob) --sendonion[rpc]--> TBCH2(Bob)) --chain_2_ln--> Carol

We could classify this test case as in the "trampoline cross-chain payments" class.
And let's also define bob as acting as a "trampoline cross-chain hop" in this payment.

A simplified notation could be:

A1 -> B1_2 -> C2

For this same use case to need 5 chains, we would need something like

A1 -> B1_2 -> C2_3 -> D3_4 -> E4_5 -> F5

3 chains:

A1 -> B1_2 -> C2_3 -> D3

3 chains already means 2 cross-chain trampoline hops, so it should be plenty to demonstrate the use case, right?
Being trampoline payments, there could be any number of hops within the same chain.

So why 5?
I got tired of searching names that corresponded to the alphabetical symbol:

Alice -> Bob -> Carol -> David -> Ezra -> Fiona

Shamelessly pinging @cdecker @bitonic-cjp and @t-bast because I think they can all answer more questions about my own example than myself.
And probably already thought of this very example, and better names for it than "trampoline cross-chain payments".
I just want to see it happen, and hopefully help making it happen.

Sorry, I hop the example test case explanation wasn't too long.

I tried to explain the same example with other words and without knowing about #3260 in https://lists.linuxfoundation.org/pipermail/lightning-dev/2019-November/002354.html

But, sorry again, one thing is the example, another one this simple PR.

@t-bast
Copy link

t-bast commented Dec 13, 2019

Using Trampoline nodes (simply the concept of a second onion inside the main payment onion) to switch chains is an interesting idea. I didn't have time to investigate it in depth but it feels like it should work with a few additional TLV records to give the proper instructions to the Trampoline/Switch nodes (which is probably what you did already).

The hard part would most likely be defining the right fees, mapping capacity between currencies, etc. And handling error cases correctly :)

@jtimon
Copy link
Contributor Author

jtimon commented Dec 15, 2019

Awesome, thanks!
I wrote more about that here: https://lists.linuxfoundation.org/pipermail/lightning-dev/2019-November/002354.html

But that was before I read #3260 , so I think some of the questions there have already been answered.
But I would definitely still appreciate any feedback there, specially yours. You know, since you basically wrote most my example (I just modified your own example for multi-chain).

But I understand the time restrictions, and slowing you down in regular trampoline implementation is the last thing I would want to do. On the contrary, I've been looking for eclair's code on trampoline payments (or, really any code on it) to see if I could help with anything and perhaps start using it in my own demo repo and playing with it with potential changes that could bring us closer to "multi-chain atomic payments", which is basically my main goal.
So, in that regard, if I can help with trampoline payments, please, let me know.
I haven't written java in years, but I still remember rewriting this patch for java is trivial. If the java lightning world is interested, I'm more than happy to write another PR for java or any other implementation.

Having said all that, let's please not mix my concrete use case with this concrete PR, like we did with #3100 . We can discuss that on the general mailing list.
So let's go back to the actual PR beyond my use case.

Is this use case enough for merging it?

I think it is, but I'm obviously biased.

Are there more use cases?

I think there are, and I think I kind of proved it.

Is N=5 the right number?

I don't know, I want at least 3. But if my use case works with N=3, it should work for N=5, so why not?

Why not N=27?

It takes too long to explain why the spanish alphabet would be the best to complete bip173_name for all of them.
Besides, let's remember we can always add more later, let's please focus on the complains about 5 being too many.

@cdecker
Copy link
Member

cdecker commented Dec 16, 2019

Thanks @jtimon for the pull request. I totally understand your desire to be
able to test across a multitude of chains, having encountered this myself in
the past. However I don't think that just blanket adding of 5 regtest networks
is actually useful:

  • The networks are exposed to users, and they might end up confusing
    people. If added the chainparams should be gated by #if DEVELOPER, so
    that only developers who opted into the sharp edges are exposed to these
    dummy networks.

  • When looking into this issue myself I found that we already have quite a
    number of networks defined (bitcoin, testnet, regtest, liquid,
    liquid-regtest, litecoin, signet, and litecoin-testnet). Out of
    these 8 networks that have parameters we already have 3 networks that are
    effectively regtest networks: regtest, liquid-regtest and
    signet. Two networks would already be sufficient to test atomic
    cross-chain swaps like you intend to do (e.g., regtest ->
    liquid-regtest and liquid-regtest -> regtest). Since the hops are
    encapsulated in the routing onion, there is no difference between jumping
    across 5 networks or just weaving in and out of the same two
    repeatedly. Hops are isolated since they can't really see what your next
    action is. So effectively what you want is already present here.

  • Regarding trampoline, I don't think that it requires any special attention
    when it comes to cross-chain scenarios, since it is just an opportunistic
    route-selection mechanism in the network, it is quite independent of
    cross-chain concerns. Gateways providing connectivity between networks is
    an interesting issue, but these can be tested in isolation, and don't
    require this increase in network configurations.

So for me this is a NACK.

@jtimon
Copy link
Contributor Author

jtimon commented Dec 17, 2019

If added the chainparams should be gated by #if DEVELOPER, so
that only developers who opted into the sharp edges are exposed to these
dummy networks.

Yeah, that makes sense, and it's easy to fix.
Non-dev users definitely don't care about this.

we already have 3 networks that are effectively regtest networks: regtest, liquid-regtest and signet

Yes, but signet doesn't work exactly the same, I would need some boilerplate since generate won't work directly and I would need some other signet, since I don't have the keys for the supported/official one so that I can sign my own blocks.

I think liquid-regtest should work, since afaik the block script is still simply OP_TRUE.
The problem with it is I need yet another binary, but admittedly this is not a big deal and I wanted to do it at some point anyway.

And regarding regtest, yeah, that's perfectly fine, of course, so in reality adding 5 we really have 6 regtest-like chains.

Two networks would already be sufficient to test atomic cross-chain swaps like you intend to do...

I don't think it is exactly the same, but I hadn't thought about this.
I guess I could do something like:

Alice(reg) -> Bob(reg/reg2) -> Carol(reg/reg2) -> David(reg/reg2) -> Ezra(reg/reg2) -> Fiona (reg2)

...instead of...

Alice(chain_1) -> Bob(chain_1/chain_2) -> Carol(chain_2/chain_3) -> David(chain_3/chain_4) -> Ezra(chain_4/chain_5) -> Fiona (chain_5)

...and it would be exactly the same.
Alice should still be forced to find the same path if I only open those channels

Or really just Alice(reg) -> Bob(reg/reg2) -> Carol(reg2) should demonstrate the thing.

I'll think more about this.
But I guess I could simply do with only one more, or just use elements and use liquid-regtest as you say.

Regarding your point about trampoline, I'm not sure I understand it.
How can alice create a cross-chain trampoline payment without somehow knowing that different trampoline nodes in different networks belong to the same gateway (are we using the same definition for gateway here?) and thus you can route through them to pay to and/or through other chains?

@jtimon
Copy link
Contributor Author

jtimon commented Dec 18, 2019

Thanks everyone again for the great feedback.
Even if this us likely to be rejected, I updated the code:

  • added macro: if DEVELOPER
  • Removed chain_1, regtest is already there
  • Removed chain_4 and chain_5, they were never needed, not even for 2 trampolines

chain_3 can also be removed for my use case. Because going from regtest to chain_2 and then back to regtest also 2 "cross-chain trampolines", as this can be generalized to N chain hops as @cdecker points out.

After that, only chain_2 would remain, and the only excuse not to remove it too, is not to need liquid-regtest.
But, again, until/if bitcoin/bitcoin#17037 gets merged chain_2 needs a custom binary too.

So...yeah, close it.

What about only adding chain_2 and only for devs?
That way I would still do N hops while not needing and extra binary.

Nah...yeah, close it.

I can't do it myself because I plan to keep using at least chain_2.

@jtimon jtimon changed the title chainparams: Hardcode 5 new custom regtests chainparams: Hardcode 2 new custom regtests Dec 18, 2019
jtimon added a commit to jtimon/multi-ln-demo that referenced this pull request Dec 18, 2019
jtimon added a commit to jtimon/multi-ln-demo that referenced this pull request Dec 18, 2019
@darosior
Copy link
Collaborator

Maybe we could allow to set custom chainparams using a file ? Like

lightningd --network custom_net.txt

@jtimon
Copy link
Contributor Author

jtimon commented Dec 20, 2019

Maybe we could allow to set custom chainparams using a file ?

That would definitely work for me too, but that would be more work and much more disruptive, since the chainparams couldn't be a const array anymore.

with regtest, now there's 3 regtest-like in total only using bitcoind
with elements, liquid-regtest gets it up to 4
@jtimon
Copy link
Contributor Author

jtimon commented Feb 25, 2020

I finally moved my demo from doing regtest -> liquid-regtest -> chain_3 to doing regtest -> liquid-regtest -> regtest as suggested.
Closing.

@jtimon jtimon closed this Feb 25, 2020
@jtimon jtimon deleted the pr-5-regtests branch February 25, 2020 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants