💬 Eunovo commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674013463)
@glozow Thanks for the clarification. I see it now. Any reason why we don't fire the `ActiveTipChange` here instead?

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674034705)
What do you mean by "here"?

💬 glozow commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1672095708)
probably `FeeFrac` would be better? `CFeeRate` loses precision and can be inferred from the fee and size

💬 Eunovo commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674043326)
By "here", I mean in https://github.com/bitcoin/bitcoin/blob/9b480f7a25a737c9c4ebc33401e94d66c2da9ec3/src/validation.cpp#L3505-L3508

🤔 hebasto reviewed a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423#pullrequestreview-2171924002)
Concept ACK.

nit: Spaces after commas and around `+` can used more consistently for better readability.

💬 hebasto commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674044249)
This check fails:
```
FAIL: test_PE (__main__.TestSecurityChecks)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/distsrc-base/distsrc-ea096063c384-x86_64-w64-mingw32/./contrib/devtools/test-security-check.py", line 88, in test_PE
self.assertEqual(call_security_check(cc, source, executable, pass_flags + ['-Wl,--disable-reloc-section']), (1, executable+': failed RELOC_SECTION'))
AssertionError: Tuples differ: (1, 'test1
...

💬 hebasto commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674030590)
Is the `-pie` flag supported here?

From the `ld` linker [docs](https://sourceware.org/binutils/docs/ld.html#index-_002dpie):
> This is currently only supported on ELF platforms.

💬 fanquake commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674050332)
Those look like the wrong docs:
```bash
x86_64-w64-mingw32-ld --help | grep pie
-pie, --pic-executable Create a position independent executable
-no-pie Create a position dependent executable (default)
```

💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#discussion_r1674079452)
Ok thanks again, I'll make sure it gets corrected at that time

💬 fanquake commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674084480)
This has turned up what looks like a bug in LIEF: https://github.com/lief-project/LIEF/issues/1076. Will re-enable the test later.

🤔 glozow reviewed a pull request: "#28984 package rbf followups"
(https://github.com/bitcoin/bitcoin/pull/30295#pullrequestreview-2172049102)
ACK 3f00aae1409

💬 achow101 commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1674109620)
We need the vector since order has to be preserved, and since the number of elements should be small, I don't think the performance impact is significant. Also, `std::find` is `O(n)`.

📝 maflcko opened a pull request: " log: LogError with FlatFilePos in UndoReadFromDisk "
(https://github.com/bitcoin/bitcoin/pull/30428)
These errors should never happen in normal operation. If they do,
knowing the `FlatFilePos` may be useful to determine if data corruption
happened. Also, handle the error `pos.IsNull()` as part of `OpenUndoFile`,
because it may as well have happened due to data corruption.

This mirrors the `LogError` behavior from `ReadBlockFromDisk`.

Also, two other fixup commits in this module.

💬 glozow commented on pull request "#28984 package rbf followups":
(https://github.com/bitcoin/bitcoin/pull/30295#issuecomment-2223121276)
cc @murchandamus, incorporates some of your test improvement suggestions https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1643074768

💬 TheBlueMatt commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2223137778)
> I sympathize with the argument that the harder we make things for the user, the less likely they are to run the software. I strongly disagree, however, with the conclusion that the best solution then is to put everything into a single binary for the user. All that matters for the user is that it Just Works ™️ and there are plenty of ways we can achieve this regardless of the technical design.

Right, I don't at think anyone was making the argument that it *has* to be in the same process, but
...

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2223139418)
I've made a few changes the past few days, and the code should be ready for more review now:
* The fuzzer serialization format for `DepGraph` was changed to something simpler.
* The linearization -> chunk feerates function `ChunkLinearization` was moved from the fuzz tests to the main cluster_linearization.h file, in a new separate commit, which also includes a fuzz test for just that function.
* The LIMO code now runs intersections with the chunk prefixes of the *current* chunk boundaries of
...

💬 hebasto commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674169416)
There seems to be some inconsistency in docs. For instance, in https://manpages.debian.org/bookworm/binutils-mingw-w64-x86-64/x86_64-w64-mingw32-ld.1.en.html:
>**-pie**
>**--pic-executable**
> Create a position independent executable. This is currently only supported on ELF platforms. Position independent executables are similar to shared libraries in that they are relocated by the dynamic linker to the virtual address the OS chooses for them (which can vary between invocations). Like norm
...

💬 maflcko commented on pull request "remove truc_policy from libbitcoin_common_a_SOURCES":
(https://github.com/bitcoin/bitcoin/pull/30427#issuecomment-2223174231)
ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c

💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2223184322)
Sorry, one small change to get rid of an extra function that I introduced, but ended up being unnecessary.

💬 glozow commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1674181253)
Because then it would fire even when it's not supposed to fire. We're aiming to add a notification for when the tip changes.