💬 furszy commented on pull request "wallet: fully migrate address book entries for watchonly/solvable wallets":
(https://github.com/bitcoin/bitcoin/pull/26761#discussion_r1134645263)
yeah 👌🏼.

💬 ryanofsky commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#issuecomment-1467021441)
I made some changes to this in a branch that could be adopted https://github.com/ryanofsky/bitcoin/commits/review.27217.1-edit

- Tries to split commits and add a lot of comments so this is easier to review
- Make enum a public wallet type instead of putting it in interfaces/ so wallet/ code does not depend on interfaces/wallet code and `interfaces::Wallet::AddressPurpose` becomes just `wallet::AddressPurpose`
- Replaced `std::optional<AddressPurpose>` with plain `AddressPurpose` in many pla
...

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134684312)
to add a second address, the test could do something like...

```
while (len(node.getnodeaddresses(count=0) < 2) {
node.addpeeraddress(...)
}
```

📝 theStack opened a pull request: "qt: remove confusing "Dust" label from coincontrol dialog"
(https://github.com/bitcoin-core/gui/pull/719)
In contrast to to all other labels on the coin selection dialog, the displayed dust information has nothing to do with the selected coins. All that this label shows is whether at least one of the _outputs_ qualify as dust, but the outputs are set in a different dialog. (Even worse, the dust check is currently simply wrong because it only looks at an output's nValue and just assumes a P2PKH script size.)

As the label clearly doesn't help the user and is, quite the contrary, rather increasing c
...

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134703332)
adding these addresses would be subject to the same sporadic failures that we've been discussing elsewhere. although `GetGroup` would return different values, they aren't isolated so will still occasionally hash to the same positions.

you have to add a while loop or something to keep trying until success to prevent sporadic failures

💬 amitiuttarwar commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134705887)
hm, seems a little low-level for release notes. maybe something like


```suggestion
- CLI -addrinfo previously only returned a subset of addresses known to the node. It has been reworked to use the new `getaddrmaninfo` RPC and returns information about the entirety of the node's addrman. (#26988)
```

📝 pablomartin4btc opened a pull request: "httpserver, rest: fix segmentation fault on evhttp_uri_get_query"
(https://github.com/bitcoin/bitcoin/pull/27253)
The issue is that an invalid uri parsed by `evhttp_uri_parse()`(part of the [libevent open-source project]( https://github.com/libevent/libevent.)) which result (NULL/ nullptr) is sent to `evhttp_uri_get_query()` will produce a segmentation fault.

https://github.com/bitcoin-core/gui/blob/86bacd75e76eff207ef8f7bdb897365f5b6e7b6b/src/httpserver.cpp#L673-L676

In order to reproduce the issue:
1. Start `bitcoind -rest`.
2. Get a block hash (e.g.: `./src/bitcoin-cli getbestblockhash`).
3. Exe
...

🚀 fanquake merged a pull request: "refactor: Split logging utilities from system.h"
(https://github.com/bitcoin/bitcoin/pull/27238)

💬 MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1135206140)
Are you sure? I tested my proposed change locally.

💬 MarcoFalke commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1135213207)
Looks like it throws `test_framework.authproxy.JSONRPCException: Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet. (-4)`, which can be caught by `assert_raises_rpc_error`?

💬 glozow commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#discussion_r1119840003)
I have a question about reusing an event name, i.e. for rejections in packages (in a future PR) could/should we add a `TRACE2(mempool, rejected, txhash, reason)` or `TRACE3(mempool, rejected, txhash, pkghash, reason)` in the package rejection code? Or should we use a different `event`?

🚀 hebasto merged a pull request: "Mask values on Transactions View"
(https://github.com/bitcoin-core/gui/pull/708)

📝 TheCharlatan opened a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by @empact and are taken from their parent PR #25152.

#### Context

There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library
...

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1467916011)
Rebased a7afa98ca38fd66fc69b77b95a6a57d50207911b -> 45048c0d7e0ee12210d226239fb5ce63d9ed4441 ([tc/2022-09-libbitcoinkernel-chainparams-args_5](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_5) -> [tc/2022-09-libbitcoinkernel-chainparams-args_6](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_5..tc/202
...

💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135396391)
```suggestion
#include <cstdint>
#include <cstdio>
```

nit

💬 MarcoFalke commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135397641)
unrelated change?

💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135402195)
nit: missing newline

💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135404419)
This is from clang format diff. Should I drop these kind of changes in the future? There are also some other not strictly related include order fixes.

💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135405169)
Pretty sure this can just be merged into the includes below.

💬 fanquake commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#discussion_r1135406000)
I think include fixing is fine. However theres no need to include unrelated clang-format changes.