💬 marcofleon commented on issue "Test p2p_headers_presync fuzz target on macOS/Windows":
(https://github.com/bitcoin/bitcoin/issues/30950#issuecomment-2371442490)
The `BUILD_FOR_FUZZING` flag isn't on for Windows and macOS CI, so the `ABORT_ON_FAILED_ASSUME` and `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` macros aren't defined. The `p2p_headers_sync` target is doing actual PoW in `CheckProofOfWorkImpl` and timing out.

On the Linux CI those macros are here: https://cirrus-ci.com/task/6411626390224896?logs=ci#L831

Temporary solution would be excluding this harness for Windows and mac in the test runner. Long term we should probably separate out the CI
...

💬 maflcko commented on issue "Test p2p_headers_presync fuzz target on macOS/Windows":
(https://github.com/bitcoin/bitcoin/issues/30950#issuecomment-2371461229)
Makes sense. Is there any value in fuzzing this target when `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` isn't set?

If not, an early return in the target could make sense when `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` isn't set.

💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773525188)
Nice catch. I believe this can be simplified to:


```diff
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -941,9 +941,7 @@ public:
// Interrupt check interval
const MillisecondsDouble tick{1000};
auto now{std::chrono::steady_clock::now()};
- auto deadline = now + timeout;
- // std::chrono does not check against overflow
- if (deadline < now) deadline = std::chrono::steady_clock::time_point::max();
+ const auto de
...

💬 ismaelsadeeq commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#discussion_r1773601347)
> The code in wallet_assumeutxo.py is spread across the test and pretty specific to the assumeutxo context. I don't know how that would have help me in the other test I added.

Never mind `test_pruned_wallet_backup` is even better than what I was thinking.

> So to hit this case we would make the wallet migration fail after the backup is created, mine a few more blocks, then prune the node to the height where the migration failed, and then try to use the backup from the failed migration.


...

🤔 ismaelsadeeq reviewed a pull request: "wallet: Write best block to disk before backup"
(https://github.com/bitcoin/bitcoin/pull/30678#pullrequestreview-2325674671)
post-merge code review and tested re-ACK f20fe33e94c6752e5d2ed92511c0bf51a10716ee

✅ fanquake closed an issue: "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing"
(https://github.com/bitcoin/bitcoin/issues/30938)

🚀 fanquake merged a pull request: "test: Use shell builtins in run_command test case"
(https://github.com/bitcoin/bitcoin/pull/30952)

📝 achow101 opened a pull request: "[28.x] Further backports"
(https://github.com/bitcoin/bitcoin/pull/30959)
* #30952

💬 achow101 commented on pull request "test: Use shell builtins in run_command test case":
(https://github.com/bitcoin/bitcoin/pull/30952#issuecomment-2371668445)
Backported in #30959

⚠️ fanquake opened an issue: "log/dump more information if a CheckQueue failure occurs"
(https://github.com/bitcoin/bitcoin/issues/30960)
I recently saw this on a node running master (90a5786bba4baac1c0270e1f4bf5bb8c790e10de iirc):
```bash
2024-09-23T13:12:14Z UpdateTip: new best=00000000000000000000d3ebbca24c44745cfa14153e758d29af82275e00c00f height=862523 version=0x20026000 log2_work=95.170324 tx=1084781037 date='2024-09-23T13:03:46Z' progress=0.999997 cache=302.1MiB(2120379txo)
2024-09-23T13:12:14Z UpdateTip: new best=0000000000000000000164dcfef37624f2cc6b177032bbca57631285c2cd0267 height=862524 version=0x20150000 log2_work=
...

📝 fanquake opened a pull request: "ci: add `LLVM_SYMBOLIZER_PATH` to Valgrind fuzz job"
(https://github.com/bitcoin/bitcoin/pull/30961)
Otherwise:
```bash
NEW_FUNC[1/23]: ==4710==WARNING: invalid path to external symbolizer!
==4710==WARNING: Failed to use and restart external symbolizer!
0xb72010 (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0xa6a010) (BuildId: 2087ad415cb752eea259ed750f3b78a7fcb0b43b)
NEW_FUNC[2/23]: 0xb72240 (/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/test/fuzz/fuzz+0xa6a240) (BuildId: 2087ad415cb752eea259ed750f3b78a7fcb0b43b)

```

💬 ismaelsadeeq commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#discussion_r1773698697)
on master
```fish
1/1 - p2p_1p1c_network.py passed, Duration: 14 s

TEST | STATUS | DURATION

p2p_1p1c_network.py | ✓ Passed | 14 s

ALL | ✓ Passed | 14 s (accumulated)
Runtime: 14 s
```

On this PR without commenting `self.noban_tx_relay = True`

```fish
1/1 - p2p_1p1c_network.py passed, Duration: 15 s

TEST | STATUS | DURATION

p2p_1p1c_network.py | ✓ Passed | 15 s

ALL | ✓ Passed | 15 s (accumulated)
...

💬 maflcko commented on pull request "util: refactor: add and use run-time safe tinyformat::try_format":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1773705825)
Hmm, so I took a look here and the following diff seems to compile (almost):


<details><summary>a diff</summary>

```diff
diff --git a/src/tinyformat.h b/src/tinyformat.h
index f536306375..079b0ccbff 100644
--- a/src/tinyformat.h
+++ b/src/tinyformat.h
@@ -142,6 +142,7 @@ namespace tfm = tinyformat;
//------------------------------------------------------------------------------
// Implementation details.
#include <algorithm>
+#include <concepts> // Added for Bitcoin Core
#in
...

💬 maflcko commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#issuecomment-2371824878)
review ACK 6c3c619b35cc03e883f9d2b3326f906aedde9ba7

💬 maflcko commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#discussion_r1773719129)
> However I got timeout's intermittently after running this PR with `self.noban_tx_relay = True` commented out

tx relay may take time, if trickle is enabled. Especially if many nodes are hopped. You'll have to increase the timeout-factor as explained in https://github.com/bitcoin/bitcoin/pull/30948#issuecomment-2367507024

🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2325866285)
Rebased d59f5a31a1b888051eb47e2f8a5fb8d901de1d10 -> 3499c2ca4ba5ae70cebe84614498cc105e208f3d ([`pr/mine-types.14`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.14) -> [`pr/mine-types.15`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.14-rebase..pr/mine-types.15)) to fix silent conflict with #30409. Also made some improvements in response to recent comments.

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1773726499)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1766639028

Updated now to use concepts from serialiize.h instead of defining new ones

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1773732081)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1771915747

Rewrote the comment so hopefully it is less confusing now.

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1773727654)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1771936209

Added a comment to mining.capnp about getting rid of BlockValidationState here and returning something simpler from testBlockValidity

💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1773730480)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1771916488

I rewrote the comment and tweaked implementation of this to avoid use of non-standard Span class, so this could be dropped and moved to libmultiprocess in the future. But leaving that for a followup since it will require other changes to libmultiprocess.