🤔 hodlinator reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3238340125)
Reviewed f6951cb74bba60fedb2e0c041038561ba2583594

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358151635)
nit in 788e45a5b210224e476543cbd31dffc7ae39b13e: Some sympathy for unwrapping the initial sentence since with the (and/an) typo fix it's just at 80 chars, but 4 space indent seems less arbitrary than 1 space, and not worth touching these lines for.

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358241717)
```suggestion
p2p3 = self.nodes[3].add_p2p_connection(BaseNode())
```

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358167560)
nit in 9ecaadc94a5a4a61263aa17ddaba23ee5d155042: Think there is some value in benefit from nodes starting up in parallel with the python code running. Would prefer starting them all here or even sooner, before the loop generating 2100 blocks above.

Sorry if my suggestion in https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2351892330 precipitated this, I was just trying to reduce complexity in the example.

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358189439)
In b577becd7167593ba55f1f084cfded59f4c11e47: Touching this line to reduce it by 1 char seems uncalled for now that you are keeping the name.

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358761689)
nit: I'd prefer avoiding having this dual state. Here are 2 variations which do that:

Preserving behavior:

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 7e82c3bbad..008bfe4d71 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2423,8 +2423,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
return true;
}

- auto fScriptChecks{true};
- std::string script_check_reason;
+ const char* script_che
...

💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2358840909)
Yes, I would prefer having a separate reason message for when we have passed beyond the assumevalid height since it's so fundamental to assumevalid, and so different from being at a height below assumevalid on a forked chain.

Agree it probably belongs in a separate commit, but still think it would be good to have in this PR.
Your first commit that currently introduces `"block not part of assumevalid chain"` should probably state something more neutral like `"block is not an ancestor of the a
...

💬 hebasto commented on issue "Guix: build riscv64 master / v30rc1 fails":
(https://github.com/bitcoin/bitcoin/issues/33426#issuecomment-3307151918)
@janb84

Could you please check your build log for the actual error message, which may appear many lines earlier?

💬 janb84 commented on issue "Guix: build riscv64 master / v30rc1 fails":
(https://github.com/bitcoin/bitcoin/issues/33426#issuecomment-3307183805)
Added console log, and build logs

[CMakeError.log](https://github.com/user-attachments/files/22406286/CMakeError.log)
[CMakeOutput.log](https://github.com/user-attachments/files/22406287/CMakeOutput.log)
[console.log](https://github.com/user-attachments/files/22406285/console.log)

💬 hebasto commented on issue "Guix: build riscv64 master / v30rc1 fails":
(https://github.com/bitcoin/bitcoin/issues/33426#issuecomment-3307217163)
> Added console log, and build logs
>
> [console.log](https://github.com/user-attachments/files/22406285/console.log)

Does it include stderr output as well?

💬 janb84 commented on issue "Guix: build riscv64 master / v30rc1 fails":
(https://github.com/bitcoin/bitcoin/issues/33426#issuecomment-3307226772)
> > Added console log, and build logs
> > [console.log](https://github.com/user-attachments/files/22406285/console.log)
>
> Does it include stderr output as well?
Unfortunately not, just piped the output to a file. Working on it :)

💬 brunoerg commented on issue "coin-grinder missing test for TOTAL_TRIES":
(https://github.com/bitcoin/bitcoin/issues/33419#issuecomment-3307249900)
For reference: https://corecheck.dev/mutation/src/wallet/coinselection.cpp (L466 and L468)

💬 hebasto commented on issue "build: secp256k1 warnings not turned into errors in MSAN job":
(https://github.com/bitcoin/bitcoin/issues/33284#issuecomment-3307302109)
Related: https://github.com/bitcoin-core/secp256k1/pull/1750.

💬 fanquake commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3307307680)
@hebasto can you followup here

💬 fjahr commented on pull request "wallet/rpc: add scan_utxoset option to getbalance(s) to verify wallet balance accuracy":
(https://github.com/bitcoin/bitcoin/pull/33392#issuecomment-3307315975)
> Just to make sure we’re on the same page — my understanding is that the idea is to:
>
> Use chunked backward rescans starting from the supplied birthdate (e.g. 1000-block increments), and stop as soon as the wallet balance matches the UTXO-set scan. This handles the common case where the birthdate is only off by a few days or weeks, so we don’t need to rescan the entire chain.

Right, so for example if the supplied birthdate was blockheight `800,000` but there is a discrepancy, then, inst
...

💬 fanquake commented on pull request "Update libmultiprocess subtree to fix intermittent mptest hang":
(https://github.com/bitcoin/bitcoin/pull/33412#issuecomment-3307342620)
cc @Sjors

💬 fanquake commented on pull request "ci: run native_fuzz_with_msan":
(https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3307360138)
Runtime is alright, and similar to Valgrind, is being blown a bit by the slowest tests. i.e mini_miner running for a half hour:
```bash
Run mini_miner with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/mini_miner')]INFO: Running with entropic power schedule (0xFF, 100).
<snip>
Done 641 runs in 1868 second(s)
Run txdownloadman_impl with args ['/home/admin/actions-runner/_work/_temp
...

💬 fanquake commented on issue "fuzz: Speed up mini_miner fuzz target?":
(https://github.com/bitcoin/bitcoin/issues/32870#issuecomment-3307388406)
No, this is still an issue. See here: https://github.com/bitcoin/bitcoin/pull/33425#issuecomment-3307360138. `mini_miner` is running for ~half an hour. cc @murchandamus @glozow

📝 john-moffett opened a pull request: "rpc: Always return per-wtxid entries in submitpackage tx-results"
(https://github.com/bitcoin/bitcoin/pull/33427)
Follow-up to #28848

When `submitpackage` produced no per-transaction result for a member, the RPC set `"error": "unevaluated"` but then continued without inserting the entry into `tx-results`, making it impossible for callers to know which `wtxids` were unevaluated.

This inserts the error result before continuing, updates help text, and adjusts functional tests to expect entries for all submitted `wtxids`.

🤔 glozow reviewed a pull request: "rpc: Always return per-wtxid entries in submitpackage tx-results"
(https://github.com/bitcoin/bitcoin/pull/33427#pullrequestreview-3239717920)
Concept ACK, this looks like an unintentional drop. I thought "unknown-not-validated" would be better since it matches the error string for skipped transactions, but I think it's better to have separate messages for "nothing was validated because the package was ill-formed" vs "this particular transaction wasn't validated."

cc @instagibbs