💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1499322028)
ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635

The only change is my previous comment addressed about using current variable to determine change output size.

💬 pinheadmz commented on issue "MAX_BLOCK_SERIALIZED_SIZE is not a consensus-enforced constant":
(https://github.com/bitcoin/bitcoin/issues/10289#issuecomment-1499351182)
Could the useage sites be switched to `MAX_BLOCK_WEIGHT`?

> an expression that computes it from consensus constants.

What would this expression be besides `MAX_BLOCK_WEIGHT * 1` ?

📝 theStack opened a pull request: "contrib: add tool to convert compact-serialized UTXO set to SQLite database"
(https://github.com/bitcoin/bitcoin/pull/27432)
## Problem description

There is demand from users to get the UTXO set in form of a SQLite database (#24628). Bitcoin Core currently only supports dumping the UTXO set in a binary _compact-serialized_ format, which was crafted specifically for AssumeUTXO snapshots (see PR #16899), with the primary goal of being as compact as possible. Previous PRs tried to extend the `dumptxoutset` RPC with new formats, either in human-readable form (e.g. #18689, #24202), or most recently, directly as SQLite d
...

💬 pinheadmz commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1499430406)
This also closes https://github.com/bitcoin/bitcoin/issues/21670 ;-)

💬 theStack commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1499432405)
Pinging users who worked on or reviewed / expressed interest in the previous attempt to solve this issue (PR #24952):
@dunxen @0xB10C @jamesob @prusnak @willcl-ark @w0xlt @jonatack @brunoerg @laanwj @fanquake

🤔 Xekyo reviewed a pull request: "wallet: coin selection, don't return results that exceed the max allowed weight"
(https://github.com/bitcoin/bitcoin/pull/26720#pullrequestreview-1375347359)
reACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635

📝 Sjors opened a pull request: "getblocktemplate improvements for segwit and sigops"
(https://github.com/bitcoin/bitcoin/pull/27433)
**Sigops**

Two recent F2Pool violated the sigops limit. Both by 3.

I suspect they were not using `getblocktemplate`. If you look at the [template](https://miningpool.observer/template-and-block/00000000000000000004e0ec4f27bd3347381e8e19ed98d7f918e8c1c292ae97) right before the valid block at the same height, which was produced two minutes later, you'll see that it matches the block with 3 small transactions difference. In other words, the valid block producer likely did use `getblocktemplat
...

💬 LarryRuane commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1499452625)
ACK 93c70287a6434c6c665a211dc4dfbbd9c3db4083

💬 stratospher commented on pull request "p2p: skip netgroup diversity of new connections for tor/i2p/cjdns":
(https://github.com/bitcoin/bitcoin/pull/27374#issuecomment-1499478379)
Thanks a lot @sdaftuar! I've used the simplified approach in your patch!

> Counting multiple sources from the same alternative network, they can fill up a theoretical maximum of 256 buckets (~16k addresses) or 1/4 of the new tables I think (but due to collisions from the the modulo operations in GetNewBucket and GetBucketPosition that depend on nKey, it is typically more like ~200 buckets in reality).
On the other hand, multiple IPv4 and IPv6 sources can fill up all 1024 buckets in the new
...

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1499500913)
Fixed co-authorship attribution to @furszy, no other changes.

💬 hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#discussion_r1160163588)
You're right. Going to amend the patch.

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1160166441)
Thanks!

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1160167278)
Thanks. I'll squashed this into another commit.

💬 Sjors commented on pull request "getblocktemplate improvements for segwit and sigops":
(https://github.com/bitcoin/bitcoin/pull/27433#issuecomment-1499509064)
The "rules" field was introduced in #7935. It's not part of [BIP 22](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki), [BIP 23](https://github.com/bitcoin/bips/blob/master/bip-0023.mediawiki) or [BIP 145](https://github.com/bitcoin/bips/blob/master/bip-0145.mediawiki) which describe the expected behaviour of `getblocktemplate`.

It's unclear to me how it is intented to be used, both as an argument and in the results. I'm also not sure how to interpret this wording in BIP 145:

...

💬 theuni commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1499550445)
Thanks for the review @TheCharlatan and @josibake. I've addressed most of the feedback with the exception of a few nits as I don't want this to drag out forever.

I squashed some of the readme changes to keep the number of commits down. The diff from the previous changes can be seen with: `git diff 96344222f3..754fb6bb81` .

🤔 ryanofsky reviewed a pull request: "refactor: Remove CAddressBookData::destdata"
(https://github.com/bitcoin/bitcoin/pull/27224#pullrequestreview-1373581514)
Updated 0ad6a533b93162ec808de092cbe5edc31a27a390 -> d34323544d7283bf46bb154a90b8feca443b103e ([`pr/nodest.18`](https://github.com/ryanofsky/bitcoin/commits/pr/nodest.18) -> [`pr/nodest.19`](https://github.com/ryanofsky/bitcoin/commits/pr/nodest.19), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nodest.18..pr/nodest.19)) applying most of the suggested changes. Thanks for the reviews!

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1158949529)
re: https://github.com/bitcoin/bitcoin/pull/27224/files#r1150232826

> you have a reason to prefer `Span<std::byte>`?

Well it should actually take a span of const bytes, so I fixed this to add const.

Passing a span makes more sense than passing a datastream because the function just needs a read-only array of bytes, it doesn't need a read/write vector or a stream object or anything more complicated.

The other functions are using CDataStream just because they are called by older code w
...

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1160135564)
re: https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150238393

> to make this an `rvalue`. there is likely a cleaner way to do this, if you end up going with `DataStream`

Thanks, responded to earlier comment

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1160133066)
re: https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150513279

> nit: could use structured bindings.

Thanks, applied suggestion

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1160150468)
> No longer the case. Same for the comment that is above this one.

Thanks, I basically just replaced "destdata" with "address data" in these comments, but you can let me know if the issue was something else