🤔 glozow reviewed a pull request: "wallet: when a block is disconnected, update transactions that are no longer conflicted"
(https://github.com/bitcoin/bitcoin/pull/27145)
Concept ACK
There is a missing dash in "Co-authored-by:" in the commit messages
(https://github.com/bitcoin/bitcoin/pull/27145)
Concept ACK
There is a missing dash in "Co-authored-by:" in the commit messages
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159806134)
Sorry to bikeshed, but I think the naming is a bit hard to follow ("conflicted" vs "conflicting" don't immediately make sense to me, especially since both sets of transactions get conflicted at some point during the test). Might I suggest names that indicate which input(s) they spend, which node broadcasts them, and what "generation" they are? For example
`tx_AB_0_parent` instead of `conflicted`
`tx_A_1` instead of `conflicting_tx_A`
`tx_B_1` instead of `conflicting_tx_B`
`C` instead of
...
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159806134)
Sorry to bikeshed, but I think the naming is a bit hard to follow ("conflicted" vs "conflicting" don't immediately make sense to me, especially since both sets of transactions get conflicted at some point during the test). Might I suggest names that indicate which input(s) they spend, which node broadcasts them, and what "generation" they are? For example
`tx_AB_0_parent` instead of `conflicted`
`tx_A_1` instead of `conflicting_tx_A`
`tx_B_1` instead of `conflicting_tx_B`
`C` instead of
...
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159787578)
nit in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb, I think this is clearer (same with the other arrays of inputs and outputs)
```suggestion
inputs_conflicted_tx = [{"txid": txid_conflict_from_1, "vout": nA}, {"txid": txid_conflict_from_2, "vout": nB}]
```
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159787578)
nit in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb, I think this is clearer (same with the other arrays of inputs and outputs)
```suggestion
inputs_conflicted_tx = [{"txid": txid_conflict_from_1, "vout": nA}, {"txid": txid_conflict_from_2, "vout": nB}]
```
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159765529)
in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb:
I think this comment is a little bit hard to follow. Something more descriptive might be
```suggestion
"""
Test that wallet correctly tracks transactions that have been conflicted by blocks, particularly during reorgs.
"""
```
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159765529)
in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb:
I think this comment is a little bit hard to follow. Something more descriptive might be
```suggestion
"""
Test that wallet correctly tracks transactions that have been conflicted by blocks, particularly during reorgs.
"""
```
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159807735)
```suggestion
self.sync_blocks()
```
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159807735)
```suggestion
self.sync_blocks()
```
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159814534)
in commit 49f8af533f2e6370f4ae5220f5c706e30b90d06c
no need for const reference to integer
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159814534)
in commit 49f8af533f2e6370f4ae5220f5c706e30b90d06c
no need for const reference to integer
💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159784792)
We should also test who "wins" in this conflict contest and what the values are, i.e. conflicting A has 8 confirmations and conflicting B has 4 confirmations.
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159784792)
We should also test who "wins" in this conflict contest and what the values are, i.e. conflicting A has 8 confirmations and conflicting B has 4 confirmations.
💬 pinheadmz commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1499130274)
> the only solution is to do a full resync and download all the way back to N-N (aka 0).
There is also `getblockfrompeer` https://github.com/bitcoin/bitcoin/pull/20295 I just tried this on my pruned node to inspect blocks below the prune depth, pretty neat!
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1499130274)
> the only solution is to do a full resync and download all the way back to N-N (aka 0).
There is also `getblockfrompeer` https://github.com/bitcoin/bitcoin/pull/20295 I just tried this on my pruned node to inspect blocks below the prune depth, pretty neat!
🤔 furszy reviewed a pull request: "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/25193#pullrequestreview-1375021974)
Instead of the global flag that requires many manual sets at different locations and the indexes threads active wait, what if we move the indexes threads start after the loading process? e.g. https://github.com/furszy/bitcoin-core/commit/1e48179f70dafff8fad31b4048681103cca90b56.
It makes code shorter and more robust. Plus, it let us keep the pruning checks as well.
(https://github.com/bitcoin/bitcoin/pull/25193#pullrequestreview-1375021974)
Instead of the global flag that requires many manual sets at different locations and the indexes threads active wait, what if we move the indexes threads start after the loading process? e.g. https://github.com/furszy/bitcoin-core/commit/1e48179f70dafff8fad31b4048681103cca90b56.
It makes code shorter and more robust. Plus, it let us keep the pruning checks as well.
💬 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
...