[29.x] 33106 backport and final changes for rc2 #33226
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33226. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
ed44446 to
2661d2f
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
2661d2f to
c9d060f
Compare
|
What is the criteria for backporting PRs to point releases? Sub 1 s/vB standardization is certainly not a bugfix. |
Github-Pull: bitcoin#33106 Rebased-From: e5f896b
Github-Pull: bitcoin#33106 Rebased-From: 85f4988
Github-Pull: bitcoin#33106 Rebased-From: 72dc184
Github-Pull: bitcoin#33106 Rebased-From: 1fbee5d
Github-Pull: bitcoin#33106 Rebased-From: d6213d6
Back when we implemented coin age priority as a miner policy, miners mempools might admit transactions paying very low fees, but then want to set a higher fee for block inclusion. However, since coin age priority was removed in v0.15, the block assembly policy is solely based on fees, so we do not need to apply minimum feerate rules in multiple places. In fact, the block assembly policy ignoring transactions that are added to the mempool is likely undesirable as we waste resources accepting and storing this transaction. Instead, rely on mempool policy to enforce a minimum entry feerate to the mempool (minrelaytxfee). Set the minimum block feerate to the minimum non-zero amount (1sat/kvB) so it collects everything it finds in mempool into the block. Github-Pull: bitcoin#33106 Rebased-From: 5f2df0e
Github-Pull: bitcoin#33106 Rebased-From: 3eab8b7
Use a virtual size of 1000 to keep precision when using a feerate (which is rounded to the nearest satoshi per kvb) that isn't just an integer. Github-Pull: bitcoin#33106 Rebased-From: 457cfb6
… explicit Github-Pull: bitcoin#33106 Rebased-From: 2e515d2
…t/kvB Let's say an attacker wants to use/exhaust the network's bandwidth, and has the choice between renting resources from a commercial provider and getting the network to "spam" itself it by sending unconfirmed transactions. We'd like the latter to be more expensive than the former. The bandwidth for relaying a transaction across the network is roughly its serialized size (plus relay overhead) x number of nodes. A 1000vB transaction is 1000-4000B serialized. With 100k nodes, that's 0.1-0.4GB If the going rate for commercial services is 10c/GB, that's like 1-4c per kvB of transaction data, so a 1000vB transaction should pay at least $0.04. At a price of 120k USD/BTC, 100sat is about $0.12. This price allows us to tolerate a large decrease in the conversion rate or increase in the number of nodes. Github-Pull: bitcoin#33106 Rebased-From: 6da5de5
Release notes are from 18720bc
c9d060f to
0034dcf
Compare
Any moderate or severe performance degradation is worth considering for backport, if it can be done cleanly, and has been done many times. Block propagation slowdowns that are avoidable are withing scope. Updating minor versions is much easier than main for production environments. having not reviewed the backport effort here, concept ack |
There was a problem hiding this comment.
ACK 0034dcf
Went through the commits to confirm they match. The only differences are the addition of the datacarriersize argument in feature_rbf.py and the additional changes in mempool_limit.py. Also ran unit and functional tests on 29.x branch locally.
|
This needs |
65dc198 doc: update example bitcoin conf for 29.1rc2 (fanquake) Pull request description: Followup to #33226. ACKs for top commit: dergoegge: ACK 65dc198 willcl-ark: ACK 65dc198 Tree-SHA512: b2924783dd98890bd031dbca8c9c126cd3ab45c3cc8d2f14dd5b5f940fcc7061f3d1f73e2d36482afceaae786f3087b59baab98db0f10bc0d19e3f016f52851a
Backports #33106 and includes final changes for 29.1rc2. Based on current network conditions (in which nodes rejecting 0.1-1sat/vB are missing many transactions), it is recommended to change these policy settings.
I did not include #32750 because it causes #33177 and I don't foresee any problems; it was just a nice to have.
For reviewers: the backport is unclean but fairly straightforward. I just had to adapt a test that is no longer in master (#32973) and include
-datacarriersizein order to pad transaction size (#32406).