💬 l0rinc commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773291395)
```suggestion
* @return parsed settings value if it is valid, otherwise `nullopt` accompanied
```

<img width="645" alt="image" src="https://github.com/user-attachments/assets/f11f67dd-04ae-492d-b053-07471e6e4ecc">

⚠️ brunoerg opened an issue: "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated"
(https://github.com/bitcoin/bitcoin/issues/30957)
Similar to #28019. The following instruction is outdated and doesn't work:
```diff
$ git apply << "EOF"
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 8195bceaec..cce2b31ff0 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -90,8 +90,12 @@ typedef char* sockopt_arg_type;
// building with a binutils < 2.36 is subject to this ld bug.
#define MAIN_FUNCTION __declspec(dllexport) int main(int argc, char* argv[])
#else
+#ifdef HFND_FUZZING_ENTRY_FUNCTION_CXX
+#
...

⚠️ fanquake opened an issue: "`invalidateblock` during background validation"
(https://github.com/bitcoin/bitcoin/issues/30958)
If you call `invalidateblock` (on the tip) during background validation, the node will currently do the following:
#### bitcoin-cli invalidateblock 00000000000000000000f5752e9599f03a7b3a3e2356af91faaf74e41fdf58fe
```bash
2024-09-24T13:01:15Z [background validation] UpdateTip: new best=0000000000000000008dd2857255b2e5fc0c3955740c74dac6bafd0de09c344e height=482000 version=0x20000000 log2_work=86.991735 tx=249395394 date='2017-08-25T19:52:51Z' progress=0.230269 cache=238.7MiB(1748399txo)
2024-0
...

🤔 maflcko reviewed a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2324834610)
review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db 🛋

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 7942951e3fcc
...

💬 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