This week in Bitcoin Core
Bitcoin Core contributor kevkevin covers some of the latest updates...
Hello 👋 folks, I'm kevkevin. I'm an open-source developer and reporter for Insider Edition. Last week, I reviewed several pull requests from the Bitcoin Core repo. Here’s what I found notable.
Merged PR’s
Every week, several changes are officially added to Bitcoin Core. This week, 21 changes were merged. Here are some I thought were interesting from this week.
p2p: TxOrphanage revamp cleanups by glozow
This pull request is a follow-up to this pull request. This pull request adds some optimizations, like automatically trimming the orphanage when needed. It also removes duplicate reconsiderations and only marks a reconsideration for 1 peer at a time. There are other, more minor changes as part of this follow-up.init: make -blockmaxweight startup option debug only by ismaelsadeeq
This was a pull request that was opened to make the blockmaxweight option debug-only. Initially, it was ConceptNack’d by ajtowns, but shortly after, the confusion was cleared up.
The reason why this option was decided to be put to debug only is that it is unlikely to be used on mainnet because of the new addition of blockreservedweight. But blockmaxweight may still be useful on testnet or signet.cmake: Install internal binaries to <prefix>/libexec/ by ryanofsky
This change is meant to seperate internal and external binaries. Internal meaning binaries that are not normally invoked by users. For example external binaries might be bitcoind and bitcoin-cli. Some internal ones being test_bitcoin, test_bitcoin-qt and bench_bitcoin.
The binaries are “no longer on the system PATH. However, they can still be invoked from the libexec/ directory”. Overall this change removes normally unused binaries from a users PATH
Open PR’s
There are always changes getting updated and reviewed in real time. Here are some notable PR’s that are still up and looking for reviews.
Inspired by #13922 and #32959, both of which are closed, the goal of this pull request is to lower the default minimum transaction fee in order to relay to the rest of the network.
blockmintxfee has been changed to 1 satoshi per kvB. minrelaytxfee and incrementalrelayfee have been changed to 100 satoshis per kvB. All other minimum fee rates remain the same. You can review the release notes to fully understand the proposed changes to the default values.
Changing policy has been a bit controversial the last few months, so naturally, this PR has received some NACKs, but overall, there seem to be more ACKs.validation: detect witness stripping without re-running Script checks by darosior
This pull request is an optimization to script validation. Before this change, the detection of a stripped witness would require running the script checks 3 times. Which then, in the worst case, requires running the script validation for every single input 3 times.
It is, however, not necessary to run the script validation to detect the stripped witness.
There are 3 types of witness program: defined program types (Taproot, P2WPKH and P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus). For undefined program types, Script validation is always going to fail regardless of the witness (by standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation.
IRC meeting notes
Every week on Thursday, there is an IRC meeting. Here are some short notes from that meeting.
glozow: Orphan resolution can be removed from the WGs (working groups)
Kernel WG Update (TheCharlatan)
TheCharlatan: I’ll pass this week
Cluster Mempool WG Update (sdaftuar, sipa)
sipa: getting close!
sipa: (to the start of the actual race, getting #28676 in)
sipa: i'm working on a small PR to improve/control/observe memory usage in TxGraph, but review can shift to sdaftuar's PR now
sipa: i had a few ideas on improving SFL too during my offline vacation too, will post about in due time, nothing urgent about that
MuSig2 WG Update (achow101)
v30.0 feature freeze
achow101: feature freeze is in 2 weeks
achow101: Anything to add or remove from the milestone?
darosior: i'd really like to have #33050 and #32473 in for 30. #32579 would be nice, but far from critical.
dergoegge: probably #33106 ?
lightlike: should #33106 be in the milestone?
achow101 says they added those PR’s
fanquake: i don't see what is relevant to 30 about #33021 or #32975 ? Seems general, and will distract review from more important things? It would be good if it's clearer why #32579 needs to go into 30, or not
l0rinc: fanquake: first is a test that was dead for some time and we've modified related code in this release; second is a very common complaint for people that they don't know why IBD is suddenly getting slow.
hodlinator: fanquake: Will look into making it more clear. But essentially the release process will probably bump some values which will make tests cover less realistic behavior. Sorry for the redundancy.
should #33106 be backported before 29.1 is released (darosior)
darosior: So it seems like we agree it should be on the milestone for 30, but what do people think of backporting it to 29.1 (currently in rc1)?
glozow: There will be an rc2 fosho
TheCharlatan: makes sense imo
Murch[m]: with such a big portion of transactions being below 1 s/vB it seems reasonable to me
darosior: I initially thought it was too late for .1 and it'd have to go in .2, but maybe that's too much churn for release managers? What do maintainers say
glozow: ^that was me saying it can go in .1
Murch[m]: Should be tested a bit more, though, especially if we not just roll it out to the new release but also backport it
darosior: (I also think it should be backported, that's what i'm bringing it up)
glozow: It doesn't seem like something we'd normally backport
darosior: @Murch[m] yeah, still no review. I've been going through it this morning but definitely needs more eyes to be sneaked into post rc1
achow101: we don't usually backport features
fanquake: this isn't a feature though? it's fixing compact block relay. if we do backport it to 29, then it should also go to 28
dergoegge: since block relay is degraded, it makes sense to backport
glozow: I agree it's closer to a bug fix than a feature
lightlike: yes, an adaptation to changed fee condition, so it's a fix
sipa: we have treated softfork activations as "bug fixes", which is why they tend to be in .1 releases... in the sense that network conditions have changed, and bitcoin core needs to adapt to those
darosior: I guess this boils down to "how much would backporting it in 29.1 provide a relief for the current network conditions?" which i gets boils down to whether we expect 29.1 to be out before 30.
achow101: uptake of minor releases after a major release is made is usually not that high
fanquake: if this pr hadn't come up, 29.1 likely would have been out within a week or so and I'd say adoptoin of 29 is currently blocked somewhat, on the existence of the .1. @Murch[m] Also, just a few nodes rolling out with the new mempool policy will probably drastically alleviate the issue for any nodes that upgrade
fanquake: @sipa and 29.2, if any, likely comes after 30.0?
fanquake: likely
achow101: @sipa yeah
sipa: i think it's not unreasonable to backport a change like this given the network conditions, but whether it's worth doing does seem to depend on the realistic timing of getting the release out
There was more discussion on the timing of the release’s which you can find in the full discussion
v29.1rc1 testing (fanquake)
Not too much testing has been done post-release
Follow this link to access the full discussion
PR review club (IRC)
It’s not every week that we have a PR review club. This month’s review club, hosted by ryanofsky was about wallet: Add exportwatchonlywallet RPC by achow101.
The participants in the IRC chat were pretty positive towards this change, 0 Nacks, 2 Concept Acks, and 1 light review ACK. Most of the conversation was technical, but to read the full discussion, it is linked below.
link to full conversation
Releases
No releases this week
Thank you for reading. Be sure to tune in again next week for your updates on Bitcoin Core!