💬 tdb3 commented on issue "init: torcontrol argument should be validated":
(https://github.com/bitcoin/bitcoin/issues/23589#issuecomment-2212578536)
Looks like we currently allow lookups for the `-torcontrol` host based on the state of `-dns` (default 1) (see below). If we don't want to change that (e.g. to disallow allow hostnames, and allow only IP addresses), we're somewhat limited in what would be considered invalid for `-torcontrol`. I'm thinking this gets clarified this first.
https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/torcontrol.cpp#L151
(https://github.com/bitcoin/bitcoin/issues/23589#issuecomment-2212578536)
Looks like we currently allow lookups for the `-torcontrol` host based on the state of `-dns` (default 1) (see below). If we don't want to change that (e.g. to disallow allow hostnames, and allow only IP addresses), we're somewhat limited in what would be considered invalid for `-torcontrol`. I'm thinking this gets clarified this first.
https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/torcontrol.cpp#L151
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667779316)
No, the serializer is serializing the `this` object; the `rebuild` object represents what the deserializer already knows when it has deserialized up to whatever has been serialized already.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667779316)
No, the serializer is serializing the `this` object; the `rebuild` object represents what the deserializer already knows when it has deserialized up to whatever has been serialized already.
👍 hodlinator approved a pull request: "random: add benchmarks and drop unnecessary Shuffle function"
(https://github.com/bitcoin/bitcoin/pull/30396#pullrequestreview-2161950964)
ACK 6ecda04fefad980872c72fba89844393f5581120
Thanks for taking the time to add valuable context!!
(I read up a bit, understand you implemented a 32-bit version of Daniel Lemire's nearly divisionless algo. Nice).
> I did notice that `std::shuffle` is a bit slower than the custom Shuffle on my system too (AMD 5950X, GCC 13.2)
Maybe worth adding some nuance to the PR summary which currently says "now that it appears that standard library `std::shuffle` beats it.".
### # `rand64()` invo
...
(https://github.com/bitcoin/bitcoin/pull/30396#pullrequestreview-2161950964)
ACK 6ecda04fefad980872c72fba89844393f5581120
Thanks for taking the time to add valuable context!!
(I read up a bit, understand you implemented a 32-bit version of Daniel Lemire's nearly divisionless algo. Nice).
> I did notice that `std::shuffle` is a bit slower than the custom Shuffle on my system too (AMD 5950X, GCC 13.2)
Maybe worth adding some nuance to the PR summary which currently says "now that it appears that standard library `std::shuffle` beats it.".
### # `rand64()` invo
...
💬 cobratbq commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#issuecomment-2212614821)
> Do you happen to know which pull request added a new positional argument in the middle? Or was this instruction always broken? cc @achow101
I don't know. I'm new to this. However, in working with this documentation, it seems that there are more discrepancies or inaccuracies. It seems now the account-ID in `getdescriptors` is provided using `--account 0`, for example.
I'm currently going through the bitcoin-core sources, to figure out where my other mistakes are, because the _external-sig
...
(https://github.com/bitcoin/bitcoin/pull/30354#issuecomment-2212614821)
> Do you happen to know which pull request added a new positional argument in the middle? Or was this instruction always broken? cc @achow101
I don't know. I'm new to this. However, in working with this documentation, it seems that there are more discrepancies or inaccuracies. It seems now the account-ID in `getdescriptors` is provided using `--account 0`, for example.
I'm currently going through the bitcoin-core sources, to figure out where my other mistakes are, because the _external-sig
...
💬 cobratbq commented on pull request "doc: external-signer, example 'createwallet' RPC call requires eight argument":
(https://github.com/bitcoin/bitcoin/pull/30354#discussion_r1667792505)
New version pushed with `blank`, `avoid_reuse` and `load_on_startup` removed.
(https://github.com/bitcoin/bitcoin/pull/30354#discussion_r1667792505)
New version pushed with `blank`, `avoid_reuse` and `load_on_startup` removed.
💬 pythcoiner commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2212989535)
I'm not sure if it's the intended behavior, but passing the same `NUM` twice (here `<1;1>`) to `importdescriptors` does not fail and ends up with a single change descriptor:
```python
self.log.info("Multipath descriptors: duplicate NUM")
self.nodes[1].createwallet(wallet_name="multipath3", descriptors=True, blank=True)
w_multipath = self.nodes[1].get_wallet_rpc("multipath3")
self.test_importdesc({"desc": descsum_create(f"wpkh({xpriv}/<1;1>/*)"),
"active": True,
...
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2212989535)
I'm not sure if it's the intended behavior, but passing the same `NUM` twice (here `<1;1>`) to `importdescriptors` does not fail and ends up with a single change descriptor:
```python
self.log.info("Multipath descriptors: duplicate NUM")
self.nodes[1].createwallet(wallet_name="multipath3", descriptors=True, blank=True)
w_multipath = self.nodes[1].get_wallet_rpc("multipath3")
self.test_importdesc({"desc": descsum_create(f"wpkh({xpriv}/<1;1>/*)"),
"active": True,
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667968459)
Ok, but is there no C++-fu incantation that can get us from a `std::pair<const COutpoint, CCoinsCacheEntry>*` to a `CCoinsMap::iterator`? I can't figure out a way to get that to compile but I'm sure there's a way using `void*` or something.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667968459)
Ok, but is there no C++-fu incantation that can get us from a `std::pair<const COutpoint, CCoinsCacheEntry>*` to a `CCoinsMap::iterator`? I can't figure out a way to get that to compile but I'm sure there's a way using `void*` or something.
💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667970205)
It might be possible to construct something that works for a specific libstdc++ or libc++ version, but it would be very ugly.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667970205)
It might be possible to construct something that works for a specific libstdc++ or libc++ version, but it would be very ugly.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667977013)
Can you point me to the way of that ugliness? :pray:
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1667977013)
Can you point me to the way of that ugliness? :pray:
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667979626)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667979626)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980741)
From the perspective of a `DepGraph`, transactions are identified by their position in the entry vector (which matches their position in the `Cluster` they came from). `DepGraph`'s equality tests whether every transaction in every position matches; there is nothing else it knows about.
I've added a comment to `DepGraph::operator==` to indicate it's primarily for testing.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980741)
From the perspective of a `DepGraph`, transactions are identified by their position in the entry vector (which matches their position in the `Cluster` they came from). `DepGraph`'s equality tests whether every transaction in every position matches; there is nothing else it knows about.
I've added a comment to `DepGraph::operator==` to indicate it's primarily for testing.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980882)
Added an `Assume` for that.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980882)
Added an `Assume` for that.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980947)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667980947)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981357)
I've pushed this simplification; it works a lot different now, but `CanAddDependency`, `GetReducedParents`, and `GetReducedChildren` are gone.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981357)
I've pushed this simplification; it works a lot different now, but `CanAddDependency`, `GetReducedParents`, and `GetReducedChildren` are gone.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981485)
It's gone.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981485)
It's gone.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981720)
With the format change, I've documented every byte of the encoding in the tests here.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981720)
With the format change, I've documented every byte of the encoding in the tests here.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981849)
Good catch, fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981849)
Good catch, fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981943)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667981943)
Fixed.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667982100)
Added a comment.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1667982100)
Added a comment.
💬 maflcko commented on pull request "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs":
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-2213093829)
rebased
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-2213093829)
rebased