Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 fanquake commented on pull request "Fix typos in 36 files | Almost only documentation":
(https://github.com/bitcoin/bitcoin/pull/30188#issuecomment-2136799942)
Thanks, however most of these are in upstream code (subtrees, macros, Doxygen config), or code we aren't going to change (like the release notes). You'll need to remove all of those changes, and then also write a proper commit message.
💬 Sajjon commented on pull request "Fix typos in 36 files | Almost only documentation":
(https://github.com/bitcoin/bitcoin/pull/30188#issuecomment-2136803187)
> Thanks, however most of these are in upstream code (subtrees, macros, Doxygen config), or code we aren't going to change (like the release notes). You'll need to remove all of those changes, and then also write a proper commit message.

👍 thx! Will give it another go later tonight. Any suggsetion on what a "proper commit message" would be?
💬 dergoegge commented on pull request "fuzz: increase `txorphan` harness stability":
(https://github.com/bitcoin/bitcoin/pull/30186#issuecomment-2136805200)
> I am not sure if a three line diff is worth a scripted diff

Just for context, I suggested it to marco as a learning exercise.
💬 fanquake commented on pull request "Fix typos in 36 files | Almost only documentation":
(https://github.com/bitcoin/bitcoin/pull/30188#issuecomment-2136808686)
> Any suggestion on what a "proper commit message" would be?

https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md.
📝 fanquake converted_to_draft a pull request: "Fix typos in 36 files | Almost only documentation"
(https://github.com/bitcoin/bitcoin/pull/30188)
Fix typos in 36 files
💬 maflcko commented on pull request "fuzz: increase `txorphan` harness stability":
(https://github.com/bitcoin/bitcoin/pull/30186#issuecomment-2136811524)
Sure, sounds good either way. Happy to review once CI is green and the UB is removed.
🚀 fanquake merged a pull request: "[27.x] Backports and rc1"
(https://github.com/bitcoin/bitcoin/pull/30092)
🤔 glozow reviewed a pull request: "Fix typos in 36 files | Almost only documentation"
(https://github.com/bitcoin/bitcoin/pull/30188#pullrequestreview-2084660953)
Thanks for your interest in contributing. Maybe take a look at https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#pull-request-philosophy.

Fwiw I think editing archived release notes (especially changing the names of pull requests) just makes them inaccurate. I think this should be closed.
🚀 fanquake merged a pull request: "refactor: Use type-safe time in txorphanage"
(https://github.com/bitcoin/bitcoin/pull/30170)
💬 fanquake commented on pull request "fuzz: More accurate coverage reports":
(https://github.com/bitcoin/bitcoin/pull/30156#discussion_r1618452097)
Might followup here.
🚀 fanquake merged a pull request: "fuzz: More accurate coverage reports"
(https://github.com/bitcoin/bitcoin/pull/30156)
💬 maflcko commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1618469965)
nit: Not sure about using the global for new code. I'd say it is cleaner to replace the SeedInsecureRand with a local FastRandomContext and then call the member methods directly. E.g `frc.randrange(64)`, etc
🚀 fanquake merged a pull request: "build: LLD based macOS toolchain"
(https://github.com/bitcoin/bitcoin/pull/21778)
💬 fanquake commented on pull request "fuzz: Handle missing BDBRO errors":
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618477590)
@achow101 can you followup here? Would be good to merge this to avoid the fuzzing issues.
💬 fanquake commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2136876648)
What is the status of this? I think it'd be easiest to just drop the contrib changes. Also no PR description now?
👋 fanquake's pull request is ready for review: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
💬 achow101 commented on pull request "fuzz: Handle missing BDBRO errors":
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618509267)
Removed
👍 dergoegge approved a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084802181)
reACK 9ddf39dd87a3729ceedaa05a207621a02c532536
💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1618525186)
The newly added `int auto_outbounds = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS, nUserMaxConnections);` seems to be duplicating this logic, but not exactly:

https://github.com/bitcoin/bitcoin/blob/be100cf4c77a3e45750773689e0a396fda39d8a7/src/net.h#L1069-L1073

and I think this is getting complicated again. I would suggest to keep it simple - drop the last commit f569b06a53851f844219cd80cd33676470e3b776 `init: fix min required fd
...
👍 TheCharlatan approved a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084829846)
ACK 9ddf39dd87a3729ceedaa05a207621a02c532536