Newsletters: add 131 (2020-01-13) #503
Conversation
8535749 to
001ad65
Compare
There was a problem hiding this comment.
Looks great so far.
I have strong opinions weakly held on bech32m, but maybe I'm too close to the proposal. My first reaction was to suggest a more detailed description of the bech32m proposal, but the more I tried coming up with concrete suggestions, the more I like it as a high-level description.
|
|
||
| - **Bech32m** Pieter Wuille [posted][wuille bech32m post] to the | ||
| Bitcoin-Dev mailing list a [draft BIP][bech32m bip] for a modified | ||
| version of the [bech32][topic bech32] algorithm that increases the |
There was a problem hiding this comment.
Optional:
| version of the [bech32][topic bech32] algorithm that increases the | |
| version of the [bech32][topic bech32] address encoding that increases the |
| for [taproot][topic taproot] addresses and possibly future new script | ||
| upgrades. |
There was a problem hiding this comment.
Let's emphasize this, because providing forward-compatible address types was one of the main design goals of bech32/bech32m.
| for [taproot][topic taproot] addresses and possibly future new script | |
| upgrades. | |
| for [taproot][topic taproot] addresses and future native segwit address versions. |
There was a problem hiding this comment.
Although "future native segwit address versions" is more precise, and so allows dropping the "possibly", I worry there are a lot of readers who don't really understand what a new address version implies. E.g., someone who went through the change from P2PKH to P2WPKH might think it's just a different way of encoding the same data, whereas the thing I think we want to communicate is that bech32m will be compatible with even major changes to script (e.g., you could in theory use bech32m to pay a Simplicity-based encumbrance).
There was a problem hiding this comment.
Alright, no worries. 😃
| q3="Starting from what number of inbound peers does eviction begin to occur?" | ||
| a3="Starting from 21 inbound peers, as at least 20 are protected by the | ||
| current logic: 4 by network group, 4 that sent us novel blocks passing | ||
| initial validity checks, 4 that sent us novel transactions accepted into |
There was a problem hiding this comment.
I hesitated to add "(with backup criteria)" to the blocks and the transactions here, or to link to the functions, and whether or not to mention the 8 non-tx-relay peers that sent us novel blocks to account for the difference between the minimum 20 and maximum 28 protected inbounds.
There was a problem hiding this comment.
I do find the difference between 28 in the previous item and 20 in this item to be confusing. Maybe s/28 peers/20 inbound peers/ in the previous item?
There was a problem hiding this comment.
Maybe s/Up to 28/Between 20 and 28 inbound/ with the positions of q2 and q3 inversed
|
|
||
| - [Bitcoin Core 0.21.0rc5][Bitcoin Core 0.21.0] is a release candidate | ||
| for the next major version of this full node implementation and its | ||
| associated wallet and other software. |
There was a problem hiding this comment.
Would we consider adding the 0.21 testing guide?
| message that could be sent during connection negotiation. A peer | ||
| that understands the message won't send the node requesting | ||
| `disabletx` any transaction announcements and won't request any | ||
| transactions from the node. It also won't send some other gossip |
There was a problem hiding this comment.
maybe s/It also won't/The BIP also recommends that it not/ as the BIP draft states: "It is RECOMMENDED that a node that has sent or received a disabletx message to/from a peer not send any of these messages to the peer: addr/getaddr, addrv2 (BIP 155)"
| Bitcoin Core [later][bitcoin core #6993] reused this mechanism for its | ||
| bandwidth-saving `-blocksonly` mode where a node requests that its | ||
| peers don't send it any unconfirmed transactions at all. Last year, | ||
| Bitcoin Core with default settings began opening a few |
There was a problem hiding this comment.
maybe s/a few/up to two/
(it is now currently up to three, or a bit more than two, if the temporary block relay connections added in bitcoin/bitcoin#19858 are counted)
| a2link="https://bitcoincore.reviews/20477#l-83" | ||
|
|
||
| q3="Starting from what number of inbound peers does eviction begin to occur?" | ||
| a3="Starting from 21 inbound peers, as at least 20 are protected by the |
There was a problem hiding this comment.
s/protected/protected from eviction/, especially if this question is inverted with the previous one as suggested in #503 (comment)
| must be taken to avoid exposing the node to attacker-triggered network | ||
| partitioning with this logic, so this code is critical. |
There was a problem hiding this comment.
I'm not sure about this sentence. Our outbound connection logic is much more important for partition resistance.
There was a problem hiding this comment.
It's from https://github.com/bitcoin/bitcoin/blob/7b975639ef93b50537a3ec6326b54d7218afc8da/src/net.cpp#L969, though not necessarily viable content in this context.
| a1="To make inbound slots available for honest peers in the network so that | ||
| new nodes can establish good outbound connections to them. Otherwise, | ||
| dishonest nodes could more easily attack new nodes by being available for | ||
| their outbound connections and by statically occupying inbound slots." |
There was a problem hiding this comment.
What does 'statically occupying inbound slots' mean? Do you mean occupy all the inbound slots of other listening nodes?
There was a problem hiding this comment.
Yes. I was trying to convey the idea of a more static network topology in the absence of evictions.
| valid transactions and blocks, and a few others. The longest-connected | ||
| remaining half are protected, including some onion peers. Of those that | ||
| remain, the youngest member of the network group with the most | ||
| connections is selected for disconnection. An attacker would have to be | ||
| simultaneously better than honest peers at all of these characteristics | ||
| to partition a node." |
There was a problem hiding this comment.
This seems like a little bit too much detail. I'd suggest perhaps giving a higher-level answer here.
There was a problem hiding this comment.
This highlights a limitation of the review club format. I felt I was overly simplifying and yet don't touch the changes made by the PR other than "adds test coverage," because the details of the PR itself aren't particularly newsworthy or quiz-worthy.
There was a problem hiding this comment.
I'm not worried about excess detail when the answer content is hidden by default and readers need to click to expand.
| to partition a node." | ||
| a2link="https://bitcoincore.reviews/20477#l-83" | ||
|
|
||
| q3="Starting from what number of inbound peers does eviction begin to occur?" |
There was a problem hiding this comment.
I think this question is confusing. We only ever evict a peer if our inbound slots are full, and by default a listening node would have 115 inbound slots. Configuring a node to only have 20 inbound slots would be highly unusual config.
I pointed the same thing out in the PR: bitcoin/bitcoin#20477 (comment). I don't think it's a useful test, and is more confusing than anything else.
I suggest just removing this question.
There was a problem hiding this comment.
I think removing this Q/A also resolves the confusion in #503 (comment) between 20 vs 28.
There was a problem hiding this comment.
I agree. I'm going to remove this.
| transactions from the node. It also won't send some other gossip | ||
| messages such as `addr` messages used for IP address announcements. | ||
| The `disabletx` negotiation persists for the lifetime of the | ||
| connection, allowing peers to use different limits for disabled |
There was a problem hiding this comment.
This is probably my lack of understanding, but what is meant by "different limits" here? It is not clear to me.
| {% include functions/details-list.md | ||
| q0="Inbound or outbound peer selection: which one is our primary defense | ||
| against eclipse attacks?" | ||
| a0="Outbound peer selection, as outbound peers are far less under attacker |
There was a problem hiding this comment.
(?) s/far less under attacker/far less likely to be under attacker
|
ACK @adamjonas PR (sans this comment https://github.com/harding/bitcoinops.github.io/pull/20/files#r555978996) |
| upgrades. This will allow wallets and services that adopt bech32m to | ||
| automatically be able to pay future wallets and services that upgrade | ||
| to taproot and many other proposed improvements (see [Newsletter | ||
| #45][news45 bech32 upgrade] for details). |
There was a problem hiding this comment.
Good addition content-wise, but after bech32 always appeared with the qualifier 'address' prior, "adopt bech32m" here might make people think that they have to use bech32m addresses first before being able to send to others using it. How about:
| upgrades. This will allow wallets and services that adopt bech32m to | |
| automatically be able to pay future wallets and services that upgrade | |
| to taproot and many other proposed improvements (see [Newsletter | |
| #45][news45 bech32 upgrade] for details). | |
| upgrades. Wallets and services implementing support for the bech32m address encoding will be able to pay future adopters of taproot and many other proposed improvements (see [Newsletter | |
| #45][news45 bech32 upgrade] for details). |
_topics/en/bech32.md
Outdated
| After a [problem][bech32 weakness] was discovered with bech32 error | ||
| detection for future upgrades under some rare circumstances, a new | ||
| bech32 modified (**bech32m**) format was proposed. The new format is | ||
| not backwards compatible with the original bech32 format, although it |
There was a problem hiding this comment.
| not backwards compatible with the original bech32 format, although it | |
| not forwards compatible with the original bech32 format, although it |
The new interpreter can resolve data written using the old schema: backwards compatible.
The old interpreter can resolve data written using the new schema: forwards compatible.
Bech32 was supposed to be forwards compatible for all native segwit versions, but bech32m now explicitly breaks forwards compatibility.
| - [Bitcoin Core #19055][] muhash FIXME:adamjonas | ||
| - [Bitcoin Core #19055][] adds the Muhash algorithm for future implementation | ||
| to calculate the UTXO set hash in `gettxoutsetinfo`. As covered in | ||
| [newsletter 123][muhash review club], MuHash is a rolling hash algorithm |
There was a problem hiding this comment.
nice idea linking to the previous review club feature
| - [Bitcoin Core #19055][] adds the MuHash algorithm so that a future implementation | ||
| can use it. As covered in |
There was a problem hiding this comment.
"so that a future implementation can use it" is a bit tautological. I think the rest of the write-up explains well why and how this can be used, so how about:
- [Bitcoin Core #19055][] adds a C++ implementation of the [MuHash algorithm](https://cseweb.ucsd.edu/~mihir/papers/inchash.pdf).
as the first sentence?
There was a problem hiding this comment.
@jnewbery hopefully better now? It's still a bit of a tautology, but I do want to make it clear that there's no way for anyone to use the feature yet (excluding tests).
| Control Protocol), the backward-compatible successor to NAT-PMP. | ||
|
|
||
| - [Bitcoin Core #19055][] adds the MuHash algorithm so that a future implementation | ||
| can use it. As covered in |
There was a problem hiding this comment.
@adamjonas FYI, I dropped the mention of MuHash being used in gettxoutsetinfo here. That was correct (AFAIK) but I think the main goal for the hash function is to be able to quickly and automatically calculate the hash after every block has been processed, and then store the hash for that block in a database that can be queried later so that everyone can agree that the hash of the UTXO set at block xxxx was yyyy, so an assumeutxo state that also (mu)hashes to yyyy can be reasonably trusted.
By comparison, adding MuHash to gettxoutsetinfo is just an intermediate step that slots the new hash function into a place where we currently have a function that's too slow for practical every-block tracking.
Your text alludes to that ultimate purpose later on, so I think it's reasonably descriptive and that mentioning gettxoutsetinfo could accidentally confuse readers and make them think this merged PR was less important than it actually was.
|
|
||
| Most of the discussion focused on understanding Bitcoin Core's peer eviction | ||
| logic. | ||
|
|
||
| {% include functions/details-list.md | ||
| q0="Inbound or outbound peer selection: which one is our primary defense | ||
| against eclipse attacks?" | ||
| a0="Outbound peer selection, as outbound peers are far less under attacker | ||
| a0="Outbound peer selection, as outbound peers are far less likely to be under attacker |
There was a problem hiding this comment.
The changes in 7d8b09f LGTM except maybe this one, as it changes the meaning; my thinking was that "outbound peers are far less under attacker control" because we choose them, but maybe I'm misinterpreting and it could probably be better expressed.
There was a problem hiding this comment.
Maybe just "are less under"
|
Thanks @harding for the latest update. |
bae280e to
bfa73fa
Compare
|
@bitschmidty some of the latest changes are missing from the last push. |
bfa73fa to
2c9007e
Compare
|
ACK 2c9007e |
Bitcoin Core #18077@xekyoBitcoin Core #19055@adamjonasC-Lightning #4320@dongcarlNote: this PR adds an old topic entry to eclipse attacks, which were described in an entry but didn't explicitly say "eclipse" so I didn't notice it when creating the Eclipse Attack topic. Relevant this week because we're linking to it.