💬 maflcko commented on issue "getJsonToken assumes underlying string is null-terminated but requires end pointer":
(https://github.com/bitcoin/bitcoin/issues/28260#issuecomment-1934121806)
It is probably not worth it to try to setup msan, as it requires building llvm from scratch.

It is easier to just use valgrind. For example: `valgrind ./src/test/test_bitcoin -t vector_tests`

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482976877)
Fixed

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482977350)
Updated 👍🏾

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482978519)
Added a test for this thanks.

🤔 jo-elimu requested changes to a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/29387#pullrequestreview-1870231673)
@BrandonOdiwuor There is a CI [error](https://github.com/bitcoin/bitcoin/pull/29387/checks?check_run_id=21316696148):
```
Uncovered RPC commands:
- abortrescan
```

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482979637)
Fixed

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482980736)
Thanks updated the tests.

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482981545)
Done

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1482981736)
Fixed

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-1934137354)
Thanks for your review @josibake
I fixed all review comments
Forced pushed from a5d10367cf832497af2ac72f8c2c42dd25398c63 to 1fae882f98205afb05e96432ce57212226bc0909 [Compare changes](https://github.com/bitcoin/bitcoin/compare/a5d10367cf832497af2ac72f8c2c42dd25398c63..1fae882f98205afb05e96432ce57212226bc0909)

💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1934152480)
@jo-elimu that's the error the fix was to catch. Check the issue

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1482999788)
I think that was a result of many changes during the path. In fact, we do not need `Initialize` anymore. We can concentrate the logic into `AddWhitelistPermissionFlags`.

💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1934168380)
In any case, a pull request with a failing CI can not be merged. Someone will need to add coverage for the RPC, here, or in a different pull.

📝 ryanofsky opened a pull request: "multiprocess: Add capnp wrapper for Chain interface"
(https://github.com/bitcoin/bitcoin/pull/29409)
**This is based on #28929.** The non-base commits are:

- [`486d39da5a3e` Add capnp serialization code for bitcoin types](https://github.com/bitcoin/bitcoin/pull/29409/commits/486d39da5a3e96791ad4033a863b606f9ae1ebc6)
- [`25e2ef26bede` Add capnp wrapper for Handler interface](https://github.com/bitcoin/bitcoin/pull/29409/commits/25e2ef26bede691ddf24f426b27987b7917437ba)
- [`22e05493c4f6` Add capnp wrapper for Chain interface](https://github.com/bitcoin/bitcoin/pull/29409/commits/22e05493c4f6
...

👋 ryanofsky's pull request is ready for review: "multiprocess: Add capnp wrapper for Chain interface"
(https://github.com/bitcoin/bitcoin/pull/29409)

💬 furszy commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483038171)
> Sure, that's why I suggest using `Assume`, and `return false` if there is no `activeTxn`. The callers of this should already be checking the return value to know whether the records were actually erased from the database.

ah, I completely skipped the `return false` last night.. sorry. All good, pushed.

🤔 furszy reviewed a pull request: "wallet: batch erase procedures and improve 'EraseRecords' performance"
(https://github.com/bitcoin/bitcoin/pull/29403#pullrequestreview-1870335464)
Updated per feedback. Thanks achow.
[Small diff](https://github.com/bitcoin/bitcoin/compare/0fd7b0dafa980a5dbcee8658d18a83f4eff57085..784034f8a17d46de9eab7de6fdec673a3a3d79d8), only changed `assert` for `Assume`.

💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1483043429)
added this commit [9f57a2e](https://github.com/bitcoin/bitcoin/pull/29402/commits/9f57a2ec49bf2c298acccbf8d889e692bb6894f7) to change the wording from `disk` to `file`, disk is used in the file once still but I think it makes sense as it is "opened mempool file from disk"

💬 brunoerg commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483043852)
Because we will need in `OpenNetworkConnection`.

🤔 furszy reviewed a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-1870367495)
Concept ACK.
Will start reviewing next week.