Add low-level tooling support for taproot channels#3079
Closed
Add low-level tooling support for taproot channels#3079
Conversation
Most of our instances of `TransactionWithInputInfo` only contained the input (`InputInfo` instance) and the transaction. This worked because we stored the (segwit v0) redeem script in the `InputInfo` instance. But we can actually get rid of that if we store with each transaction the parameters that allow re-creating the scripts at runtime. We don't need many parameters, so this is quite simple to add: - delayed txs simply need the `to_self_delay` value used - HTLC txs simply need the `payment_hash` and `htlc_expiry` Based on that we can trivially re-build scripts and redeem info at runtime using `ChannelKeys`. This ensures that we will only need the `commitmentFormat` to know the exact script that is being used, instead of having to encode different redeem information in `InputInfo`. This will also reduce the size of our channel data, because storing redeem info can be removed. In order to handle data that was encoded before this change, we: - update `ChannelCodecs4.scala`: this is quite a simple change, but this file is getting messy since we made many changes to transaction codecs so please review carefully - when re-connecting a channel, if it contains transactions that are missing some data, we simply re-create those transactions based on the commitment data
We don't need this helper for other transactions: it is only used to verify the remote signatures received in `commit_sig`.
In this commit, we somewhat heavily re-design `Transactions.scala`. The main change is that we stop exposing the default `sign` function that was too abstract for callers, and have each `TransactionWithInputInfo` expose its dedicated `sign` function, with simpler arguments depending on each transaction type. One very important thing to note is that there is a heavy distinction between transactions that require both a local and a remote signature (commit-tx, splice-tx, closing-tx and htlc-tx) and transactions that only need a local signature. This means we can completely remove the `addSigs` step for transactions that only need a local signature, and directly create signed signatures, which simplifies the calling code. This also lets us better encapsulate the `extraUtxos` parameter, which only makes sense for txs for which we add wallet inputs, which is only a small subset of our transactions. The previous commits ensured that we store the data we need for each transaction type to allow re-deriving and signing everything based only on the following runtime data: - the commitment format - the commitment funding keys - the commitment keys This makes the calling code much less error-prone and removes the need to store the redeem script in the `InputInfo` class, which wouldn't have worked for taproot transactions. Note that for simplicity's sake, we currently keep the field (to avoid a painful codecs migration) since we will introduce v5 codecs when implementing taproot channels, which will let us clean up this inconsistency. We also move each transaction's creation function in its companion object: this makes the diff a bit harder to review, but ensures that each transaction's implementation is well encapsulated, which makes it easy to review that each transaction type is correctly implemented. Instead of using placeholder signatures when evaluating the weight of transactions, we actually sign them. This means we sign twice, but is not a performance issue in this case because it only happens once when we initiate a force-close, and simplifies the code. We also introduce a `RedeemInfo` trait which makes the code forward compatible with taproot channels. The changes to `Transactions.scala` are a lot to review, but unfortunately I didn't find a good way to split them while having something consistent and all the unit tests passing...
We also rename this function, which didn't make any sense: we are not checking that this transaction is spendable (which means that we could spend its *outputs*), but rather that the transaction is fully signed and valid.
Transactions that spend the channel funding output don't behave the same way as transactions that can be unilaterally signed, so it makes sense to use different traits. This lets us better scope our shared code and remove unused parameters (e.g. sighash is much simpler for transactions spending the channel funding output and the concept of `TxOwner` is unnecessary). We also take this opportunity to explicitly separate the two distinct signing cases for `ChannelSpendTransaction`: 2-of-2 multisig and musig2, which create different signature types. By pattern matching on the commitment format, callers should decide whether they want to produce an individual signature or a partial signatures. This may need some rework to fully adapt to taproot, but it has the benefit of exposing which part of the code should be concerned about the type of sigs it creates.
We previously hard-coded the input index to always be 0, which actually created issues with replaceable txs for which we added wallet inputs. We don't need to hard-code this value, we can always find the input index by matching the `outPoint`.
Anchor signing functions will either use the local or remote commitment keys for taproot commitments, depending on whether the local or remote commitment has been broadcast.
We can actually use pre-computed weights for every type of force-close transaction, which simplifies the code and ensures that the tests use deterministic fees, regardless of the signature size (since ECDSA sigs are der-encoded, they have a variable size which slightly impacts the witness weight). Note that we use per-transaction constants, even though some have the same weight: this is more future-proof since future commitment formats may use different script combinations.
Add the actual commit tx output to CommitmentOutput. Add the 2nd-level HTLC output to InHtlc/OutHtlc.
We remove the re-creation of closing transactions on restart in favor of a cleaner codec migration. We add test vectors generated from the `master` branch.
We previously included confirmation targets directly in HTLC and anchor transactions, which didn't really belong there. We now instead set confirmation targets at publication time, based on the current state of the force-close attempt. We also extract witness data for HTLCs (preimage and remote signature) when creating the command to publish the transaction, instead of doing it inside the publisher actors. We improve the checks done in the pre-publisher actors, which now take into account the min-depth for commit txs before aborting anchor txs. The `ReplaceableTxFunder` is also simpler now, thanks to a better encapsulation of the transaction's contents.
Member
Member
Author
Yes it makes sense, I used #3075 to validate that it works well with taproot (and it does!). |
Member
Author
|
Replaced by #3086 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
It does not add new wire messages or modify the channel creation/update workflow.
Based on #3075