📝 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.

👍 laanwj approved a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2064310065)
re-ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca

💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1605497660)
Ugh, I'll push a fix.

📝 TheCharlatan opened a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132)
When restarting `bitcoind` during an ongoing reindex without setting the `-reindex` flag again, the block and coins db is left intact, but any data from the optional indexes is discarded. While not a bug per se, wiping the data again is
wasteful, both in terms of having to write it again, as well as potentially leading to longer startup times. So keep the index data instead when continuing a prior reindex.

Also includes some smaller code cleanups around the reindexing code.

💬 TheCharlatan commented on pull request "[refactor] Check CTxMemPool options in ctor":
(https://github.com/bitcoin/bitcoin/pull/28830#issuecomment-2118336056)
Updated 856c8563ca342303a83977146df22768bb6e2c7e -> f2f50a4db2f61b767fd16ca2e99e136eb7dff8a6 ([mempoolArgs_10](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_10) -> [mempoolArgs_11](https://github.com/TheCharlatan/bitcoin/tree/mempoolArgs_11), [compare](https://github.com/TheCharlatan/bitcoin/compare/mempoolArgs_10..mempoolArgs_11))

* Addressed @achow101's [comment](https://github.com/bitcoin/bitcoin/pull/28830#discussion_r1605494321), fixing unreachable Assert.
* Slightly adjusted
...

💬 brunoerg commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-2118348115)
I couldn't reproduce on MacOS 14.3 with that input file.

> If I add limits (1024 for example) to ConsumeRandomLengthByteVector and ConsumeRandomLengthString the harness works. But that could be defeating the purpose of fuzzing.

Changing the harness can invalidate the input. Also, `ConsumeRandomLengthByteVector` is used in a lot of other targets without a limit. Does it happen for other targets as well?

📝 theStack opened a pull request: "test: remove unneeded `-maxorphantx=1000` settings"
(https://github.com/bitcoin/bitcoin/pull/30133)
It's unclear what the motivation for increasing the orphan pool is here, and it seems that this not needed at all. None of these tests involve orphan transactions explicitly, and if they would occur occasionally, there is no good reason to prefer a value of 1000 over the default of 100 (see DEFAULT_MAX_ORPHAN_TRANSACTIONS).

📝 brunoerg opened a pull request: "fuzz: add more coverage for `ScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/30134)
This PR adds more coverage for `ScriptPubKeyMan`:

- Check `GetKey` and `HasPrivKey` after adding descriptor key.
- Cover `GetEndRange` and `GetKeyPoolSize`.
- Cover `MarkUnusedAddresses` with the scripts from ScriptPubKeys and `GetMetadata` with the destinations from them.

Also, this PR removes unnecessary includes from `fuzz/scriptpubkeyman.cpp`

👍 hebasto approved a pull request: "bench: enable wallet creation benchmarks on all platforms"
(https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2064368273)
re-ACK 7c8abf3c2001152423da06d25f9f4906611685ea, I agree with changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2061694682).

💬 brunoerg commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-2118365692)
Added https://github.com/bitcoin/bitcoin/pull/30134 to the list for review. It adds more coverage for `ScriptPubKeyMan`.

👍 theStack approved a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-2064403732)
ACK d51fbab4b32d56765e8faab6ad01245fb259b0ca

Convinced myself that this parser works reliably partly by looking at the original code, partly by extending the test framework's BDB parser to match this PR's logic (see #30125), which helped a lot in understanding the structure. Also tested with a few random signet legacy wallets that I still had around, though those were all quite small.

Btw, I couldn't manage to find or create a BDB wallet where the DELETE flag is set in a record header (wou
...

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1605585592)
typo nit
```suggestion
parser.add_argument("--swap-bdb-endian", action="store_true",help="When making Legacy BDB wallets, always make them byte swapped internally")
```

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1605553167)
nit: isn't thrown anywhere, can be removed
```suggestion
```