👍 maflcko approved a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/29387#pullrequestreview-1867666239)
Thanks. lgtm

💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#discussion_r1481384059)
```suggestion
subprocess.check_output([sys.executable, tests_dir + 'create_cache.py'] + flags + [f"--tmpdir={tmpdir}/cache", "--descriptors"])
```

💬 Empact commented on pull request "refactor: Remove excess reserve() call for SecureString":
(https://github.com/bitcoin/bitcoin/pull/29364#issuecomment-1931936133)
Is it not worthwhile to prevent reallocation if we can, even if there is not a data disclosure risk with this allocator?

💬 maflcko commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/29398#issuecomment-1931938143)
I presume the following may be interesting for someone to debug this, if you don't mind to share:

Which wallet settings are you using? (Feerate, etc)
Which RPCs are you using?
Which amounts and settings are you passing to the RPCs? SFFO?
How many wallet coins do you have?
Maybe an exact list with amounts ?

💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#discussion_r1481398891)
fixed

💬 azazar commented on issue "Internal bug detected: Fee needed > fee paid":
(https://github.com/bitcoin/bitcoin/issues/29398#issuecomment-1931949888)
bitcoind settings: -limitancestorcount=1000 -txindex=1 -testnet -server

RPCs: sendmany('', $amounts, 0, '', [], true, 1) / getbalance('*', 1) / getbalance('*', 0)

Amounts vary, but quite small always. You can examine some of the last transactions:

https://live.blockcypher.com/btc-testnet/tx/980f2c14c3930d31485b94ae449177dfa8dadeeaba838d5a0a171e039d955dd2
https://live.blockcypher.com/btc-testnet/tx/298d311345c671b6cac8ca58d683789f736de106d69b67fb49cb261dc2b351c1
https://live.blockcyphe
...

💬 maflcko commented on issue "Update nChainTx to 64bit type":
(https://github.com/bitcoin/bitcoin/issues/29258#issuecomment-1931952848)
> This does seem like an nice and easy solution. Would be good to incorporate in #29331

Not sure if it so easy. On 32-bit platforms, `nTx` currently starts at byte 60, so making it 16-bit will still eat 32-bit with padding. So forcing a 64-bit blob of both values is needed, but that requires changing all code that uses either field. Not sure if worth it for a +-5% memory difference?

💬 furszy commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1481412610)
This test was originally generic and it was moved to sqlite-only because of a bdb bug, see https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1795448339.

💬 fjahr commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1481442396)
nit: Could remove this log line

📝 kevkevinpal opened a pull request: "mempool: Loading progress added for dumping mempool transactions to disk"
(https://github.com/bitcoin/bitcoin/pull/29402)
Sometimes when shutting off bitcoind it can take a while to dump the mempool transaction onto the disk so
this change adds additional logging to the `DumpMempool` method in `kernel/mempool_persist.cpp`

Similar to https://github.com/bitcoin/bitcoin/pull/29227 this
- adds a maximum of 10 new log lines
- logs the progress for every 10%

This is in response to https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1893375082

The logs will now look like this
```
2024-02-07T13:03:5
...

💬 maflcko commented on pull request "mempool: Loading progress added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1932034222)
Why is this needed? A progress log for something that takes less than a second does not sound useful

💬 kevkevinpal commented on pull request "mempool: Loading progress added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1932077013)
> Why is this needed? A progress log for something that takes less than a second does not sound useful

This is in response to this comment https://github.com/bitcoin/bitcoin/pull/29227#pullrequestreview-1817064507
the logs I posted in the description are from testnet I can run this on mainnet to get the time it takes on shutdown there

📝 furszy opened a pull request: "wallet: batch erase procedures and improve 'EraseRecords' performance"
(https://github.com/bitcoin/bitcoin/pull/29403)
The first two commits are coming from #26836. They are required for these changes.

Seeks to optimize and simplify `WalletBatch::EraseRecords`. Currently, this process opens a cursor to iterate over the entire database, searching for records that match the type prefixes, to then call the `WalletBatch::Erase` function for each of the matching records.
This PR rewrites this 40-line manual process into a single line; instead of performing all of those actions manually, we can simply utilize the
...

💬 brunoerg commented on pull request "rpc: getdescriptorinfo also returns normalized descriptor":
(https://github.com/bitcoin/bitcoin/pull/29396#issuecomment-1932083218)
```
test 2024-02-07T08:26:22.085000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wal
...

💬 maflcko commented on pull request "mempool: Loading progress added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1932088394)
The delay is probably from leveldb, but it would be good to first check

💬 maflcko commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1481506563)
nit, feel free to ignore: Any reason to add `__func__` to this log, when none of the other logs have it here? If someone is interested in the exact source location, they can enable the corresponding log option. If this is needed for context, it could make more sense to clarify the log message itself, so that it is unique and clear without requiring the source code as context.

💬 furszy commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1481525685)
> Any reason to add `__func__` to this log, when none of the other logs have it here?

Not really. It is just a bad habit. I pulled 9a4e1f5 from #26836 because it is required for this changes. Will change it there.

💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1932120188)
> It would be good to check this. If you have a completely separate machine, you could try to install the "anti"-virus software there and see if the issue happens?

I don't have another windows machine, so unfortunately I'm not able to check this. I'm not sure if it would do much, because I've now gotten to IBD several times without issues, whereas before when antivirus was still active, the issues came up persistently around block height 350,000-450,000 or so.

So I didn't get the previous
...

🤔 furszy reviewed a pull request: "wallet: batch and simplify addressbook migration process"
(https://github.com/bitcoin/bitcoin/pull/26836#pullrequestreview-1867915441)
Updated per @maflcko review in https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1481506563. [Small diff](https://github.com/bitcoin/bitcoin/compare/238993ea40c1c384e9fbcf83bfcf45efc73ad1bb..4c4c62b459e5b92392a67cb369101b07fd4bad28).

💬 furszy commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#issuecomment-1932141334)
Updated per feedback. Thanks Marko.