💬 maflcko commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773095740)
nit in 7eccdaf16081d6f624c4dc21df75b0474e049d2b: Any reason to use the throwing function? The internal error isn't caught anywhere, IIUC. So, the program will terminate either way. By using `*Assert(AnyPtr(context))` here instead, at least a usable error message is printed.

<!--
For reference:

```
2024-09-24T10:38:06Z opencon thread exit
terminate called after throwing an instance of 'UniValue'
Aborted (core dumped)

💬 maflcko commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773215233)
b94b27cf05c709674117e308e441a8d1efddcd0a: Can you explain the meaning of "overflow" in the context of `double`?

💬 pablomartin4btc commented on issue "show error "could not sign any more inputs" when sign PSBT for multisig":
(https://github.com/bitcoin/bitcoin/issues/30177#issuecomment-2371290717)
This is being addressed in the GUI repository at https://github.com/bitcoin-core/gui/pull/832.

💬 maflcko commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-2371310988)
You'll have to put in some effort to create a pull request that is reviewable. It is fine to have a draft pull request, while you work on it, or ask specific questions about the implementation. However, it is unlikely that someone is going to review an incomplete draft pull request that needs rebase.

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773416406)
I probably just copied similar code from another place :-)

Can make a followup, though `*Assert(AnyPtr(context))` looks pretty scary.

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773420568)
It was in response to: https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1713879417

💬 KristijanSajenko commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2371390776)
Is it workin

💬 maflcko commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1773447523)
You probably copied it from an RPC method implementation. There, it would be the right choice, because the http threads catch exceptions and then continue with the next RPC call.

However, here no exceptions are caught (and doing so wouldn't make sense, probably). The program will terminate either way, for example:

```
2024-09-24T10:38:06Z opencon thread exit
terminate called after throwing an instance of 'UniValue'
Aborted (core dumped)
```

However, by using `Assert`/`assert`, at le
...

💬 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)
...