💬 brunoerg commented on pull request "fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction":
(https://github.com/bitcoin/bitcoin/pull/31203#issuecomment-2451947424)
cc: @maflcko

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825871071)
This was a good suggestion, thanks -- with the introduction of changesets, we can make `RemoveStaged()` a private method now, and so I updated the commit message to explain that the public removal methods may not be invoked while a changeset is outstanding.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825871365)
Gone thanks

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825871510)
Done

💬 sr-gi commented on pull request "ci: Do not error on unused-member-function in test each commit":
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2451951381)
> Can you add a dummy commit to trigger the CI, so that the flag difference can be seen, and to confirm that there are no unwanted interactions?

Done, will drop + force-push once CI runs

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825876761)
Because if you remove the parent from the reconciliation set but not the children, it could be the case that you reconcile with that peer before fanning out the parent, and all those children will be orphan

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825888636)
Because it is doing more than just moving the code around, so I thought it may be easier for reviewers already familiar with the original approach to understand the changes. I wouldn't mind squashing it once all the discussion around `RelayTranaction` is resolved

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825893170)
Happy to consider an alternative approach that allows us to check there are no collisions without having to expose a public method if you have something in mind

⚠️ pinheadmz opened an issue: "Source code mapping for debugger has changed since cmake"
(https://github.com/bitcoin/bitcoin/issues/31204)
developer-notes.md says:


>1. Configure source file mapping.
>
>For `gdb` create or append to `.gdbinit` file:
>```
>set substitute-path ./src /path/to/project/root/src
>```
>
>For `lldb` create or append to `.lldbinit` file:
>```
>settings set target.source-map ./src /path/to/project/root/src
>```

But I found I needed to create a `.lldbinit` file with these lines:
```
settings append target.source-map build/src/test/ /Users/matthewzipkin/Desktop/work/bitcoin/
settings append
...

💬 maflcko commented on pull request "build: Make G_FUZZING constexpr, require -DBUILD_FOR_FUZZING=ON to fuzz":
(https://github.com/bitcoin/bitcoin/pull/31191#issuecomment-2451988229)
> this approach means libraries built for fuzz testing do not function correctly if used in a release, and libraries built for releases are mostly useless for fuzz testing.

I don't think it is a supported use-case to take libraries from one build (with different build options) and drop them into another build (with different build options) and expect it to work, or be a supported use-case. This has also been the case up until two weeks ago, before commit 9f243cd7fa6654e3b71ba6bff82cceed547c5d
...

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825901123)
Ok, given new interface seems impossible to hit. Added another test case to `ComapreChunks` to just make sure it was well-defined:

```
diff --git a/src/test/rbf_tests.cpp b/src/test/rbf_tests.cpp
index c6d06c937c..d610661878 100644
--- a/src/test/rbf_tests.cpp
+++ b/src/test/rbf_tests.cpp
@@ -631,20 +631,32 @@ BOOST_AUTO_TEST_CASE(feerate_chunks_utilities)
{
// Sanity check the correctness of the feerate chunks comparison.

// A strictly better case.
std::vector<Fee
...

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825906517)
history is cleaner, thank you

💬 maflcko commented on pull request "fuzz: fix `implicit-integer-sign-change` in wallet_create_transaction":
(https://github.com/bitcoin/bitcoin/pull/31203#issuecomment-2452007044)
lgtm ACK 5a26cf7773efdc316ef8235c541ad39e230cd10a

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1825913918)
I don't think this comment is accurate, since this can never be the case. A transaction considered for relay cannot have in-mempool descendants by definition, since they would have been orphans at the time of considering them.

I cannot recall if this is a re-word of the previous approach, which tried to consider descendants, or I tried to refer to the collisions and it did;t make much sense.

I'll remove the comment

💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#issuecomment-2452011156)
ACK 9512bd34a3d74d15c3ce1086a80a1edfa5ac3acf

via `git range-diff master e2324779781b695024d3b17340001fa2da5d3c7f 9512bd34a3d74d15c3ce1086a80a1edfa5ac3acf`

💬 hebasto commented on pull request "depends: bump boost to 1.86.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2452015248)
FWIW, Debian provides `boost_headers-config.cmake`:
- https://packages.debian.org/bookworm/amd64/libboost1.81-dev/filelist
- https://packages.debian.org/trixie/amd64/libboost1.83-dev/filelist

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825925061)
Done.

💬 sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1825925932)
Maybe worth a separate PR to ensure the test coverage.

💬 maflcko commented on pull request "ci: Do not error on unused-member-function in test each commit":
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2452025672)
This pull: https://github.com/bitcoin/bitcoin/actions/runs/11630801646/job/32390526596?pr=31187#step:6:337

```
C++ compiler flags .................... -Wno-error=unused-member-function -O2 -g -std=c++20 -fPIC -fdebug-prefix-map=/home/runner/work/bitcoin/bitcoin=. -fmacro-prefix-map=/home/runner/work/bitcoin/bitcoin=. -Werror -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-
...

💬 maflcko commented on pull request "ci: Do not error on unused-member-function in test each commit":
(https://github.com/bitcoin/bitcoin/pull/31187#issuecomment-2452026337)
lgtm ACK 54d07dd37d5919c4e3bc535ae498d623282741d1