-
Notifications
You must be signed in to change notification settings - Fork 133
Newsletters: add 268 (2023-09-13) #1296
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
Conversation
harding
commented
Sep 8, 2023
•
edited
Loading
edited
- Lede, releases/RCs, topic entries @harding
- Bitcoin Core PR Review Club @LarryRuane
- Bitcoin Core 26567 @murchandamus
f5e1516
to
df9de83
Compare
_topics/en/client-side-validation.md
Outdated
|
||
**[Taproot Assets][]**, formerly called **Taro**, is a protocol heavily | ||
inspiried by RGB that uses [taproot][topic taproot]'s commitment | ||
structure to make allow transactions to commit to additional data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
structure to make allow transactions to commit to additional data. | |
structure to allow transactions to commit to additional data. |
_topics/en/client-side-validation.md
Outdated
inspiried by RGB that uses [taproot][topic taproot]'s commitment | ||
structure to make allow transactions to commit to additional data. | ||
Taproot's construction itself derives from pay-to-contract. As the name | ||
suggests, initial protocol developemnt is specifically focused on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggests, initial protocol developemnt is specifically focused on the | |
suggests, initial protocol development is specifically focused on the |
df9de83
to
d798991
Compare
Force pushed for review comments, thanks, @bitschmidty |
d798991
to
3e106fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple quick notes for @LarryRuane, with the overall text looking good.
deserialization that causes the node to reject a consensus-valid | ||
block, also a soft fork." | ||
a3link="https://bitcoincore.reviews/28165#l-45" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although instagibbs calls the <4MB theoretical bug a soft fork, neither glozow nor anyone else I see calls the generalized deserialization problem a soft fork. I'm not sure everything in that class of failures would be a soft fork; e.g., a non-deterministic deserialization failure (kinda similar to the BIP50 failure) wouldn't be a soft fork; more relevant, the type of platform-inconsistent serialization/deserialization behavior that BIP66 surreptitiously fixed isn't something I'd have described as a soft fork if it had been triggered and caused a consensus failure.
In other words, I think we should drop the phrase ", also a soft fork" from the end of this paragraph. (But I don't have strong feelings about it.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, to be designated a soft fork, the behavior change in question must be intentional?
But I'm fine with dropping both mentions of "soft fork" if it's at all confusing, I think the answer will still be understandable and accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, to be designated a soft fork, the behavior change in question must be intentional?
Maybe? But the point I'm trying to make is that, to be designated a soft fork, the behavior needs to be consistent. If the behavior between nodes is inconsistent, it's a consensus failure. We might eventually resolve a consensus failure using a soft fork (e.g., BIP50 resolved the BDB locks inconsistency with a soft fork that temporarily reduced the maximum block size below the level where the maximum lock count was reached, and BIP66 resolved the DER decoding inconsistency with a stricter decoding rule) but my point is that a bug in the (de)serialization code wouldn't necessarily produce a soft fork because it wouldn't necessarily occur consistently between full nodes, so it could cause a consensus failure (and consensus failures are not soft forks, at least in my personal terms sheet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an excellent way to look at it, thanks, @harding!
transport for the next packet to send, and finally calls `m_sock->Send()`." | ||
a5link="https://bitcoincore.reviews/28165#l-83" | ||
|
||
q6="How many bytes are sent over the wire for the `sendtxrcncl`message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: can we mention what sendtxrcncl
message does? I'm parsing it as SendTransactionCancel, but I doubt that's correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems that it is “received from a peer demonstrating readiness to announce transactions via reconciliations.”
might even be required to support three or more different protocols | ||
simultaneously, to the disadvantage of all. | ||
|
||
Sander's summary had not received any comments as of this writing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sander's summary had not received any comments as of this writing. | |
Sander's summary has not received any comments as of this writing. |
This comment is going to set the nittyness record, but the verb "had" slightly implies that it did receive comments after the time of this writing.
030940b
to
0fa0980
Compare
Force pushed to resolve Dave's comments, thanks! (Even though I asked you a follow-up question, I decided to just go ahead and make the suggested changes.) |
_topics/en/client-side-validation.md
Outdated
parties; and a transaction containing the final commitment is published | ||
onchain. | ||
|
||
Only wallets that want to support RGB or Taproot Assets needs to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only wallets that want to support RGB or Taproot Assets needs to | |
Only wallets that want to support RGB or Taproot Assets need to |
a3="A bug that restricts the maximum message size less to than | ||
4MB, which may cause the node to reject a block that other |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a3="A bug that restricts the maximum message size less to than | |
4MB, which may cause the node to reject a block that other | |
a3="A bug that restricts the maximum message size to less than | |
4MB, which may cause the node to reject a block that other |
transport for the next packet to send, and finally calls `m_sock->Send()`." | ||
a5link="https://bitcoincore.reviews/28165#l-83" | ||
|
||
q6="How many bytes are sent over the wire for the `sendtxrcncl`message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it seems that it is “received from a peer demonstrating readiness to announce transactions via reconciliations.”
0fa0980
to
c30f518
Compare
Force pushed for Murch's comment, good catch, thanks! |
Pushed edits for all review comments (thanks @bitschmidty @murchandamus @LarryRuane!), added lede, checked for updates to the releases/RCs (no updates), and reviewed contributions (including updates) from @LarryRuane and @murchandamus (thanks!). Also added topic entries. Thanks everyone! |
ce3ce89
to
deb9aea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fixups
Squashed and merged 🚀 |