📝 0xB10C opened a pull request: "test, tracing: don't use problematic `bpf_usdt_readarg_p()`"
(https://github.com/bitcoin/bitcoin/pull/31848)
Instead of using the undocumented bcc helper `bpf_usdt_readarg_p()`, use [`bpf_usdt_readarg()`][1] and [`bpf_probe_read_user()`][2]/[`bpf_probe_read_user_str()`][3] as documented in the [bcc USDT reference guide][1].

Note that the `bpf_probe_read_user()` documentation says the following:
> For safety, all user address space memory reads must pass through bpf_probe_read_user().

It's [assumed](https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2286505348) that using `bpf_usdt_read
...

💬 furszy commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1952982015)
Nvm, answered offline by mzumsande. It's for an edge case: the user invalidates the tip after startup and before receiving any blocks from the network, and the node then finds out that there are two competing blocks for the tip's ancestor.

💬 0xB10C commented on pull request "test, tracing: don't use problematic `bpf_usdt_readarg_p()`":
(https://github.com/bitcoin/bitcoin/pull/31848#issuecomment-2654199608)
I did [>15 re-runs](https://github.com/0xB10C/bitcoin/actions/runs/12912670830 ) of https://github.com/bitcoin/bitcoin/compare/master...0xB10C:bitcoin:2024-12-dont-use-bpf_usdt_readarg_p-testing similar to the 20 runs in https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656 to make sure this won't intermittently fail again.

💬 0xB10C commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2654207216)
opened #31848 which reverts https://github.com/bitcoin/bitcoin/pull/30574 and should fix the intermittent failures.

💬 darosior commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#discussion_r1952997408)
Good catch. Seems like this could also be useful for other harnesses.

💬 mzumsande commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r1952998754)
I don't think that this is an edge case we'd necessarily need to support though - it was just the only case I could come up where it would matter.

🤔 instagibbs reviewed a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363#pullrequestreview-2512158583)
reviewed up to "txgraph: (feature) add initial version" 781c15bfca1ebaffe7b634196e19144f5ab10a50 , basically all nits at this point

One question I had coming up is use of `Assume()`, there is a number of places that `Assume()` is used, then subsequently may result in UB or unexpected behavior. In a few spots I gave suggestions but was wondering if there is thinking behind doing more or not to handle that in release builds.

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1890424349)
```suggestion
/** The position this transaction in the main linearization (if present). **/
```

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943179984)
```Suggestion
// Figure out which elements elem needs to be placed after.
```
we are putting the *other* elements "before"

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943163414)
```Suggestion
// until it is placed after all its ancestors.
```

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943228435)
a921ae75c2f7162b3b617ddeccb84e3e60728cf8

Should we add a test that completes a cycle then asserts it's not acyclic to add coverage that way?

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943260292)
comment clarity: "it has no effect" meaning the dependency addition?

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943239737)
c89d147209c91bb0464321f5bc733a4eeab0dea0 in commit message:
"... and so for." and so on?

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943200510)
in release builds this will result in `j--` wrapping around to max value of uint32_t, causing an OOB access? We should probably just stop trying to fix things if it happens?

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943247961)
> This is only allowed when it is empty

Question: how is this enforced?

💬 instagibbs commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#discussion_r1943352763)
c89d147209c91bb0464321f5bc733a4eeab0dea0

could mention nullptr -> "unlinked"

🤔 furszy reviewed a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640#pullrequestreview-2612547860)
Code review ACK 177d07f

📝 fanquake opened a pull request: "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain"
(https://github.com/bitcoin/bitcoin/pull/31849)
According to the CMake docs, this is the correct way to setup a toolchain file for cross-compilation using Clang. See https://cmake.org/cmake/help/latest/manual/cmake-toolchains.7.html#cross-compiling-using-clang

Internally it looks like CMake will only take this variable into account if it detects the compiler to be Clang, so this shouldn't effect other builds, but in the case of our Apple cross builds, we'd end up with a duplicated `--target=$ARCH-apple-darwin` on the compiler line, given w
...

💬 theuni commented on pull request "build: simplify by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1953081930)
@hebasto Looking at this again, is there any reason [this needs to be an object library](https://github.com/bitcoin/bitcoin/blob/master/cmake/minisketch.cmake#L43)? It triggers the bug that necessitates the workaround here. I'm wondering if rather than using our hack or the (better) workaround suggested upstream, if we should just make it an archive library and avoid the issue altogether?

💬 achow101 commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#discussion_r1953089601)
Other than the dependency itself, I don't think so. The `pypropject-build-system` is part of python itself, and IIRC `python-poetry-core` is pure python as well with no dependencies.