Skip to content

Newsletters: add 58 (2019-08-07)#176

Merged
bitschmidty merged 1 commit into
bitcoinops:masterfrom
harding:2019-08-07-newsletter
Aug 7, 2019
Merged

Newsletters: add 58 (2019-08-07)#176
bitschmidty merged 1 commit into
bitcoinops:masterfrom
harding:2019-08-07-newsletter

Conversation

@harding
Copy link
Copy Markdown
Collaborator

@harding harding commented Aug 4, 2019

No description provided.

This week's newsletter announces the maintenance release of Bitcoin Core
0.18.1, summarizes a discussion about BIP174 extensions, and notes a
proposal for LN trampoline payments. Also included are our regular
sections on bech32 sending support and notable changes in popular
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps worth explicitly calling out the interview / field report from BRD in the summary as it is pretty compelling and notable.

Copy link
Copy Markdown
Contributor

@bitschmidty bitschmidty left a comment

Choose a reason for hiding this comment

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

tACK.

Pushed a commit to include the bech32 section into the bech32 blog post.

Also pushed a small wording correction commit.

Finally, wanted to note that the compatibility css change that highlights in blue the element being anchor linked to is active on the site now. I think it looks good throughout the site and I hope others agree. Otherwise we can limit the scope of the css selector as @harding I believe already noted.

Copy link
Copy Markdown
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.

A few comments inline.

Anchors to list items and blue highlighting are a great addition!

