💬 pinheadmz commented on issue "Log X-Forwarded-For for rpc ":
(https://github.com/bitcoin/bitcoin/issues/9397#issuecomment-1499239728)
I think this issue can be closed as wont-fix. However, I think the only docs we have regarding stunnel are in [the 0.12 release notes](https://github.com/esotericnonsense/bitcoin/blob/b583b3993a0e22d8adfed477077dc9b889f8b2ad/doc/release-notes/release-notes-0.12.0.md#rpc-ssl-support-dropped) and were not sufficient for me trying to follow them (we could mention the stunnel conf file, firewall rules, etc -- not to mention @laanwj suggestions in the above comment).
(https://github.com/bitcoin/bitcoin/issues/9397#issuecomment-1499239728)
I think this issue can be closed as wont-fix. However, I think the only docs we have regarding stunnel are in [the 0.12 release notes](https://github.com/esotericnonsense/bitcoin/blob/b583b3993a0e22d8adfed477077dc9b889f8b2ad/doc/release-notes/release-notes-0.12.0.md#rpc-ssl-support-dropped) and were not sufficient for me trying to follow them (we could mention the stunnel conf file, firewall rules, etc -- not to mention @laanwj suggestions in the above comment).
💬 pinheadmz commented on issue "verifytxoutproof can return witnesses but cannot verify them":
(https://github.com/bitcoin/bitcoin/issues/11242#issuecomment-1499265142)
This is a good idea and seems like something we should support. The output would need to prove the entire coinbase tx and then include a wtxid proof to the witness commitment. `merkleblock` is well-documented but this extra proof might need a BIP spec even if its just for an RPC.
Are there any other uses for merkleblock proofs outside of SPV wallets? There hasn't been any more discussion on this issue for 6 years and we have client-side filtering available now which doesn't require proofs at
...
(https://github.com/bitcoin/bitcoin/issues/11242#issuecomment-1499265142)
This is a good idea and seems like something we should support. The output would need to prove the entire coinbase tx and then include a wtxid proof to the witness commitment. `merkleblock` is well-documented but this extra proof might need a BIP spec even if its just for an RPC.
Are there any other uses for merkleblock proofs outside of SPV wallets? There hasn't been any more discussion on this issue for 6 years and we have client-side filtering available now which doesn't require proofs at
...
💬 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.
(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` ?
(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
...
(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 ;-)
(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
(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
(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
...
(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
(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
...
(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.
(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.
(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!
(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.
(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:
...
(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` .
(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!
(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
...
(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
(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