📝 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.

💬 TheBlueMatt commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2223194931)
This seems like a lot of additional code that is very Sv2-specific. Is there no way to DRY a lot of this code up with connman itself? @pinheadmz may also have some thoughts here as I believe he's looking at replacing libevent with stuff that uses the common Core socket code anyway.

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

Thanks for taking time to follow-up on my comments.

🤔 marcofleon reviewed a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2172193524)
Lightly tested ACK 178a1da9c4da955c3dc78e6b03f110f687e82508. I tested on the three inputs brought up in https://github.com/bitcoin/bitcoin/issues/28812 and they all executed in 1ms. I can run the target for a while and see if any other timeouts come up. But looks good to me.

💬 marcofleon commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674183937)
Based on the two inputs that caused the timeouts:
[clusterfuzz-testcase-mocked_descriptor_parse-5224675118481408.bin.not.txt](https://github.com/bitcoin/bitcoin/files/13877884/clusterfuzz-testcase-mocked_descriptor_parse-5224675118481408.bin.not.txt)
[clusterfuzz-testcase-minimized-descriptor_parse-5888623437217792.bin.txt](https://github.com/bitcoin/bitcoin/files/15238132/clusterfuzz-testcase-minimized-descriptor_parse-5888623437217792.bin.txt)

I think it's supposed to be counting the
...

💬 Sjors commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2223234523)
> DRY a lot of this code up with connman itself?

Yes, I think the key is to make the copy-pasted parts of `CConnman::SocketHandlerConnected` reusable.

📝 maflcko opened a pull request: "rpc: Use CHECK_NONFATAL over Assert"
(https://github.com/bitcoin/bitcoin/pull/30429)
Any RPC method should not abort the whole node when an internal logic error happens.

Fix it by just aborting this single RPC method call when an error happens.

Also, fix the linter to find the fixed cases.

📝 fanquake converted_to_draft a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423)
The current `test-security-check` script is hard to understand, and change (i.e https://github.com/bitcoin/bitcoin/pull/29987/files#diff-52aa0cda44721f089e53b128cb1232a876006ef257b211655456b17dfb2ec712); tests are also not done in isolation (when-possible). Fix that, and add missing checks. Simplifies future toolchain/security/hardening changes.

🚀 glozow merged a pull request: "wallet: Migrate legacy wallets to descriptor wallets without requiring BDB"
(https://github.com/bitcoin/bitcoin/pull/26596)

💬 mzumsande commented on issue "intermittent issue in feature_reindex.py: Reindex fails to continue after restart; `initload` thread hangs forever":
(https://github.com/bitcoin/bitcoin/issues/30424#issuecomment-2223311441)
> There is no reindex progess (it should pick up the previous work and try to make progess)

I don't think that is the problem here. We have the (in my opinion slightly weird) behavior that when reindex continues after an interrupted reindex run, it calls [ActivateBestChain](https://github.com/bitcoin/bitcoin/blob/9b480f7a25a737c9c4ebc33401e94d66c2da9ec3/src/validation.cpp#L5112-L5115) immediately when processing the genesis block, which result in multiple blocks being connected that were rein
...

💬 josibake commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2223334508)
> but rather in the same project so that Just Works is even an option

I don't agree that everything being in the same project is necessary for things to "Just Work." However, I think it's entirely premature to be talking about where the code lives, what project, etc until we've at least decided on a technical design.

> (as long as the interface is sufficiently performant - one very old complaint about getblocktemplate is we end up wasting multiple milliseconds encoding all the crap we sho
...

💬 hebasto commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#discussion_r1674313294)
```suggestion
std::printf("the quick brown fox jumps over the lazy god\\n");
```

Otherwise, it fails:
```
test1.cpp:5:21: warning: missing terminating " character
5 | std::printf("the quick brown fox jumps over the lazy god
| ^
test1.cpp:5:21: error: missing terminating " character
5 | std::printf("the quick brown fox jumps over the lazy god
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
test1.cpp:6:
...

⚠️ fanquake opened an issue: "ci: failure in `p2p_unrequested_blocks.py`"
(https://github.com/bitcoin/bitcoin/issues/30430)
Initially thought it was another occurance of #30390. https://github.com/bitcoin/bitcoin/actions/runs/9880260514/job/27288384486#step:27:584:
```bash
node0 2024-07-10T20:00:55.007544Z [msghand] [D:\a\bitcoin\bitcoin\src\validation.cpp:4305] [AcceptBlockHeader] [validation] AcceptBlockHeader: Consensus::ContextualCheckBlockHeader: 5539fb56d40fa94857d5bee8e1640a3d5a7e50fefa38d1361d2fd43110dcec03, time-too-old, block's timestamp is too early
node0 2024-07-10T20:00:55.007575Z [msghand] [D:\a\
...

💬 fanquake commented on issue "ci: failure in `wallet_multiwallet.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/29073#issuecomment-2223378634)
https://cirrus-ci.com/task/5564623879929856?logs=ci#L3392