💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2188910716)
I modified to calculate chainwork 100 blocks greater than the current `best_header`, and I checked that the test still verifies the expected behaviour. See https://github.com/bitcoin/bitcoin/pull/31615/commits/a53d43bc0f010513b0922ad48d611f402ec0e511

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189149829)
done

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189152069)
Great! Thanks

💬 Sjors commented on pull request "refactor: use options struct for signing and PSBT operations":
(https://github.com/bitcoin/bitcoin/pull/32876#discussion_r2189159233)
@optout21 see https://github.com/bitcoin/bitcoin/pull/32876#issuecomment-3036784505

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189162753)
Great! Thanks :)
Done

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189174459)
True! Done

💬 Sjors commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2189175462)
> To be sure, the "show this help message" does not appear when you just run bitcoin with no arguments

That would be great, but it doesn't for me:

```
% build/bin/bitcoin
Usage: bitcoin [OPTIONS] COMMAND...

Options:
...
-h, --help Show this help message
```

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189201868)
I don't see in which case a 0 or less can be passed. It should never happen.
If it fails on the assume then there's something wrong in another part of the code and we should not treat that as an expected case.

Happy to go back to return -1 if there's a specific case for that.

🤔 hodlinator reviewed a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2992479971)
Thanks for checking this out @danielabrozzoni! Pushed some minor changes based on your feedback.

💬 hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2189176855)
Agree that's cleaner, fixed!
Also changed from
`== std::nullopt` to
`.has_value()`.

💬 hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2189181808)
Changed to the following, let me know what you have further suggestions:
> Start feeding back headers into the permanent block index once more than this number of them have been received and validated against commitments.

💬 hodlinator commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2189214990)
In this case we have the comment right above describing the same thing:
https://github.com/bitcoin/bitcoin/blob/79bdc6a785bca96dababc698336c04b8625b7d1c/src/test/headers_sync_chainwork_tests.cpp#L140

Went through the code and realized `HappyPath` was repeating `/*full_headers_message=*/` in one piece of newer code, so removed that.

This boolean parameter to `ProcessNextHeaders()` is somewhat of a code smell. Maybe could be split into 2 public functions (`ProcessNextHeaders()` + `ProcessLa
...

💬 maflcko commented on pull request "threading: use correct mutex name in reverse_lock fatal error messages":
(https://github.com/bitcoin/bitcoin/pull/32829#issuecomment-3043838702)
If we want more debug in the ci task, we could go full `Debug` type, but enable optimizations:

```
-DAPPEND_CXXFLAGS='-O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2189259717)
lgtm, done!

💬 Sjors commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3043856634)
Assuming this is specific to external wallets, and not unrelated bug in the test itself, the most interesting code is:

```cpp
void CWallet::SetupDescriptorScriptPubKeyMans()
{
...
UniValue signer_res = signer->GetDescriptors(account);
```

Beyond that wallet creation is more or less the same between regular and external signer wallets.

```cpp
UniValue ExternalSigner::GetDescriptors(const int account)
{
return RunCommandParseJSON(m_command + " --fingerprint " + m_fingerprint + Netwo
...

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3043873716)
This may also be useful for Silent Payments, cc @josibake.

💬 eval-exec commented on pull request "rest: replace `rf_names[0].rf` by `RESTResponseFormat::UNDEF` for code clarity":
(https://github.com/bitcoin/bitcoin/pull/32884#issuecomment-3043891057)
Friendly request @fanquake to review this PR.

📝 maflcko opened a pull request: "ci: Use optimized Debug build type in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/32888)
An optimized debug build is mostly as fast as a release build, because hot loops of heavy debug-only code are rare. So use that setting in the test-each-commit CI, to enable more checks almost "for free".

💬 stratospher commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3043958524)
reACK a53d43b.

💬 maflcko commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3043971964)
I'd say it would be useful if the `mocks` scripts had more debug logging enabled; maybe some command tracing when entering the script and when exiting it?