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

dual-funding -> use tx-abort now #5767

Merged
merged 6 commits into from
Feb 5, 2023

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Dec 1, 2022

Built on #5670. Starts at fa30d80d5ad1151f5374e72158038f02b46dc33b

Kind of a fun one. We've got a fancy new message (tx-abort) which will signal we're ending a channel open negotiation.

What's fun about this is that now when we abort/end an open channel negotiation (v2s only), we don't disconnect from the peer. Instead the subdaemon for the open dies. If the peer retries, we restart the daemon.

What's nice about this is that we now keep the connection open for any other channels that we may or may not currently have with the peer we're talking to.

@rustyrussell
Copy link
Contributor

OK, #5670 merged, so I'm rebasing this on master...

@rustyrussell
Copy link
Contributor

Ack dc5b692

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK dc5b692

Dropping the connection is bad behavior on an openchannel failure,
especially given that there might be other channels currently connected.

We should maintain the connection but close out the dualopend
daemon for that attempt.

This test partially documents the behaivor, but fails

Changelog-None
Code is identical to `channel_fail_transient`
When a channel open fails, we use tx-abort instead of warning/error.

This means that the peer won't disconnect! And instead when a new
message arrives, we'll need to rebuild the dualopend subd (if missing).

Makes opens a bit easer to retry (no reconnect needed), as well as keeps
the connection alive for other channels we may have with that peer.

Changelog-Changed: Experimental-Dual-Fund: open failures don't disconnect, but instead fail the opening process
Wait until we get a tx-abort back to terminate the process.

Nota Bene: this can cause RPC calls to hang if the peer never
responds back with tx-abort.

Note that we also have to re-route how open-abort + negotiation_failed
handle failures, as open_abort no longer closes the process
automagically.
make the number of blocks mined father away from the cltv timeout

from borked/flakey test run:

	lightningd-3 2023-01-26T21:45:19.261Z DEBUG   022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-channeld-chan#2: billboard perm: Received error channel 27a4a4dd880e86
	1e390517de3e786a237c5ad1f00faab277382664e76b5c3870: Fulfilled HTLC 0 SENT_REMOVE_COMMIT cltv 116 hit deadline
@rustyrussell
Copy link
Contributor

Fix test flake and a local mocks build error on latest master.

It's possible that l2 hasn't completely processed the connection yet:

```
        # create l2->l1 channel.
        l2.fundwallet(amount_sat * 5)
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
>       l2.rpc.fundchannel(l1.info['id'], amount_sat * 3)

tests/test_pay.py:3974:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
contrib/pyln-client/pyln/client/lightning.py:833: in fundchannel
    return self.call("fundchannel", payload)
contrib/pyln-testing/pyln/testing/utils.py:706: in call
    res = LightningRpc.call(self, method, payload, cmdprefix, filter)
...
E           pyln.client.lightning.RpcError: RPC call failed: method: fundchannel, payload: {'id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'amount': 3000000, 'announce': True}, error: {'code': 400, 'message': 'Unable to connect, no address known for peer', 'data': {'id': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', 'method': 'connect'}}

contrib/pyln-client/pyln/client/lightning.py:422: RpcError
```

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Contributor

Ack c787bba

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK c787bba

@vincenzopalazzo vincenzopalazzo merged commit 28b31c1 into ElementsProject:master Feb 5, 2023
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