UpfrontShutdownScript field#1290
Merged
Merged
Conversation
We don't implement the upfront_shutdown_script feature. However we update our encoding to always specify it. This allows extending OpenChannel/AcceptChannel with tlv streams. There is one caveat: Phoenix shipped with a version that's incompatible. So we use a workaround to identify unpatched Phoenix versions and send them the old encoding.
Codecov Report
@@ Coverage Diff @@
## master #1290 +/- ##
==========================================
+ Coverage 77.18% 79.15% +1.96%
==========================================
Files 143 144 +1
Lines 9976 11022 +1046
Branches 408 472 +64
==========================================
+ Hits 7700 8724 +1024
- Misses 2276 2298 +22
|
Remove the hack inside the metered codec. We can actually encode this logic directly in the openChannel codec.
pm47
reviewed
Jan 29, 2020
| // Bolt 2 specifies that upfront_shutdown_scripts must be P2PKH/P2SH or segwit-v0 P2WPK/P2WSH. | ||
| // The length of such scripts will always start with 0x00, so we use that to discriminate whether we're decoding an | ||
| // upfront_shutdown_script or a tlv stream. | ||
| (b: BitVector) => Attempt.successful(DecodeResult(b.nonEmpty && b.startsWith(hex"00".bits), b)) |
Member
There was a problem hiding this comment.
How about something more explicit:
// Bolt 2 specifies that upfront_shutdown_scripts must be P2PKH/P2SH or segwit-v0 P2WPK/P2WSH.
// Those have predictable lengths, which we use to discriminate whether we're decoding an
// upfront_shutdown_script or a tlv stream.
(b: BitVector) => Attempt.successful(DecodeResult(b.startsWith(hex"0016".bits) || b.startsWith(hex"0017".bits) || b.startsWith(hex"0019".bits) || b.startsWith(hex"0022".bits), b))* Clarify comment * Use TLV type in custom range
pm47
approved these changes
Jan 29, 2020
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
3 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.
We don't implement the
upfront_shutdown_scriptfeature yet.However we update our encoding to always specify its field and support reading it.
This allows extending
OpenChannel/AcceptChannelwith tlv streams while staying compliant with the spec and other implementations (lnd just started to add theupfront_shutdown_scriptfield to theirOpenChannelmessage, even if we don't activate the feature bit).There is one caveat: Phoenix shipped with a version that's incompatible because it interprets the
upfront_shutdown_scriptfields as a TLV stream.So we use a workaround to identify unpatched Phoenix versions and send them the old encoding.
This PR can be applied and deployed safely today.
Once Phoenix is upgraded with that code in the next release, and enough users have updated, we can remove that ugly switch and forget this ever happened :).
Tested against:
TODO: