📝 edilmedeiros opened a pull request: "Increase contrib/signet/miner search space"
(https://github.com/bitcoin/bitcoin/pull/30130)
The miner script will call `bitcoin-util grind` to compute PoW which will try to exhaust the block's nonce field and fail if it can't find a valid hash. This behavior does not appear for low difficulty chains, but make the miner unusable for higher difficulty settings.

We capture `bitcoin-util grind` failure, build a new block header by changing the block time and try to grind again.

Fixes #30102.

## How this was tested

This is a follow-up from #30091 and #30102.

I've started a ne
...

💬 edilmedeiros commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#issuecomment-2117932334)
Force pushed so that the commit message is the same as the PR title.

👍 stickies-v approved a pull request: "kernel: De-globalize fReindex"
(https://github.com/bitcoin/bitcoin/pull/29817#pullrequestreview-2063857291)
ACK b47bd959207e82555f07e028cc2246943d32d4c3

💬 stickies-v commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1605255453)
nit: perhaps `static SynchronizationState GetSynchronizationState(const ChainstateManager& chainman)` makes for a more logical function signature now, but it doesn't really seem to make for much of a clean-up either, so... not sure.

💬 theStack commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2117946696)
Concept ACK

Can you squash the two commits? (Currently compiling d06227fd6b87b5a427441a212c04a0ba64edc142 fails).

💬 TheCharlatan commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1605278661)
Mmh, I think I prefer this as is.

💬 edilmedeiros commented on pull request "contrib/signet/miner: increase miner search space":
(https://github.com/bitcoin/bitcoin/pull/30130#issuecomment-2117967175)
A small caveat: I couldn't get rid of the `Could not satisfy difficulty target` (see the debug log just before timestamp `2024-05-17 12:56:01`). This is emitted by `bitcoin-util` itself before failing. I could not find a way to make the Python script intercept that to keep the terminal clean. I fear the complexity to achieve such an effect would not justify the benefit.

👍 theuni approved a pull request: "util: avoid using thread_local variable that has a destructor"
(https://github.com/bitcoin/bitcoin/pull/30095#pullrequestreview-2063982122)
Agree with @laanwj's take.

utACK d35ba1b3f16071b8fe9b36398ba15352dbf2a54d

👍 theuni approved a pull request: "build: Enable `thread_local` for MinGW-w64 builds"
(https://github.com/bitcoin/bitcoin/pull/30099#pullrequestreview-2063984747)
utACK 5822a82a88e46ef08e5c18c41489ffd3e9d899c6.

After this and #30095, is there any need to keep the thread_local option at all?

💬 theuni commented on pull request "rpc: avoid copying into UniValue":
(https://github.com/bitcoin/bitcoin/pull/30115#issuecomment-2118018627)
@willcl-ark Thanks, that's very helpful. Out of curiosity, what build options are you using for these? Are optimizations on?

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605307850)
```suggestion
"Usage: bitcoind [options] Start " PACKAGE_NAME "\n"
```

🤔 edilmedeiros requested changes to a pull request: "Update manpage descriptions"
(https://github.com/bitcoin/bitcoin/pull/29686#pullrequestreview-2063952610)
Concept ACK

I would keep the old comment for each command even in face of the additional information provided (see review comments). I tend to rely a lot on those to quickly remember what the commands do (but will not oppose taking them out if this is the consensus). I understand that they may break the formatting of the manual pages, so feel free to discard them.

For the `bitcoin-util` tool, I suggest taking the description from the PR that introduced the tool (see comment).

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605316111)
```suggestion
"Usage: bitcoin-tx [options] <hex-tx> [commands] Update hex-encoded bitcoin transaction\n"
```

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605316427)
```suggestion
"or: bitcoin-tx [options] -create [commands] Create hex-encoded bitcoin transaction\n"
```

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605322723)
```suggestion
"\nUsage: bitcoin-cli [options] <command> [params] Send command to " PACKAGE_NAME "\n"
"or: bitcoin-cli [options] -named <command> [name=value]... Send command to " PACKAGE_NAME " (with named arguments)\n"
"or: bitcoin-cli [options] help List commands\n"
"or: bitcoin-cli [options] help <command> Get help for a command\n"
```

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605327472)
```suggestion
"The bitcoin-util tool provides bitcoin related functionality that does not rely on the ability to access a running node.\n"
```

Why not use the description from #19937 which introduced the tool?

💬 edilmedeiros commented on pull request "Update manpage descriptions":
(https://github.com/bitcoin/bitcoin/pull/29686#discussion_r1605320367)
Kind of not agree since the tool is intended to be used with Bitcoin Core RPC and maintained for that only purpose. On the other hand, I understand that knots (and other forks) is compatible in this regard with Core and the bitcoin-cli tool will work anyhow without discriminating the server.

💬 tdb3 commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#discussion_r1605333835)
Was talking with @pinheadmz about this. Maybe as an alternative, the test could fetch the `getversion` RPC response (e.g. with "27.0..."), then use `assert_debug_log()` to check that the RPC response matches what's in the log.

🤔 marcofleon reviewed a pull request: "fuzz: wallet, add target for `Crypter`"
(https://github.com/bitcoin/bitcoin/pull/28074#pullrequestreview-2064071922)
I used ASan and UBSan. I think it might just be a mac specifc issue, I'm fuzzing on a mac with m3 chip.

Here's the error:
```bash
INFO: seed corpus: files: 111934 min: 1b max: 5242880b total: 2040057759b rss: 216Mb
#8192 pulse cov: 938 ft: 2162 corp: 81/730b exec/s: 4096 rss: 318Mb
#16384 pulse cov: 1049 ft: 2510 corp: 127/2096b exec/s: 5461 rss: 416Mb
#32768 pulse cov: 1571 ft: 4427 corp: 219/9334b exec/s: 4681 rss: 565Mb
#65536 pulse cov: 1774 ft: 7969 corp: 371/56Kb exec/s: 4681
...

💬 achow101 commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2118128913)
ACK 9408a04e424cee0d226bde79171bd4954f9caeb0