Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 brunoerg commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552838477)
> Yes I think that would be better, otherwise there is no need to add them.

Ok, cool. I will change this PR to draft.
📝 brunoerg converted_to_draft a pull request: "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`"
(https://github.com/bitcoin/bitcoin/pull/27678)
This PR adds `recv_flood_size` and `prefer_evict` in `CNodeOptions` when creating a new CNode in `ConsumeNode`. I noticed they were missing while working on an improvement for net fuzzing.

Checked that #27324 added `recv_flood_size` into `CNodeOptions` and #25962 added`prefer_evict`.
💬 fanquake commented on pull request "fuzz: net, add `recv_flood_size`, `prefer_evict` in `ConsumeNode`":
(https://github.com/bitcoin/bitcoin/pull/27678#issuecomment-1552839469)
I think it can just be closed for now? Can be re-opened when there are relevant changes.
👍 fanquake approved a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690#pullrequestreview-1432431581)
ACK fa3761d19d716c2a5b25c76d092b1afbf65ec1d0
🚀 fanquake merged a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690)
💬 MarcoFalke commented on pull request "fuzz: Print error message when FUZZ is missing":
(https://github.com/bitcoin/bitcoin/pull/27672#discussion_r1197651079)
Thx, done
👍 TheCharlatan approved a pull request: "doc: remove mention of glibc 2.10+"
(https://github.com/bitcoin/bitcoin/pull/27689#pullrequestreview-1432438295)
ACK 7014e080154f9c82e4fbe043af292a1e761cdb55
👍 TheCharlatan approved a pull request: "doc: Rework build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27685#pullrequestreview-1432442207)
ACK fa29651c3ff3e13a42d6505b14971265562f088a
💬 stickies-v commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1197660996)
I'm not sure that's a strict improvement. Documenting the interface (i.e. pretty much just the first line of what's in `net.cpp`) seems more appropriate to be in the header indeed, but all the rest sounds more like implementation details that may best be kept close to the source code?

Also, my main issue is with (having just) the word "find", which to me sounds like there are no side effects.
🚀 fanquake merged a pull request: "doc: remove mention of glibc 2.10+"
(https://github.com/bitcoin/bitcoin/pull/27689)
🚀 fanquake merged a pull request: "msvc: Rename `libbitcoinconsensus` to `libbitcoin_consensus` and other adjustments"
(https://github.com/bitcoin/bitcoin/pull/27615)
💬 hebasto commented on issue "Check usages of `#if defined(...)`":
(https://github.com/bitcoin/bitcoin/issues/16419#issuecomment-1552877492)
On the master branch @ 4e8a7654f623fc0dad933b3d93dc54d8206c605d plus https://github.com/bitcoin/bitcoin/pull/27696, there are only two cases in our build system of explicit unconditional usage of the `AC_DEFINE` and `AC_DEFINE_UNQUOTED` macros (except for one which define string literals like `COPYRIGHT_YEAR`):

- https://github.com/bitcoin/bitcoin/blob/4e8a7654f623fc0dad933b3d93dc54d8206c605d/configure.ac#L1230
- https://github.com/bitcoin/bitcoin/blob/4e8a7654f623fc0dad933b3d93dc54d8206c605
...
💬 hebasto commented on pull request "build: Check usages of #if defined(...)":
(https://github.com/bitcoin/bitcoin/pull/25302#issuecomment-1552880786)
I've re-reviewed this PR and none of the suggested changes seem required to me.

More details see in https://github.com/bitcoin/bitcoin/issues/16419#issuecomment-1552877492.
💬 MarcoFalke commented on issue "Can't compile v24.0.1":
(https://github.com/bitcoin/bitcoin/issues/27680#issuecomment-1552885846)
> Also I didn't quite understand the note and how to specify the absolute path ...

Thanks, I agree that the section about absolute folders is confusing. So I removed it in https://github.com/bitcoin/bitcoin/pull/27685
👋 MarcoFalke's pull request is ready for review: "build: Bump minimum supported GCC to g++-9"
(https://github.com/bitcoin/bitcoin/pull/27662)
💬 MarcoFalke commented on pull request "test: Return dict in MiniWallet::send_to":
(https://github.com/bitcoin/bitcoin/pull/27640#issuecomment-1552892950)
Anything left to do here?
💬 fanquake commented on pull request "build: Fix USDT detection on FreeBSD":
(https://github.com/bitcoin/bitcoin/pull/27458#issuecomment-1552895961)
What are we doing here?

If we aren't doing this yet, I think it can just be closed and/or combined in #26593. Otherwise,
> Maybe adjust the PR title/OP and the commit message.

?
💬 hebasto commented on pull request "bench: Add SHA256 implementation specific benchmarks":
(https://github.com/bitcoin/bitcoin/pull/27598#issuecomment-1552896322)
Rebased on top of the merged #27615 as it was [relevant](https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1541888040).
💬 fanquake commented on pull request "build: Check usages of #if defined(...)":
(https://github.com/bitcoin/bitcoin/pull/25302#issuecomment-1552896958)
Going to close for now. We can continue any discussion in #16419.
fanquake closed a pull request: "build: Check usages of #if defined(...)"
(https://github.com/bitcoin/bitcoin/pull/25302)