💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1140811067)
Ah, I see. Actually I've decided to just rebase this PR on top of #27195.

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1140812610)
Fixed.

💬 achow101 commented on pull request "bumpfee: allow send coins back to yourself":
(https://github.com/bitcoin/bitcoin/pull/27195#discussion_r1140814055)
In 7cb4dc157a711210e18f3c9b492150b6eb984b30 "bumpfee: enable send coins back to yourself"

`output_value` is not guaranteed to match what the user had actually specified for the single output.

💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1140814808)
Could:
```c++
auto [it, inserted] = m_records.emplace(key_data, value_data);
if (!inserted && overwrite) { // overwrite if requested
it->second = value_data;
inserted = true;
}
return inserted;
```

💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1140822051)
nit:
could use the previously introduced function.
```c++
GetMockableDatabase(wallet).m_pass = false;
```

💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1140836779)
tiny nit:
```c++
return std::make_unique<MockableDatabase>(dynamic_cast<MockableDatabase&>(database).m_records);
```

💬 furszy commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1140832310)
this include doesn't seems to be needed here.

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1140873089)
maybe I was thinking of `BOOST_CHECK(!addrman->Select(/*new_only=*/true, NET_I2P).first.IsValid());`?

not sure, I [added](https://github.com/bitcoin/bitcoin/compare/09d514583f15860f3bc7ae0c89e640c94fae3c71..17e705428ddf80c7a7f31fe5430d966cf08a37d6#diff-34d1a0e093152df355fc3a6b5b06156f7a9b936bfffb26bb221e62828e44532fR211) `BOOST_CHECK(!addrman->Select(/*new_only=*/true, NET_CJDNS).first.IsValid());` for real this time 🙃

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1140873566)
opted for the simple assertions because its less invasive & I didn't feel like there was a big advantage of switching over

💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1474557343)
two small updates:
- added assertions for bounds checking in `GetEntry`
- added the suggested test

💬 amitiuttarwar commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1474717814)
code review ACK 72e8ffd7f8dbf908e65da6d012ede914596737ab

👍 izlan90 approved a pull request: "p2p: Improve diversification of new connections"
(https://github.com/bitcoin/bitcoin/pull/27264)

💬 martinus commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#discussion_r1140953637)
Benchmarks are registered into a `std::map` so they are alphabetically sorted anyways https://github.com/bitcoin/bitcoin/blob/master/src/bench/bench.h#L68

💬 martinus commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1474761651)
code review & tested ACK, here are my benchmark results:

| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 3,822.12 | 261,634.61 | 0.3% | 22,931.00 | 8,536.02 | 2.686 | 4,512.00 | 0.1% | 0.5
...

💬 mxaddict commented on pull request "doc: fix typo in interface_usdt_utxocache.py":
(https://github.com/bitcoin/bitcoin/pull/27268#issuecomment-1474785590)
> I was under the impression PRs like this were pointless time-sinks on the maintainers' time.

Makes sense

✅ mxaddict closed a pull request: "doc: fix typo in interface_usdt_utxocache.py"
(https://github.com/bitcoin/bitcoin/pull/27268)

💬 MarcoFalke commented on pull request "refactor: wallet, do not translate init options names":
(https://github.com/bitcoin/bitcoin/pull/25666#issuecomment-1474788345)
lgtm ACK e43a547a3674a31504a60ede9b4912e014a54139

💬 MarcoFalke commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1474789198)
If the documentation for the field is unclear, what about changing the help text over changing the code?

💬 MarcoFalke commented on issue "Separate RPC events from catch-all debug.log":
(https://github.com/bitcoin/bitcoin/issues/7199#issuecomment-1474790082)
Closing for now. If there is anything left to be done here, a new feature request can be submitted, or this one reopened.

✅ MarcoFalke closed an issue: "Separate RPC events from catch-all debug.log"
(https://github.com/bitcoin/bitcoin/issues/7199)