Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Newsletters: add 108 (2020-07-29) #442

Merged
merged 5 commits into from
Jul 29, 2020

Conversation

harding
Copy link
Contributor

@harding harding commented Jul 27, 2020

allow two peers with an existing channel between them to negotiate a
new commitment transaction using a different format. Commitment
transactions are used to allow LN nodes to put the current channel
state onchain, but the existing protocol only allows nodes to upgrade
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know we use 'onchain' as an adjective, but its use here as a preposition seems a bit unusual.

state onchain, but the existing protocol only allows nodes to upgrade
to new commitment formats by opening new channels. Osuntokun's
proposal would allow a node to signal to its peer that it wants to
switch formats; if the peer agrees, the two nodes would port their
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/; if/. If/

Development Meeting in relation to taproot activation. It was noted
that the version of taproot merged into Bitcoin Core (see PR
[#17977][Bitcoin Core #17977]) will likely be based on wtxid relay.
This will make it more complicated to backport taproot to the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To summarise what I think the problem is:

I'm not sure how difficult it actually is to backport wtxid relay. It seems to me that it shouldn't be too hard. v0.20 was only released a few weeks ago, and the wtxid PR was open long before that. I think we should check on how difficult it actually is before running with this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @jnewbery that it's not /that/ hard to add wtxidrelay to 0.20 -- it's just not a matter of cherry-picking the patch from master; the code will need to be adapted and re-reviewed. For most backports it /is/ just a mater of cherry-picking a patch, so this would just be "hard" in comparison to that, in my opinion.

The "only announce taproot transactions using wtxids" is an additional change needed (that's not done yet, afaik) to actually avoid 0.20 and earlier nodes from spamming themselves by re-requesting taproot spends that they don't understand by txid from every peer.

The drawback of releasing taproot in a 0.20.x release but only for block consensus but not tx relay is that it would both reduce the ability to actually use taproot when it activated (if you can't spend a taproot tx via p2p and get it reliably to the miners, taproot's not really usable), and, if people did use taproot, would reduce the effectiveness of compact block relay (since nodes wouldn't already have the taproot spending transactions). All of which is to say this approach still means waiting for 0.21.y before taproot's usable, so doesn't really speed anything up compared to just releasing taproot in 0.21.y.

I think it would be super-useful to expand on the explanation in the newsletter a bit; there isn't any other canonical reference of how taproot/wtxid interacts anywhere else to point people to apart from the not very explicit description in the wtxid PR description, and having one would be helpful :)

transactions for [anchor outputs][topic anchor outputs]. Not yet
added is higher-level support that will allow Eclair to negotiate
the use of anchor outputs with peers, but this early step does pass
all [proposed test vectors][BOLTs #688].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should link directly to the comment: lightning/bolts#688 (comment)?

@bitschmidty
Copy link
Contributor

Bitcoin Core #19473 summary added.

Also added a todo item for the River Financial field report as it looks that is ready.

Copy link
Member

@adamjonas adamjonas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments and corrections. The one I see to tighten up is the wtxid's impact on taproot activation paragraph. Thanks @harding!

Comment on lines 36 to 37
All discussion participants seemed to support the basic idea. In
discussion, Bastien Teinturier [suggested][teinturier simple] that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cut wordiness and reduce repetition

Suggested change
All discussion participants seemed to support the basic idea. In
discussion, Bastien Teinturier [suggested][teinturier simple] that
All discussion participants seemed to support the basic idea. Bastien Teinturier [suggested][teinturier simple] that

All discussion participants seemed to support the basic idea. In
discussion, Bastien Teinturier [suggested][teinturier simple] that
it would be simplest to only allow switching commitment formats when
channels had no pending payments (HTLCs); this implies nodes would
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
channels had no pending payments (HTLCs); this implies nodes would
channels had no pending payments (HTLCs); this that implies nodes would


ZmnSCPxj [noted][zmnscpxj re-funding] that the same basic idea could
be used to essentially update the funding transaction offchain, for
example in the case where proposals such as [taproot][topic taproot]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
example in the case where proposals such as [taproot][topic taproot]
example, in the case where proposals such as [taproot][topic taproot]

_posts/en/newsletters/2020-07-29-newsletter.md Outdated Show resolved Hide resolved
current 0.20.x branch of Bitcoin Core and so possibly ensure that
the earliest release of taproot activation logic will be in the
Bitcoin Core 0.21.x branch, whose major release is roughly
[expected][Bitcoin Core #18947] in December 2020. A proposed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at @jnewbery's comments, maybe there is a larger re-write here, but noting that this last sentence is a little hard to follow. I'd suggest breaking into two sentences.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Waiting to rewrite based on @jnewbery's comments until I see confirmation that John's summary is basically what AJ was thinking (John asked about it in the ##taproot-activation channel).

all [proposed test vectors][BOLTs #688].

- [LND #4455][] enables safe PSBT-based batched channel opens. Previously, each
successful channel negotation in the batch would prematurely broadcast the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
successful channel negotation in the batch would prematurely broadcast the
successful channel negotiation in the batch would prematurely broadcast the

Comment on lines 142 to 143
whole transaction with all channel funding outputs. This meant that it was
possible for subsequent channel negotiation failures to result in stuck funds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Slightly simpler

Suggested change
whole transaction with all channel funding outputs. This meant that it was
possible for subsequent channel negotiation failures to result in stuck funds.
whole transaction with all channel funding outputs. This meant
subsequent channel negotiation failures could result in stuck funds.

@bitschmidty
Copy link
Contributor

Added commits for the field report and stack exchange.

Copy link
Contributor Author

@harding harding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits on the River report, the B.SE section, and Mike and Carl's PRs. Everything looks basically good to me; I'll make any edits tomorrow which still need to be addressed. Thanks everyone!

_includes/articles/river-descriptors-psbt.md Outdated Show resolved Hide resolved
_includes/articles/river-descriptors-psbt.md Outdated Show resolved Hide resolved
_posts/en/2020-07-29-river-descriptors-psbt.md Outdated Show resolved Hide resolved
_posts/en/newsletters/2020-07-29-newsletter.md Outdated Show resolved Hide resolved
_posts/en/newsletters/2020-07-29-newsletter.md Outdated Show resolved Hide resolved
_posts/en/newsletters/2020-07-29-newsletter.md Outdated Show resolved Hide resolved
_posts/en/newsletters/2020-07-29-newsletter.md Outdated Show resolved Hide resolved
possible for subsequent channel negotiation failures to result in stuck funds.
This merged PR introduces a `--no_publish` flag to the `openchannel` command,
which can be used to delay transaction broadcast until the very last channel
in the batch.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
in the batch.
in the batch has been created.


- **Upgrading channel commitment formats:** Olaoluwa Osuntokun
[posted][osuntokun upgrade] to the Lightning-Dev mailing list this
week with a suggested extension to the LN specification that will
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

µ-nits in this paragraph:

  • s/will/would/ allow (there is also "Osuntokun’s proposal would allow" and "would port" in the same paragraph)

  • "between them" seems unneeded

  • maybe s/upgrade to new/change/ commitment formats

  • s/on chain/on-chain/? ("put something on chain" seems awkward)

(started reviewing, but I'll be offline for half an hour, so posting this in case GH loses it)

@bitschmidty
Copy link
Contributor

Addressed feedback from @harding and @adamjonas , thanks guys!

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great newsletter. Thanks to all the authors!

I have a couple of small comments inline. Otherwise looks really good.

_posts/en/2020-07-29-river-descriptors-psbt.md Outdated Show resolved Hide resolved

The solution implemented in this merged PR is to announce
transactions by their wtxid---which includes a commitment to the
witness data for segwit transactions. It is expected that the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'expected' is maybe a bit strong. When I asked Pieter about whether we'd only relay taproot using wtxid relay, he said that he hadn't really thought about it. Perhaps you could soften this to something like "A taproot implementation in Bitcoin Core could then only relay transactions by their wtxid ..."

Copy link
Collaborator

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit more than halfway through, but since I'm reviewing somewhat late in the cycle I'm putting up this feedback now to hopefully allow time to use it.

would be paid to a new funding transaction that is kept offchain. If
the channel terminates with a mutual close, the original funding
transaction output is paid to the final channel balances; otherwise,
the offchain secondary funding transaction can be published onchain
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: here there is "published onchain"; two paragraphs above there is "publish...on chain"

2. **Creating and importing new wallets is easy because the descriptor language
is able to define desired scripts.** River is able to maintain many wallets
with different scripts and as a result have separate security models for each
wallet. A P2WSH multi-signature descriptor is used for the cold wallet and a
Copy link
Collaborator

@jonatack jonatack Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 2 previous uses of "multi-signature" in this report are not hyphenated; maybe settle on one of the two spellings.

in the feature set. Descriptors is a language for describing scripts that was
authored by Pieter Wuille and used in Bitcoin Core. In River’s wallet software,
the descriptor language is leveraged in several places from wallet creation to
address generation. Before descriptors, there had been no interoperable way for
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"interop*" (interoperability, interoperate, interoperable, and interoperability) is used four times in the report. Perhaps vary the form ("universal", "industry-wide", "compatible", etc.) or clarify the messages if they are not saying the same thing.

greatly reduced the complexity in wallet operations and has improved flexibility
in the feature set. Descriptors is a language for describing scripts that was
authored by Pieter Wuille and used in Bitcoin Core. In River’s wallet software,
the descriptor language is leveraged in several places from wallet creation to
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the descriptor language is leveraged in several places": "leverage" is already used in the second paragraph. Perhaps "deployed", "used", or something else if a more precise meaning is intended.


The decision to implement Output Script Descriptors in the wallet software has
greatly reduced the complexity in wallet operations and has improved flexibility
in the feature set. Descriptors is a language for describing scripts that was
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"flexibility" is already stated in the second and in the last paragraph and it's somewhat unclear what "flexibility in the feature set" means here before reading the bullet points below...perhaps ease/speed of new/custom feature development, expanded the possible feature set, or allowed a wider range of features.


- [What are the different upgradability features in the BIP-Taproot (BIP341) proposal?]({{bse}}96951)
Michael Folkson answers a question from Twitter regarding the different ways
in which taproot can enable upgradability including leaf versions for script
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: a comma seems to be missing after "upgradability"

(fwiw it seems both versions are acceptable but I do prefer "upgradeability" with an "e", as otherwise it seems like "grad" would be pronounced like "graduate" or "Stalingrad" :-))

for taproot. Pieter also answers a [similar question][stack exchange miner
signaling] about mining signaling support.

- [Could we skip the Taproot soft fork and instead and use Simplicity to write the equivalent of Taproot scripts?]({{bse}}97049)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/and instead and use/and instead use/


- [Could we skip the Taproot soft fork and instead and use Simplicity to write the equivalent of Taproot scripts?]({{bse}}97049)
Michael Folkson, quoting Pieter Wuille, outlines the current state of
[Simplicity][news96 simplicity] noting also that integrating Simplicity in
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ISTM there is a missing comma before "noting", or alternatively s/noting also/and also notes/

[Hardware Wallet Interface (HWI)][hwi], [Bitcoin Improvement Proposals
(BIPs)][bips repo], and [Lightning BOLTs][bolts repo].*

- [Bitcoin Core #19473][] adds support for `networkactive` as both a command line
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest placing this PR after 18044 wtxid relay (which to me is the headlining change of the week's news, but I digress)

(BIPs)][bips repo], and [Lightning BOLTs][bolts repo].*

- [Bitcoin Core #19473][] adds support for `networkactive` as both a command line
start-up and configuration file option. Setting this option enables or
Copy link
Collaborator

@jonatack jonatack Jul 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: not sure "support for", "both" and "start-up" are needed; maybe just "adds networkactive as a command line and configuration file option."

Copy link
Collaborator

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more comments. All the links seem good. Great work on the wtxid relay discussion! This for me is the highlight of the week. Apologies for the late review. Who said summer was quiet?

transactions to their peers by the transaction's txid. However, txids
don't commit to the witness data in segwit transactions, so a node who
downloaded an invalid or unwanted segwit transaction can't safely
assume that any transaction with that same txid is also invalid or
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/that/the/

announce transactions they wouldn't accept themselves, so only a
disruptive peer that wanted to waste its own upload bandwidth would
advertise invalid or unwanted transactions. However, one type of
unwanted transaction today are spends of v1 segwit UTXOs---the types
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after "one type of..." maybe s/are spends of/is the spending of/


However, after this PR was merged into Bitcoin Core's master
development branch, it was [discussed][meeting xscript] during the
weekly Bitcoin Core Development Meeting whether taproot's soft
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe "weekly Bitcoin Core development meeting"


1. **Backport wtxid:** both wtxid relay and taproot will be
backported if there's a 0.20.x taproot release. John Newbery has
already created a quick [wtxid relay backport][].
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe omit "quick"

also arguably unclear what it means -- quick to make, quick to review, (quick in performance ;), etc.


4. **Don't backport anything:** don't backport wtxid relay or
taproot---let taproot wait until after the release of Bitcoin
Core 0.21, roughly [expected][Bitcoin Core #18947] in December
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure no one reading this will confuse it with saying that taproot is expected in December, maybe drop the phrase about the release timing.

Core 0.21, roughly [expected][Bitcoin Core #18947] in December
2020.

No clear conclusion on which of these options to follow has been
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe: "At the time of this writing, no consensus was reached yet on which option to follow."

to this merge, all Bitcoin nodes announced new unconfirmed
transactions to their peers by the transaction's txid. However, txids
don't commit to the witness data in segwit transactions, so a node who
downloaded an invalid or unwanted segwit transaction can't safely
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/who/that/

s/downloaded/downloads/ or s/can't/couldn't/

weekly Bitcoin Core Development Meeting whether taproot's soft
dependency on wtxid relay will make it more complicated to backport
taproot to the current 0.20.x branch of Bitcoin Core. Four options
were mentioned during the meeting and in subsequent discussions:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are good, but I think they basically resolve into "backporting wtxid is best", so I'm going to leave readers just the meeting link.

@bitschmidty bitschmidty merged commit c2dcbb0 into bitcoinops:master Jul 29, 2020
@bitschmidty
Copy link
Contributor

Great newsletter team!

@harding with the meat, @dongcarl with the potatoes, and @adamjonas @jnewbery @jonatack @ajtowns taste testing, thanks!

@bitschmidty
Copy link
Contributor

@philipglazman Thank you for your contribution on the field report from River! (dessert?)

@harding
Copy link
Contributor Author

harding commented Jul 29, 2020

(And @bitschmidty with the gravy!)

@jnewbery
Copy link
Contributor

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants