π¬ darosior commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#issuecomment-3474716867)
> See #33498.
This is only one such vector. It's good to have a warning until we can be reasonably confident that it's not trivially possible to link two different network addresses of a node. I am also skeptical we could ever reach this level of confidence.
(https://github.com/bitcoin/bitcoin/pull/33750#issuecomment-3474716867)
> See #33498.
This is only one such vector. It's good to have a warning until we can be reasonably confident that it's not trivially possible to link two different network addresses of a node. I am also skeptical we could ever reach this level of confidence.
π¬ roconnor-blockstream commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3474717031)
> If there were real coins locked due to this policy, it seems like they should have been already recovered via getting a non-standard spend mined directly.
I don't know why you would think that they should have already been recovered. You think anyone can just call up a mining pool and get their non-standard transactions mined? How much money needs to be at stake before you can even get a pool's attention? Or how small in value do UTXOs have to be so that it is okay to soft-confiscate it
...
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3474717031)
> If there were real coins locked due to this policy, it seems like they should have been already recovered via getting a non-standard spend mined directly.
I don't know why you would think that they should have already been recovered. You think anyone can just call up a mining pool and get their non-standard transactions mined? How much money needs to be at stake before you can even get a pool's attention? Or how small in value do UTXOs have to be so that it is okay to soft-confiscate it
...
π€ janb84 reviewed a pull request: "refactor: unify container presence checks"
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3405953584)
re ACK 21cff12b0e6dc58fb8c66543bbd713796d9941ae
I still think this a good idea, it may be a "style change" but the increased clarity will decrease bugs and mental load when reading the code.
changes since last ack:
- reduced changes to not hit cluster mempool.
(https://github.com/bitcoin/bitcoin/pull/33192#pullrequestreview-3405953584)
re ACK 21cff12b0e6dc58fb8c66543bbd713796d9941ae
I still think this a good idea, it may be a "style change" but the increased clarity will decrease bugs and mental load when reading the code.
changes since last ack:
- reduced changes to not hit cluster mempool.
π€ jonatack reviewed a pull request: "doc: document fingerprinting risk when operating node on multiple networks"
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3406173039)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33750#pullrequestreview-3406173039)
Concept ACK
π¬ jonatack commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482681588)
Idem.
```suggestion
- Operating a node that listens on multiple networks (e.g., Tor and I2P) can help
```
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482681588)
Idem.
```suggestion
- Operating a node that listens on multiple networks (e.g., Tor and I2P) can help
```
π¬ jonatack commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482683578)
While the study linked to in the OP described IPv4 and Tor, I suspect the most frequent multiple network pairing would be Tor and I2P, as privacy network users might opt to avoid clearnet (say, the kind of node operator that pre-I2P may have been running over Tor only).
Suggested change
- Operating a node that listens on multiple networks (e.g. IPv4 and I2P) can help
...
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482683578)
While the study linked to in the OP described IPv4 and Tor, I suspect the most frequent multiple network pairing would be Tor and I2P, as privacy network users might opt to avoid clearnet (say, the kind of node operator that pre-I2P may have been running over Tor only).
Suggested change
- Operating a node that listens on multiple networks (e.g. IPv4 and I2P) can help
...
π¬ polespinasa commented on pull request "rpc: Optionally print feerates in sat/vb":
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2482777067)
> Generally, it is best to write unit or fuzz tests for newly written code
Done
> Also, I am not sure if `CFeeRate` should be bundled with univalue. The conversion could be a standalone helper, but this is just a nit.
I decided to make it a function of `CFeeRate` because is the only class that should use it. But I'm ok moving it to `core_writte`.
(https://github.com/bitcoin/bitcoin/pull/33741#discussion_r2482777067)
> Generally, it is best to write unit or fuzz tests for newly written code
Done
> Also, I am not sure if `CFeeRate` should be bundled with univalue. The conversion could be a standalone helper, but this is just a nit.
I decided to make it a function of `CFeeRate` because is the only class that should use it. But I'm ok moving it to `core_writte`.
π€ jonatack reviewed a pull request: "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency"
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3406254553)
Approach ACK 016ab85a13408ad980c3dbce4e041e14a4fcf3b8, thanks for updating, tested with a server on master, not yet tested with a server running pre-v26 (before getaddrmaninfo)
```
$ ./build/bin/bitcoin-cli -addrinfo
{
"all addresses known (used for selecting peers)": {
"ipv4": 48133,
"ipv6": 8017,
"onion": 17257,
"i2p": 3757,
"cjdns": 11,
"total": 77175
},
"addresses known after quality/recency filtering (for original -addrinfo compatibility)": {
"ipv4": 45921
...
(https://github.com/bitcoin/bitcoin/pull/26988#pullrequestreview-3406254553)
Approach ACK 016ab85a13408ad980c3dbce4e041e14a4fcf3b8, thanks for updating, tested with a server on master, not yet tested with a server running pre-v26 (before getaddrmaninfo)
```
$ ./build/bin/bitcoin-cli -addrinfo
{
"all addresses known (used for selecting peers)": {
"ipv4": 48133,
"ipv6": 8017,
"onion": 17257,
"i2p": 3757,
"cjdns": 11,
"total": 77175
},
"addresses known after quality/recency filtering (for original -addrinfo compatibility)": {
"ipv4": 45921
...
π¬ jonatack commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2482731134)
```suggestion
- CLI -addrinfo previously (v22 - v30) returned addresses known to the node after filtering for quality and recency.
```
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2482731134)
```suggestion
- CLI -addrinfo previously (v22 - v30) returned addresses known to the node after filtering for quality and recency.
```
π¬ jonatack commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2482783485)
```suggestion
* - getnodeaddresses (v22.0+) for stats of addresses in the addrman filtered by quality (i.e. not IsTerrible), used by -addrinfo since v22.0 and kept for data continuity)
```
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r2482783485)
```suggestion
* - getnodeaddresses (v22.0+) for stats of addresses in the addrman filtered by quality (i.e. not IsTerrible), used by -addrinfo since v22.0 and kept for data continuity)
```
π¬ da1sychain commented on pull request "doc: document fingerprinting risk when operating node on multiple networks":
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482805132)
This combination, IMO, poses less risk to the privacy-concerned individual as they are exposing their node over two privacy networks. In fact I think the risk in this configuration would be lean more towards isolation / partition.
(https://github.com/bitcoin/bitcoin/pull/33750#discussion_r2482805132)
This combination, IMO, poses less risk to the privacy-concerned individual as they are exposing their node over two privacy networks. In fact I think the risk in this configuration would be lean more towards isolation / partition.
π¬ Pantamis commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3475216642)
Hello there !
I finally had the motivation to upgrade my node to run the last version of this PR and sync the new `txospenderindex` since the last time in... 2022 πΆβπ«οΈ !
Here are the size of the indexes once synced at blockheight 921677:
<img width="363" height="53" alt="Capture dβeΜcran 2025-11-01 aΜ 00 03 40" src="https://github.com/user-attachments/assets/b0e69f58-449e-4000-804f-00a9e2aa4342" />
Great to see it is now comparable with the usual `txindex` even with the surge of tx
...
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3475216642)
Hello there !
I finally had the motivation to upgrade my node to run the last version of this PR and sync the new `txospenderindex` since the last time in... 2022 πΆβπ«οΈ !
Here are the size of the indexes once synced at blockheight 921677:
<img width="363" height="53" alt="Capture dβeΜcran 2025-11-01 aΜ 00 03 40" src="https://github.com/user-attachments/assets/b0e69f58-449e-4000-804f-00a9e2aa4342" />
Great to see it is now comparable with the usual `txindex` even with the surge of tx
...
π¬ fjahr commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3475259644)
> and it feels like whichever default you pick shouldn't matter too much because it will change again soon when asmap data is embedded? (I'm assuming when asmap data is embedded, the default will be to use embedded data unless you specify -noasmap or a filename.)
The long term plan is to have the embedded data used by default, however first the idea was to have a release or two with the data embedded but the usage still off by default. This is a cautious intermediate step similar to how assum
...
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3475259644)
> and it feels like whichever default you pick shouldn't matter too much because it will change again soon when asmap data is embedded? (I'm assuming when asmap data is embedded, the default will be to use embedded data unless you specify -noasmap or a filename.)
The long term plan is to have the embedded data used by default, however first the idea was to have a release or two with the data embedded but the usage still off by default. This is a cautious intermediate step similar to how assum
...
π¬ ajtowns commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3475291617)
> > If there were real coins locked due to this policy, it seems like they should have been already recovered via getting a non-standard spend mined directly.
>
> I don't know why you would think that they should have already been recovered. You think anyone can just call up a mining pool and get their non-standard transactions mined?
That's what things like slipstream allow today, so, yes? Template building by hashers also make that more accessible today than previously. In any event, if
...
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3475291617)
> > If there were real coins locked due to this policy, it seems like they should have been already recovered via getting a non-standard spend mined directly.
>
> I don't know why you would think that they should have already been recovered. You think anyone can just call up a mining pool and get their non-standard transactions mined?
That's what things like slipstream allow today, so, yes? Template building by hashers also make that more accessible today than previously. In any event, if
...
π¬ brunoerg commented on issue "Wallet fuzzing tracking issue":
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-3475368672)
`wallet_create_transaction` and `scriptpubkeyman` are two targets that I'm taking a look to try to speed them up. I'm getting less than 10 exec/s for both of them on my personal server which is bad. Just added to the "Nice to have" section the "Speed up current targets".
(https://github.com/bitcoin/bitcoin/issues/29901#issuecomment-3475368672)
`wallet_create_transaction` and `scriptpubkeyman` are two targets that I'm taking a look to try to speed them up. I'm getting less than 10 exec/s for both of them on my personal server which is bad. Just added to the "Nice to have" section the "Speed up current targets".
π¬ sdaftuar commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3475369269)
Looks like there's a thread safety issue ([CI run in #33591 caught it](https://github.com/bitcoin/bitcoin/actions/runs/18986107827/job/54230018048?pr=33591)); if I understand correctly, it looks like in `AcceptSingleTransaction()` we acquire the lock and release it before the `MemPoolAccept` object is destroyed, causing a change to txgraph (when the changeset is destroyed) while the lock is not held. Will fix.
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3475369269)
Looks like there's a thread safety issue ([CI run in #33591 caught it](https://github.com/bitcoin/bitcoin/actions/runs/18986107827/job/54230018048?pr=33591)); if I understand correctly, it looks like in `AcceptSingleTransaction()` we acquire the lock and release it before the `MemPoolAccept` object is destroyed, causing a change to txgraph (when the changeset is destroyed) while the lock is not held. Will fix.
π roconnor-blockstream opened a pull request: "Fix BIP143 standardness rules for CHECKMULTISIG"
(https://github.com/bitcoin/bitcoin/pull/33759)
From https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki#restrictions-on-public-key-type
> As a default policy, only compressed public keys are accepted in P2WPKH and
> P2WSH. Each public key passed to a sigop inside version 0 witness program must
> be a compressed key: the first byte MUST be either 0x02 or 0x03, and the size
> MUST be 33 bytes. Transactions that break this rule will not be relayed or mined
> by default.
PR #8499 's implemenation is insufficent to meet BIP14
...
(https://github.com/bitcoin/bitcoin/pull/33759)
From https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki#restrictions-on-public-key-type
> As a default policy, only compressed public keys are accepted in P2WPKH and
> P2WSH. Each public key passed to a sigop inside version 0 witness program must
> be a compressed key: the first byte MUST be either 0x02 or 0x03, and the size
> MUST be 33 bytes. Transactions that break this rule will not be relayed or mined
> by default.
PR #8499 's implemenation is insufficent to meet BIP14
...
π¬ roconnor-blockstream commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3475458507)
Those outputs look like multisigs that are composed out of valid compressed or uncompressed pubkeys, and I believe spending them would pass standardness rules (except for 3482ea1d219120b13f3be49f63fd7079136785dff2bf7487726b640da5876e33:0, which appears to simply be a broken script).
The UTXO's I'm looking for would have pubkeys with an invalid prefix, or an invalid length for their prefix.
Based on the code prior to https://github.com/CounterpartyXCP/counterparty-core/pull/443/commits/f1a8
...
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3475458507)
Those outputs look like multisigs that are composed out of valid compressed or uncompressed pubkeys, and I believe spending them would pass standardness rules (except for 3482ea1d219120b13f3be49f63fd7079136785dff2bf7487726b640da5876e33:0, which appears to simply be a broken script).
The UTXO's I'm looking for would have pubkeys with an invalid prefix, or an invalid length for their prefix.
Based on the code prior to https://github.com/CounterpartyXCP/counterparty-core/pull/443/commits/f1a8
...
π¬ optout21 commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3475797299)
reACK 21cff12b0e6dc58fb8c66543bbd713796d9941ae
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3475797299)
reACK 21cff12b0e6dc58fb8c66543bbd713796d9941ae
π¬ optout21 commented on pull request "transaction: Adding script witness to ToString for CTxIn":
(https://github.com/bitcoin/bitcoin/pull/33711#issuecomment-3475810317)
Concept ACK 61cd0e53ca45684cd5ef9084829ecc6d59154595
I notice that `CTransaction::ToString()` prints the inputs and the input script witnesses additionally, with that change script witnesses will be included duplicated. The order there is first all inputs, then all witnesses. Please consider removing the duplication.
Options:
- keep the duplication
- remove duplication, change the order
- remove the duplication and keep the order -- there should be a version of `CTxIn::ToString()` withou
...
(https://github.com/bitcoin/bitcoin/pull/33711#issuecomment-3475810317)
Concept ACK 61cd0e53ca45684cd5ef9084829ecc6d59154595
I notice that `CTransaction::ToString()` prints the inputs and the input script witnesses additionally, with that change script witnesses will be included duplicated. The order there is first all inputs, then all witnesses. Please consider removing the duplication.
Options:
- keep the duplication
- remove duplication, change the order
- remove the duplication and keep the order -- there should be a version of `CTxIn::ToString()` withou
...