💬 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

🚀 achow101 merged a pull request: "crypto: add `NUMS_H` const"
(https://github.com/bitcoin/bitcoin/pull/30048)

💬 sipa commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2118163439)
It appears that RocksDb (more-developed derivative of LevelDB) uses a default of 64 MiB (https://github.com/facebook/rocksdb/blob/main/include/rocksdb/advanced_options.h#L468). See also my comment in https://github.com/bitcoin/bitcoin/pull/30059#issuecomment-2117888385.

🤔 ryanofsky reviewed a pull request: "refactor: interfaces, make 'createTransaction' less error-prone "
(https://github.com/bitcoin-core/gui/pull/807#pullrequestreview-2064121988)
Concept ACK edc3f9a733aaa1a1cc2547013d09c27828151e8a, but this has some changes I like and other changes I don't think are good. Left a more detailed comment below.

💬 ryanofsky commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#discussion_r1605397646)
In commit "gui: remove unreachable AmountWithFeeExceedsBalance error" (355199c7a6c58eb812dd7ef8e204e6e068c142ae)

This is a nice catch noticing that this code doesn't work. But I think the fact that it doesn't work is a pretty unfortunate problem, and the current PR will make fixing it harder.

It seems like #20640 unintentionally introduced a bug where the more accurate error message "The total exceeds your balance when the %1 transaction fee is included" can never trigger, and instead only
...

💬 achow101 commented on issue "stringop-overflow warning with GCC 14":
(https://github.com/bitcoin/bitcoin/issues/30114#issuecomment-2118174319)
> Looks like it derives that `.begin()` is the same as `.end()` which would bring the size to `-1`. This would require `CPubKey::GetLen` to return 0, which is possible if the first byte is invalid. Maybe it'd make sense to make it return `1` in that case because there's always the first byte?

That worked, but since `CPubKey` is used in a ton of places, I'm a little bit hesitant to change `GetLen` since that may have unexpected side effects. However, I do have a fix that seems to work that cha
...

📝 achow101 opened a pull request: "wallet, tests: Avoid stringop-overflow warning in PollutePubKey"
(https://github.com/bitcoin/bitcoin/pull/30131)
Fixes #30114

💬 ryanofsky commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2118182017)
I just noticed https://github.com/bitcoin/bitcoin/pull/25269, which I hadn't seen when I posted my comment above. It looks like that PR was closed for inactivity but it does look like a good approach that solves the issue I didn't like in my comment (though maybe it is still a slight regression because it doesn't include the amount of the fee in the error message). I wonder if you'd consider reopening that PR and maybe including the changes in this PR or basing this PR on that one?

Or if you
...

💬 laanwj commented on issue "stringop-overflow warning with GCC 14":
(https://github.com/bitcoin/bitcoin/issues/30114#issuecomment-2118192325)
Yes, could also handle it at another level, though it's strange for a class to return `begin()==end()` when there's actually one byte in it.

💬 achow101 commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#issuecomment-2118259798)
ACK b47bd959207e82555f07e028cc2246943d32d4c3

🚀 achow101 merged a pull request: "kernel: De-globalize fReindex"
(https://github.com/bitcoin/bitcoin/pull/29817)

🤔 furszy reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2064293082)
reACK d51fbab4b32d56765e8faab6ad01245fb259b0ca

💬 achow101 commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2118299920)
ACK 856c8563ca342303a83977146df22768bb6e2c7e

💬 achow101 commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1605494321)
```
../../../src/test/fuzz/tx_pool.cpp: In function ‘CTxMemPool {anonymous}::MakeMempool(FuzzedDataProvider&, const node::NodeContext&)’:
../../../src/test/fuzz/tx_pool.cpp:134:1: error: control reaches end of non-void function [-Werror=return-type]
134 | }
| ^
```

Also seems like the `Assert` is unreachable too.