Skip to content

Zero fee htlc preparation: Part 2 - #6074

Merged
rustyrussell merged 18 commits into
ElementsProject:masterfrom
rustyrussell:zero-fee-htlc-prep-2
Apr 7, 2023
Merged

Zero fee htlc preparation: Part 2#6074
rustyrussell merged 18 commits into
ElementsProject:masterfrom
rustyrussell:zero-fee-htlc-prep-2

Conversation

@rustyrussell

@rustyrussell rustyrussell commented Mar 7, 2023 •
edited by cdecker
Loading

Copy link
Copy Markdown
Contributor

(Based on #6073 , starts at hsmd: add support for lightningd signing onchain txs. )

This simply shifts the onchaind logic from "tell lightningd to broadcast a tx for us" to "tell lightningd to create a tx for us". This would be fairly simple, except for three things:

  1. We generalize the "how do onchaind know whether a tx we see is the one we asked you to create" by having lightningd return the witness, with signature parts flagged (since those can change with fee changes).
  2. The tests had many assumptions about the format of messages from onchaind.
  3. The hsmd interface assumed that onchaind would ask for tx sigs directly, but now lightningd does it.

This removes the rbf logic for now: the next step is to bring it back.

Changelog-None No user visible changes

@rustyrussell rustyrussell added this to the v23.05 milestone Mar 7, 2023
@rustyrussell
rustyrussell requested a review from cdecker as a code owner March 7, 2023 00:52
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from b734de5 to 6a7ee63 Compare March 23, 2023 10:19
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch 3 times, most recently from b8fee58 to db93f41 Compare March 23, 2023 23:20
@rustyrussell rustyrussell mentioned this pull request Mar 23, 2023
Merged
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch 2 times, most recently from 4e2a4e4 to 5758155 Compare March 24, 2023 03:29
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from 5758155 to 68168a9 Compare April 1, 2023 03:55
@cdecker cdecker added hsmd VLS Marks issues and PRs that are relevant to VLS labels Apr 5, 2023
@cdecker

cdecker commented Apr 5, 2023

Copy link
Copy Markdown
Member

ACK 68168a9

@rustyrussell
hsmd: add support for lightningd signing onchain txs.
4499948 
We previously used WIRE_HSMD_SIGN_DELAYED_PAYMENT_TO_US,
WIRE_HSMD_SIGN_REMOTE_HTLC_TO_US, WIRE_HSMD_SIGN_PENALTY_TO_US and
WIRE_HSMD_SIGN_LOCAL_HTLC_TX which allow onchaind to sign txs,
but only for its specific channel.

We now want lightningd to sign these, but it's not bound to a specific
channel.  So let's add variants that don't require that.

We are also now explicit about *what input* to sign.  It's always zero
for now, but future combinations may change that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
lightningd: handle first case of onchaind handing a tx to us to create.
3bfce44 
We add code for the case of spending a (timelocked) to-us output of an
HTLC output, so lightningd can do it (rather than onchaind doing all
the work itself).

onchaind still needs to know whether we bothered to create the tx
(fees might have caused it to evaporate, so it should consider it
immediately resolved rather than waiting for it), and what the
witnesses were, and which parts of the witnesses were signatures (as
these parts might change, with RBF or in future, combining other txs).

The inputs (known to onchaind) and the witnesses (told by lightningd)
uniquely identify the spend for the purposes of onchaind.  In
particular, they definitely distinguish HTLC-timeout and HTLC-success
cases.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: infrastructure to offload tx creation to lightningd.
2210551 
Since we do both our own internal handling and handing it to
lightningd, we add to `proposed_resolution` to handle the lightningd
case.

Note, in particular, that we fix the blockheight calculation: it's out
by one, in that if we see a tx and our CSV lock is 5, we only need to
wait 4 more blocks, not 5.  This will matter as we start using it, and
convert the tests.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pyln-testing: adapt wait_for_onchaind_broadcast function for when onc…
8ec916f 
…haind uses lightningd for broadcast.

We can no longer grab the tx in one line as we did with
wait_for_onchaind_broadcast, we need to track the broadcast from
lightningd.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd to send "delayed_output_to_us" from HTLC txs.
d48ad0d 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd for spending our unilateral "to us" output.
aa34bb9 
This follows the same pattern as the "spend htlc tx" in fact.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
lightningd: remember depth of closing transaction.
3c57452 
We'll use this later to calculate deadlines for spending txs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind, pytest: disable RBF logic.
6653f69 
We'll reimplement it once lightningd makes all the onchain txs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: have lightningd create our penalty txs.
77d6d26 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
common: expose low-level htlc_tx function.
2609888 
We'll want this, as lightningd will want to produce htlc txs based on
what it's told from onchaind, so we need a lower-level accessor.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd to sign and broadcast htlc_success transacti…
ace8dc2 
…ons.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd to sign and broadcast htlc spending txs.
e775180 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd to sign and broadcast htlc_timeout transacti…
09b59b5 
…ons.

This breaks tests/test_closing.py::test_onchain_all_dust's accouting
checks.

That test doesn't really test what it claims to test; sure, onchaind
*says* it's going to ignore the output due to high fees, but the tx
still gets mined.

I cannot figure out what the test is supposed to look like, so I
simply disabled the accounting checks :(

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: use lightningd to sign and broadcast htlc expired txs.
70a0387 
This is when they closed the channel, we can simply make our own tx to
expire the HTLC.  (The other case is where we closed the channel, and
we have a special htlc_timeout tx which we have their signature for).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
pytest: clean up wait_for_onchaind_tx interface, remove wait_for_onch…
97af067 
…aind_broadcast

Using single tuples in Python is ugly, so:
1. Rename wait_for_onchaind_tx to wait_for_onchaind_txs.
2. Make it take tuples explicitly.
3. Make wait_for_onchaind_tx a simpler wrapper/unwrapper.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: propose_ignore specifically to ignore if output reaches depth.
94d217c 
We do this for HTLCs which will timeout to them: we watch them in case we
want to fulfill them as a preimage comes in, but once they reach depth we
can forget about them.

We change the message, which causes some more test churn.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: remove now-unused direct tx creation.
fb32374 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
onchaind: no longer need information about current feerates.
450ae4d 
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
rustyrussell force-pushed the zero-fee-htlc-prep-2 branch from 68168a9 to 450ae4d Compare April 5, 2023 23:34
@rustyrussell

rustyrussell commented Apr 5, 2023

Copy link
Copy Markdown
Contributor Author

Trivial rebase on master

Ack 450ae4d

@rustyrussell
rustyrussell merged commit a3b81ba into ElementsProject:master Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdecker cdecker Awaiting requested review from cdecker cdecker is a code owner

Assignees

No one assigned

Labels

hsmd VLS Marks issues and PRs that are relevant to VLS

Projects

None yet

Milestone

v23.05

Development

Successfully merging this pull request may close these issues.

2 participants

@rustyrussell @cdecker