Refactor output redeem type from TransactionWithInputInfo#3056
Refactor output redeem type from TransactionWithInputInfo#3056
TransactionWithInputInfo#3056Conversation
t-bast
force-pushed
the
refactor-input-info
branch
from
bb5db04 to
1cd554a
Compare
We refactor `TransactionWithInputInfo` to extract the redeem information from the `InputInfo` class. This is necessary for taproot channels (and other future segwit versions) to easily integrate into our transaction class hierarchy: this way we can explicitly specify which kind of redeem paths are possible for each transaction type. For example, we can now express that a `CommitTx` uses either a segwit v0 input or a taproot input that we can spend via the key path. Note that when we'll later introduce v3 commitments, we may have txs that can be spent: - via segwit v0 for non-taproot channels - via a taproot script path for taproot channels - AND via a taproot key path for v3 taproot channels This is why we cannot simply pick a single one of the taproot redeem paths, we may need to support multiple of them in the future. The core of this PR is just the following small type changes, that should be reviewed first: - we introduce the `RedeemInfo` trait - we remove the redeem information from `InputInfo`, which now becomes a trivial container for `OutPoint` + `TxOut` - we update `CommitmentOutputLink` to use an abstract `RedeemInfo` in the `CommitmentOutput`, which will make it easy to add taproot outputs without modifying the `CommitmentOutputLink` type - we take the `commitmentFormat` parameter when creating transactions, which makes it easy to pattern match on it to produce the right redeem information Note that this highlighted that we unnecessarily recomputed HTLC scripts when creating claim-htlc transactions: we can directly leverage the existing `CommitmentOutput`s. I wondered if we could do this for every transaction creation, but we can't because we need to handle spending revoked commitments, for which we don't have a `CommitmentSpec` object and thus cannot create the set of `CommitmentOutput`s.
t-bast
force-pushed
the
refactor-input-info
branch
from
1cd554a to
fe0252c
Compare
| case class ToRemote(redeemInfo: RedeemInfo.TaprootScriptPathOrSegwitV0) extends CommitmentOutput | ||
| case class ToLocalAnchor(redeemInfo: RedeemInfo.TaprootKeyPathOrSegwitV0) extends CommitmentOutput | ||
| case class ToRemoteAnchor(redeemInfo: RedeemInfo.TaprootKeyPathOrSegwitV0) extends CommitmentOutput | ||
| case class InHtlc(redeemInfo: RedeemInfo.TaprootScriptPathOrSegwitV0, incomingHtlc: IncomingHtlc) extends CommitmentOutput |
There was a problem hiding this comment.
This does not look right: redeem information is logically attached to inputs, not outputs because (we don't know how they will be spent and for the same taproot outputs is could be through different script paths or through the key path.
There was a problem hiding this comment.
redeem information is logically attached to inputs, not outputs
Those are outputs of the commitment transaction, which means they are outputs that we will spend if this commitment is confirmed. An output of a commitment transaction is simply an input of a 2nd-stage transaction, so I'm not sure your comment makes sense? We want to store this information in the output of a commitment, so that we can easily construct the corresponding input of the 2nd-stage transaction.
Also, this isn't introduced by this refactoring: we were already storing the redeemScript along with the CommitmentOutput in CommitmentOutputLink, I only made that clearer by bundling them together because:
- they were always used in combination
- and it made pattern matching and typing function arguments simpler
for the same taproot outputs is could be through different script paths or through the key path.
Do you have an example of that? On the contrary, this relies on the fact that we always only use a single spending path per output (otherwise our whole addSigs architecture wouldn't work either):
ToLocal: we can only spend it with our local key after the delayToRemote: we cannot spend it, we could in theory not fill this informationToLocalAnchor: the only way we're spending that is with our keyToRemoteAnchor: the only way we could spend that is after the 16-blocks delay (but we never really implemented claiming them)InHtlc: the only way we can spend is by revealing the preimage (success path)OutHtlc: the only way we can spend is by using the timeout path
Note that revoked commitments are handled differently: they are the only "other" way we can spend a commitment output.
Or am I misunderstanding what you're suggesting?
| sealed trait TransactionWithInputInfo { | ||
| def input: InputInfo | ||
| /** Redeem information for the [[input]] this transaction spends. */ | ||
| def redeemInfo: RedeemInfo |
There was a problem hiding this comment.
Why not make it a member of InputInfo ?
There was a problem hiding this comment.
This is actually linked to the previous comment: the RedeemInfo is first created as part of a CommitmentOutput, and later used to create a 2nd-stage transaction that contains the RedeemInfo and the outpoint details. But for the first step, when you create the commit tx itself, you're creating an output, so including input details is impossible (chicken-and-egg).
Put differently, we know the redeem path of each output when we create commitment transactions. We thus create redeem information to enrich outputs with details about how to spend them. We later can use this information to actually spend those outputs. I think that it makes a lot of sense when you put it that way?
|
Closing in favor of #3074 |
2 participants
We refactor
TransactionWithInputInfoto extract the redeem information from theInputInfoclass. This is necessary for taproot channels (and other future segwit versions) to easily integrate into our transaction class hierarchy: this way we can explicitly specify which kind of redeem paths are possible for each transaction type. For example, we can now express that aCommitTxuses either a segwit v0 input or a taproot input that we can spend via the key path.Note that when we'll later introduce v3 commitments, we may have txs that can be spent:
This is why we cannot simply pick a single one of the taproot redeem paths, we may need to support multiple of them in the future.
The core of this PR is just the following small type changes, that should be reviewed first:
RedeemInfotraitInputInfo, which now becomes a trivial container forOutPoint+TxOutCommitmentOutputLinkto use an abstractRedeemInfoin theCommitmentOutput, which will make it easy to add taproot outputs without modifying theCommitmentOutputLinktypecommitmentFormatparameter when creating transactions, which makes it easy to pattern match on it to produce the right redeem informationNote that this highlighted that we unnecessarily recomputed HTLC scripts when creating claim-htlc transactions: we can directly leverage the existing
CommitmentOutputs. I wondered if we could do this for every transaction creation, but we can't because we need to handle spending revoked commitments, for which we don't have aCommitmentSpecobject and thus cannot create the set ofCommitmentOutputs.