💬 fanquake commented on pull request "Update chainparams for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27223#issuecomment-1458705526)
> Might have jumped the gun on this.

Yea, it's a bit too early to be doing this. See #26549.

💬 fanquake commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1458711392)
Concept ACK - will test

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1128463603)
thanks! done.

💬 fanquake commented on pull request "doc: DummySignInput mention external signer":
(https://github.com/bitcoin/bitcoin/pull/27180#issuecomment-1458722234)
cc @S3RK @ishaanam @furszy

💬 MarcoFalke commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1458725581)
Changes:

* Adding a wrong hunk? https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1123429117
* Fixing my feedback from https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1071035557
* Adding commit 7673fd0b41

re-ACK 142718890e718397e0026c315c8940102b9657ce 📬

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDD
...

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1458734131)
>Instead of being a string, wouldn't make sense it to be an array? E.g. I want to get new/tried table address count for ipv5 and ipv6 together.

i'm confused about this since making it into `RPCArg::Type::ARR` would make the RPC harder to type and use?
`$ bitcoin-cli getaddrmaninfo "[\"ipv4\", \"ipv6\"]"`

and there are only few network types. would be interested in what others think.

💬 fanquake commented on pull request "refactor: Split util/system into exception, shell, and fs-specific files":
(https://github.com/bitcoin/bitcoin/pull/25152#issuecomment-1458740529)
> I'm still keen on moving this work forward. If you don't have the time right now, I'll attribute the commits to you and try to get my branch merged instead.

Yep, it would be good to start getting some of this merged.

💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1128487451)
Feels like there must be a way to express this without rewriting the filter? I think the only reason you need to do that here is because `list(coinbase_filter)` exhausts the iterator so you need to start it over. What if `coinbase_filter` was actually a list called `mature_coins` from the beginning of the function? I dunno, just an idea.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1128491150)
I'll keep this as is, to me this reads easier. We also do use the options struct before to read the fastprune flag.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1458819504)
Thank you for the review! These are valuable and I hope they allow me to improve future commits. I'm pushing changes addressing comments up to the 4th commit now.

Updated a1db3966d1c4f4213d7bfd6be0ea7fb308fe4e11 -> 65d026b47270c2fb9d2dab7f3d448b4abc37b93a ([tc/2022-09-libbitcoinkernel-chainparams-args_0](https://github.com/TheCharlatan/bitcoin/commits/tc/2022-09-libbitcoinkernel-chainparams-args_0) -> [tc/2022-09-libbitcoinkernel-chainparams-args_1](https://github.com/TheCharlatan/bitcoin/co
...

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1128545452)
Thank you for spelling this out, I updated the description.

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1128547248)
Good catch, updated. [diff](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_0..tc/2022-09-libbitcoinkernel-chainparams-args_1)

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1128549764)
Thank you for taking the time to get into this. I hope the update I pushed is more in style. [diff](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_0..tc/2022-09-libbitcoinkernel-chainparams-args_1)

💬 TheCharlatan commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#discussion_r1128550399)
Good point, updated. [diff](https://github.com/TheCharlatan/bitcoin/compare/tc/2022-09-libbitcoinkernel-chainparams-args_0..tc/2022-09-libbitcoinkernel-chainparams-args_1)

💬 jonatack commented on pull request "refactor / kernel: Move non-gArgs chainparams functionality to kernel":
(https://github.com/bitcoin/bitcoin/pull/26177#issuecomment-1458861089)
Concept ACK, thanks for working on this.

💬 jamesob commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1458876649)
> Missed this on last review (sorry):

Fixed!

💬 willcl-ark commented on issue "Rescanning use only 1 core":
(https://github.com/bitcoin/bitcoin/issues/19992#issuecomment-1458921201)
@MarcoFalke this can be closed as `zapwallettxes` is removed?

💬 junderw commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1458921569)
> then could someone please do it? (you?)

There are many C++ devs you could pay to have this PR done for you. That would be much quicker, especially considering how simple the change is. The only hard part might be writing tests, since the testing harness etc. does take a bit of getting used to... (that said, looking at the other tests usually can give one a good head start)

There are also many bounty websites where you can lock BTC into a vault that is only released to the person who does
...

📝 ryanofsky opened a pull request: "refactor: Remove CAddressBookData::destdata"
(https://github.com/bitcoin/bitcoin/pull/27224)
This is cleanup that doesn't change external behavior. Benefits of the cleanup are:

- Removes awkward `StringMap` intermediate representation for wallet address metadata.
- Simplifies `CWallet`, deals with used address and received request serialization in `walletdb.cpp` instead of higher level wallet code
- Adds test coverage

This PR doesn't change externally observable behavior. Internally, the only change in behavior is that `EraseDestData` deletes rows directly from the database beca
...

💬 junderw commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1458924629)
@MarcoFalke perhaps this could also use a "good first issue" tag? Seems pretty straight forward.