Clarifying Moderation Policies for External Behavior - This Week in Bitcoin Core #47
Delete Libevent and make coin selection faster...
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.
This week in the IRC meeting, there was a topic brought up about how Bitcoin Core should moderate users whose external presence is abusive, but within the Bitcoin Core repos, they follow the guidelines.
Merged PR’s
Every week, several changes are officially added to Bitcoin Core. This week, multiple changes were merged. Here are some I found interesting this week.
rpc: make getprivatebroadcastinfo and abortprivatebroadcast fail if privatebroadcast is not enabled by polespinasa
This change is conceptually pretty straightforward. There was a new feature added to Bitcoin Core v31.0 called private broadcast, in which two RPC calls were added for getprivatebroadcast and abortprivatebroadcast.
Now it makes sense to have them enabled when we also have private broadcast enabled. The issue is that before this PR that was not the case. So, what Pol Espinasa made a condition was that if -privatebroadcast=0 then we would disable those RPC methods.p2p: Prevent integer overflow in LocalServiceInfo::nScore by codeabysss
This change by codeabysss also their first change, which closes an issue from 2022.The overflow for signed arithmetic yields undefined behavior.
This changes prevents undefined behavior in local address scoring by saturatingnScoreupdates atINT_MAXin bothSeenLocal()andAddLocal()update paths.
There are always changes being updated and reviewed in real-time. Here are some notable PR’s that are still up and looking for reviews.
refactor: Add util::Result failure types and ability to merge result values by ryanofsky
Add `util::Result` support for returning more error information and make use of it in [LoadChainstate method](https://github.com/bitcoin/bitcoin/pull/25665/commits/d0d2535cac193a448395fbb1d32df072b2e491a3) as an initial application. Followup PRs [#25722](https://github.com/bitcoin/bitcoin/pull/25722) and [#29700](https://github.com/bitcoin/bitcoin/pull/29700) use it more broadly to return errors and warnings from wallet and kernel functions as well.
IRC meeting notes
Every week on Thursday, there is an IRC meeting. Here are some short notes from that meeting.
stickies-v: #topic Kernel WG Update (sedited)
sedited: nothing to share this weekstickies-v: #topic Benchmarking WG Update (l0rinc, andrewtoth)
sliv3r__: hi
andrewtoth: l0rinc opened #35465 to address the compaction.
andrewtoth: willcl-ark discovered a speed regression https://github.com/bitcoin/bitcoin/issues/35457. l0rinc is looking into ways to resolve that.
andrewtoth: Got some good review on #35295. We are also now continuously fuzzing leveldb along with concurrent reads, which is used for chainstate reads in that PR. More review welcome.
andrewtoth: that’s itstickies-v: #topic Fuzzing WG Update (dergoegge)
dergoegge: no update
stickies-v: I don’t see any other WG leaders here, so that concludes the WG section unless anyone pops in last-minute
stickies-v: Anything else to discuss?
andrewtoth: Can we consider merging https://github.com/bitcoin-core/bitcoincore.org/pull/1255 and making a mailing list post?
sedited: not sure if that makes sense without a release.
andrewtoth: Do we want to let users know about workarounds before a release?
stickies-v: alerting earlier seems useful to me too, especially since the vulnerability is public already anyway
sliv3r__: I think it makes sense anouncing before a release
andrewtoth: I don’t think that blog post would make sense as is with a release, since the best workaround would be to upgrade
sliv3r__: agree, didn’t read in detail the post but it basically gives workarounds until a fix is released, makes sense to publish it as soon as it is ready
stickies-v: i think it would be useful if more contributors at least concept ack’d blog posts, not that this one seems controversial
sedited: ok
eugenesiegel: should this not have been public? afaict the security mailing list isn’t for privacy issues. anybody have an idea on the preferred way to handle this?
andrewtoth: all things considered it would be better IMO if this were not yet public
andrewtoth: but, not sure how we would have made a fix without revealing it
instagibbs: there are ways of doign so, but horse is out of the barn long ago
andrewtoth: since it is public, it would be good to get info about it out
stickies-v: alright, looks like we’re done here. thanks for bringing it up andrewtoth
andrewtoth: what is holding up a 31.1 release?
Murch[m]: 31.1 should be out soon, right?
stickies-v: (okay not done)
Murch[m]: I just took a look at the milestone, all the todos seem to be checked off
dergoegge: eugenesiegel: This was fine to be public imo
sedited: I think we were waiting on #35465
Murch[m]: Ah sorry, the list of backports refers to merged PRs, but the PR is still draft
Murch[m]: https://github.com/bitcoin/bitcoin/pull/35331
andrewtoth: #35331
andrewtoth: Please review 35465 then
stickies-v: anything else here? otherwise i’m wrapping up
Murch[m]: I though https://github.com/bitcoin-core/meta/issues/46 was an interesting question.
Murch[m]: While it seems reasonable to judge code contributions on their own merit, it also seems reasonable that extremely uncharitable behavior in other avenues should at some point be addressed.
Murch[m]: Anyway, if people haven’t seen that conversation yet, maybe take a look
Murch[m]: :s/though/thought/
instagibbs: thanks for bringing it up
Murch[m]: Like, more concretely, if someone has been accusing us (without evidence) of crimes and shitting on our project in public for years, why should we continue to platform them in our repository?
Murch[m]: I can’t think of any other situation in life where this sort of behavior would be acceptable
stickies-v: so if someone is an asshole in real life, but contributes a good patch, we should not merge it because they’re an asshole?
stickies-v: that seems unworkable to me
Murch[m]: I don’t think your description of the situation matches what’s going on
Murch[m]: Anyway, I think that a ban would be in order. I don’t understand why we continue to subject ourselves to this person.
stickies-v: maybe i missed a recent conversation or incident but i’m not getting it
stickies-v: happy to review an issue or PR if there is one, i just subscribed to the meta repo
stickies-v: it looks like we’re done here, so i’ll wrap upRead here for the full meeting
Releases
No releases this week
Thank you for reading. Be sure to tune in again next week for your updates on Bitcoin Core!
If there are any comments, suggestions, or errors, do not hesitate to reach out or comment