watchtower: conditionally reconstruct justice txns for anchor channels - #4576
Conversation
91cefdd to
7ee5507
Compare
There was a problem hiding this comment.
Why no TypeRewardAnchorCommit? Also why didn't we need to change these once we added support for static remote key?
There was a problem hiding this comment.
Don't we also need to update the encode/decode methods? As looks like they'll fail with an unknown blob type as is.
There was a problem hiding this comment.
nope, the encode/decode logic is gate on FlagCommitOutputs which is also set in TypeAltruistAnchorCommit. the payload format doesn't change at all between the legacy and anchor channel blobs
There was a problem hiding this comment.
FWIW, when in the non nested-p2wsh land I usually like to refer to this as the "witness script" to distinguish things a bit.
There was a problem hiding this comment.
hmm, technically this is referred to as the witness program in BIP 143. IMO the term witness script is ambiguous because we use it interchangeably as as you said for witness program, but also for redeem script.
There was a problem hiding this comment.
Hmm, from my PoV (non-blocking here):
- witness program -> the pkScript iself
- this is also used as the redeem script for nested p2wsh
- witness script -> the top element in the witness stack for p2wsh sends
- this is typically what's signed, but there's a quirk where Script "injects" an "unrolled p2wkh" script
There was a problem hiding this comment.
Heh yeh this can be super confusing for those that haven't read BIP 141 closely and/or implemented the verification on the script level....
There was a problem hiding this comment.
the confusing part is the sign descriptor accepts a WitnessScript, which can be either a redeem script or a witness program. just noting that our API could be improved.
7ee5507 to
3a2b11f
Compare
wpaulino
left a comment
There was a problem hiding this comment.
Not my area of expertise, but the changes read well to me.
This is also a prepatory step to making TestJusticeKitRemoteWitnessConstruction parameteried by the blob type so we can test both anchor and legacy witness construction.
This is preparation for later commits where the values returned by member methods will need to be conditioned on the blob type used during decryption.
This commit modifies the ToRemoteWitnessScript function to be conditioned on the blob type, and return either the legacy or anchor to-remote script. The same witness satisfies either script, so no changes are necessary to ToRemoteWitnessStack.
This commit modifies the JusticeDescriptor to support creation of justice transactions spending from anchor commitments. Rather than the unencumbered p2wkh scripts from before, the tower will now use the to-remote-confirmed that includes the additional CSV delay of 1. This also requires setting the sequence number appropriately on the to-remote input.
This commit fixes the to-local-witness estimate to use the correct witness size estimate for anchor channels. We retain the off-by-one bug from the original constant otherwise.
3a2b11f to
a0e54a9
Compare
| // avoid invalidating signatures by older clients. For anchor channels | ||
| // we correct this and use the correct witness size. | ||
| if p.JusticeKit.BlobType.IsAnchorChannel() { | ||
| weightEstimate.AddWitnessInput(input.ToLocalPenaltyWitnessSize) |
This PR is the first in a series that will enable watchtower support for anchor channels in 0.12.
We start by adding a new
blob.FlagAnchorChannelindicating whether an encrypted blob issupposed to spend an anchor channel. Upon successful decryption, this bit is used to signal
that the tower needs to craft the justice txn such that properly spends a
to_remote_confirmedp2wsh input rather than the legacy
to_remotep2wkh input.The good news is that these changes require no modification of the encrypted payload format.
The anchor payloads are equal size and contain exactly the same witness info as the legacy
payloads, only requiring light modifications to the reconstruction logic.