Comment thread _includes/specials/bech32/21-brd.md Outdated
@@ -0,0 +1,76 @@
As we begin the final four weeks of Optech publishing this special
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This intro sentence works well in the newsletter, but not so well in a permanent blog post (where the number of weeks remaining in the series doesn't mean anything). Perhaps this sentence could be separated from the rest of the feature and placed directly in the newsletter, with a different intro sentence in the blog post?


{% include specials/bech32/20-percentage-loss.md %}

## BRD's bech32 implementation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This subheading might be more exciting if we called it "field report" or "implementation lessons" or similar

[libsecp256k1][libsecp256k1 repo], [Bitcoin Improvement Proposals
(BIPs)][bips repo], and [Lightning BOLTs][bolts repo].*

- [Bitcoin Core #15878][] changes the `walletcreatefundedpsbt` to signal
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be PR15911 (15878 is the issue)


- [LND #3164][] creates a new database into which information about the
past 1,000 payments is stored. (The default of 1,000 can be changed.)
On upgrade, details about previous payments will be populated into
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't read the code in detail, but I think the point of this PR is not about upgrades. I think it's storing routing failures persistently to disk, so that across restarts probability based path finding (lightningnetwork/lnd#2802) can be used.

this database from LND's lower-level HTLC-tracking database.

- [LND #3359][] adds a `ignore-historical-filters` configuration option
that will cause LND to to ignore a `gossip_timestamp_filter` sent from
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

'to to' is two 'to's. That's one 'to' too many.

a peer. That filter allowed peers to request all announcements that would've
been gossiped during an earlier date range. By ignoring requests for
the filter, LND doesn't need to keep those historic announcements in
memory or send them to peers, lowering its memory and bandwidth
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know about "keep those historic announcements in memory". Can you point out where that's happening in the code?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm editing this to just make the same claims as describe in the commit and code comments (that it lowers memory and bandwidth) without providing an explanation for how it achieves the memory savings. I don't want to dig through the LND code, but it was my guess that LND only reads old gossip entries from disk into memory on demand (like most memory-cached databases), so they reduced memory usage by eliminating demand for historic entries.

`payment_hash` field to allow the user to find additional information
about the HTLC. Additionally, the PR provides a new `forward_event`
notification for plugins that alerts when an HTLC has been offered,
settled, failed by both parties, or failed locally (unilaterally).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest reversing the order of these (new forward_event notification then change to listforwards rpc). The motivation given for the PR was the forward_event notification.

@harding
Copy link
Copy Markdown
Collaborator Author

harding commented Aug 6, 2019

Pushed a commit that should address both @moneyball and @jnewbery feedback (thanks!), as well as updating based on @bitschmidty's contacting BRD (super thanks!).

Copy link
Copy Markdown
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.

Lovely and educational writeup! A few items below for consideration or dismissal :)

Comment thread _includes/specials/bech32/21-brd.md Outdated
be rolled out to BRD's entire user base as the default setting. In
preparation of this transition, it is important to get as many companies
and services as possible to voluntarily start supporting bech32 sending
capability. To help encounage adoption, we created the [WhenSegwit][]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s/enounage/encourage/

third-party library dependencies as much as possible, with it currently
using only Pieter Wuille's [libsecp256k1][]. Minimizing dependencies is
typical for a high-security crypto project. For the bech32
implementation, we found that bech32's [BIP173][] is pretty well
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Curious shift here, and a second time in line 72, from third-person to first-person narrative and back. Particularly this first one as the case study begins in the third person.

## Action items

- **Upgrade to Bitcoin Core 0.18.1:** this maintenance release makes
available several bug fixes and other improvements. [Upgrading][bitcoin
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

IIUC the link here points to https://bitcoincore.org/bin/bitcoin-core-0.18.1/. Perhaps a link to the release notes, e.g. behind "bug fixes and improvements" or "(release notes here)" etc., would be handy if not already present.

- *Reserved types for proprietary use:* some applications are
already including data in PSBTs using types that aren't specified
in [BIP174][]. It's proposed that one type byte or a range of
type bytes be reserved for private PSBTs extensions, similar to
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

tentative nit: s/PSBTs/PSBT/

in [BIP174][]. It's proposed that one type byte or a range of
type bytes be reserved for private PSBTs extensions, similar to
reserved IP address ranges for private networks. The exact
construction of this mechanism was a particular focus for
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: s/for/of/

discussion this week.

- *Global version number:* although the goal is to design enhancements
to PSBTs so that they're backwards compatible, Chow proposes to
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: s/proposes/proposed/ for past tense like the other proposed proposals

very beneficial to low-bandwidth LN clients (such as on mobile
devices) that don't want to keep up with gossip traffic and so only
know how to route to a small number of nodes. Sending a trampoline
payment will require coordination between multiple nodes and so
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps s/so/[thus|thereby|therefore|]/ or something else to not repeat "and so"


## Bech32 sending support

*Week 21 of 24 in a [series][bech32 series] about allowing the people
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Perhaps a more general "enabling users to access..." esp. if the audience includes wallet providers.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That would be more general but it'd also be a bit less clear that this section is meant to be focused on getting wallets to do that bare minimum of work to accept bech32 addresses and generate native segwit scriptPubKeys. "Enabling users to access all of segwit's benefits" sounds like we're advocating full segwit support, which (of course) we do, but at a lower priority than trying to get near-universal support for the ability to pay bech32 addresses. (We used to have a longer introduction that spelled all that out, but at least one reader mistook its length for us repeating the whole section each week.)

This is meant for use with LND's Mission Control feature that
tries to better use information from past payment attempts (especially
failures) to choose better routes for subsequent payment attempts.
When you first upgrade to a version including this change, details about previous payments will be populated into
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To not shift narrative: s/When you/During the/

[libsecp256k1][libsecp256k1 repo], [Bitcoin Improvement Proposals
(BIPs)][bips repo], and [Lightning BOLTs][bolts repo].*

- [Bitcoin Core #15911][] changes the `walletcreatefundedpsbt` to signal
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggestion: s/changes/improves/ to change things up with the changes in the next item

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Repeated phrasing in paragraph text (such as the "and so" you identified earlier) can indeed be jarring to read and so should be avoided, but in lists, I think it's beneficial as the parallelism makes reading (and particularly skimming) faster. My usual template for Bitcoin Core RPC changes is to say adds, removes, or changes followed by the RPC name(s) followed by the actual changes. Since most people only use a few RPCs, that puts the information they need to quickly figure out whether this change affects them near the start of the list item, making skimming more effective. (As a general rule, I will quite often use somewhat odd phrasing near the start of a list item so that it's easier to skim because it starts by mentioning the systems affected.)

Copy link
Copy Markdown
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.

A few minor comments

@@ -0,0 +1,83 @@

*The following case study contributed by Optech member company [BRD][]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are still a lot of instances of 'BRD' in this field report. I think it'd sound more natural if some of them were replaced by we/us.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm changing this to consistently use pronouns for things their development team did and to use BRD for the software itself. Hopefully that resolves this concern and @jonatack's concern here: #176 (comment)

Comment thread _includes/specials/bech32/21-brd.md Outdated
well, as it may produce less overhead than the separate HD derivation
path recommended by [BIP84][].

Other observations from BRD's implementation of bech32 support include
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

'Another observation'? (this paragraph includes a single observation)

Comment thread _includes/specials/bech32/21-brd.md Outdated
path recommended by [BIP84][].

Other observations from BRD's implementation of bech32 support include
that fee calculation on Bitcoin is hideous---feerates spike multiple
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this fee estimation in 'Bitcoin' or in 'Bitcoin Core'?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I strongly believe he meant Bitcoin in general, as surrounding comments in the interview not included here referred to having a fixed maximum block size.

@practicalswift
Copy link
Copy Markdown

Another excellent newsletter! Thanks!

ACK b7f630f modulo the feedback from others above

When you first upgrade to a version including this change, details about previous payments will be populated into
this database from LND's lower-level HTLC-tracking database.

- [LND #3359][] adds a `ignore-historical-filters` configuration option
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: an ignore-historical-filters

@adamjonas
Copy link
Copy Markdown
Member

tACK b7f630f. Great work as always.

@harding
Copy link
Copy Markdown
Collaborator Author

harding commented Aug 6, 2019

Pushed a commit that should resolve all feedback (except that which I left a comment explaining my reasoning) from @jonatack, @jnewbery, and @adamjonas (thanks!).

@jnewbery
Copy link
Copy Markdown
Contributor

jnewbery commented Aug 6, 2019

ACK 90dad66

Great stuff, Dave. Thanks for being so responsive to all the feedback!

0.18.1, summarizes a discussion about BIP174 extensions, and notes a
proposal for LN trampoline payments. Our *bech32 sending support*
section this week features a special case study contributed by BRD and
our *notable changes* section highlights several possibly significant
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are we planning on emphasizing the sections moving forward? Was not sure if this was intentional.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, this was a special case requested by @moneyball https://github.com/bitcoinops/bitcoinops.github.io/pull/176/files/07e854c6acc8aaf9ccbce9a7b16972d0ed040833#diff-ef4489117fbc2501b497cd6ec1feb572 (which I agree with). The code changes section is only emphasized because it sounded weird to me to only emphasize the bech32 section (and because it really was a week with lots of changes).

@bitschmidty
Copy link
Copy Markdown
Contributor

tACK 90dad66

@jonatack
Copy link
Copy Markdown
Collaborator

jonatack commented Aug 7, 2019

Thanks Dave, from word one the case study is now a first-person narrative and it feels more personable as well.

@bitschmidty bitschmidty force-pushed the 2019-08-07-newsletter branch from 90dad66 to 82a7452 Compare August 7, 2019 14:17
@bitschmidty bitschmidty merged commit d65d129 into bitcoinops:master Aug 7, 2019
@bitschmidty
Copy link
Copy Markdown
Contributor

58 is filled with a lot of news
@jonatack moved his reviews to tues

Andrew Chow extends BIP174
BRD talks bech32 in breadwallet-core

Thanks again @harding for authoring and @jonatack @adamjonas @jnewbery @moneyball for reviews. Special thanks to @breadwallet for the field report this week.

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