💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1629155080)
> a majority of my network specific connection attempts were to IPV6 addresses, even though I don't have an IPV6 address and can't make connections to it
Yeah. That is also an issue on `master`, without this PR. Maybe it deserves a separate PR. Opened https://github.com/bitcoin/bitcoin/issues/28061 to track that.
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1629155080)
> a majority of my network specific connection attempts were to IPV6 addresses, even though I don't have an IPV6 address and can't make connections to it
Yeah. That is also an issue on `master`, without this PR. Maybe it deserves a separate PR. Opened https://github.com/bitcoin/bitcoin/issues/28061 to track that.
🤔 ishaanam reviewed a pull request: "wallet: do not backdate locktime if it may lead to fingerprinting"
(https://github.com/bitcoin/bitcoin/pull/26902#pullrequestreview-1522310111)
This is a good start, but I think that these transactions would be harder to fingerprint if the following modifications were made:
1. For RBF transactions: the `nLockTime` is set to the same value as the transaction being replaced.
2. For spending unconfirmed utxos: Use the same behavior as before, but make sure that the `nLockTIme` value is greater than or equal to the highest `nLockTIme` of the inputs.
For convenience I've implemented this here: https://github.com/ishaanam/bitcoin/commi
...
(https://github.com/bitcoin/bitcoin/pull/26902#pullrequestreview-1522310111)
This is a good start, but I think that these transactions would be harder to fingerprint if the following modifications were made:
1. For RBF transactions: the `nLockTime` is set to the same value as the transaction being replaced.
2. For spending unconfirmed utxos: Use the same behavior as before, but make sure that the `nLockTIme` value is greater than or equal to the highest `nLockTIme` of the inputs.
For convenience I've implemented this here: https://github.com/ishaanam/bitcoin/commi
...
💬 furszy commented on issue "migratewallet crashes (wallet/scriptpubkeyman.cpp:1915: std::optional<MigrationData> wallet::LegacyScriptPubKeyMan::MigrateToDescriptor(): Assertion `IsMine(desc_spk) != ISMINE_NO' failed.)":
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1629158947)
Ok, found the `Unrecognized descriptor found` error cause. You might have imported a hex script and the migration process translated into a `sh(addr(mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW))#0ezg4j29` descriptor. Which isn't a valid descriptor because the `addr()` func can only appear at the top level.
The assertion cause is still unknown but hey, one bug less at least :).
(https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1629158947)
Ok, found the `Unrecognized descriptor found` error cause. You might have imported a hex script and the migration process translated into a `sh(addr(mqRzyzrW7DPGT1KZN2mLuyGPVUugpfEVPW))#0ezg4j29` descriptor. Which isn't a valid descriptor because the `addr()` func can only appear at the top level.
The assertion cause is still unknown but hey, one bug less at least :).
🚀 fanquake merged a pull request: "test: make assumeUTXO test capture the expected fatal error"
(https://github.com/bitcoin/bitcoin/pull/28050)
(https://github.com/bitcoin/bitcoin/pull/28050)
💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629211279)
Hey @MarcoFalke should I revert the last commit? All checks except the lint test are passing.
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629211279)
Hey @MarcoFalke should I revert the last commit? All checks except the lint test are passing.
👍 TheCharlatan approved a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1522386920)
re-ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550
(https://github.com/bitcoin/bitcoin/pull/27607#pullrequestreview-1522386920)
re-ACK ca91c244ef1ba7eac6643d66a5fc56d3a2a8b550
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629226202)
Don't revert the commit; just remove the stray file and squash.
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629226202)
Don't revert the commit; just remove the stray file and squash.
💬 vasild commented on issue "p2p sends local-scope addresses when IPv4 is behind NAT":
(https://github.com/bitcoin/bitcoin/issues/8616#issuecomment-1629232150)
To summarize, there are 3 issues here:
1. The problem from OP of sending local addresses to peers was nonexistent. That was never done.
2. Sending our addr in the `VERSION` message, fixed by https://github.com/bitcoin/bitcoin/pull/8740
3. Sending our addr via `ADDR` (or `ADDRv2`) message. I need to look at this more carefully, but a brief look tells me that maybe the problem exists. I mean sending our IPv4 address to a Tor peer or sending our Tor address to an IPv4 peer. At least `CNetAddr:
...
(https://github.com/bitcoin/bitcoin/issues/8616#issuecomment-1629232150)
To summarize, there are 3 issues here:
1. The problem from OP of sending local addresses to peers was nonexistent. That was never done.
2. Sending our addr in the `VERSION` message, fixed by https://github.com/bitcoin/bitcoin/pull/8740
3. Sending our addr via `ADDR` (or `ADDRv2`) message. I need to look at this more carefully, but a brief look tells me that maybe the problem exists. I mean sending our IPv4 address to a Tor peer or sending our Tor address to an IPv4 peer. At least `CNetAddr:
...
🚀 ryanofsky merged a pull request: "index: make startup more efficient"
(https://github.com/bitcoin/bitcoin/pull/27607)
(https://github.com/bitcoin/bitcoin/pull/27607)
💬 pinheadmz commented on pull request "wallet: Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1258472175)
> I'm not sure if or when legacy wallet support is expected to be phased out, but if isactive is only different from ismine in legacy wallets, is it worth adding a field (and all the new methods), versus just updating the ismine field documentation in the help for descriptor wallets? (When I'm double-checking an address with getaddressinfo before using it to receive, ismine is a field I'm already checking.)
@jonatack after diving back in to this issue I realized that `ismine != isactive` for
...
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1258472175)
> I'm not sure if or when legacy wallet support is expected to be phased out, but if isactive is only different from ismine in legacy wallets, is it worth adding a field (and all the new methods), versus just updating the ismine field documentation in the help for descriptor wallets? (When I'm double-checking an address with getaddressinfo before using it to receive, ismine is a field I'm already checking.)
@jonatack after diving back in to this issue I realized that `ismine != isactive` for
...
⚠️ ishaanam opened an issue: "Creating a Wallet Feature Guidelines Document"
(https://github.com/bitcoin/bitcoin/issues/28062)
It would be useful to the project if there was a document detailing what features are considered "out of scope" of the Bitcoin Core Wallet. This would make evaluating wallet PRs that add features easier for reviewers. It would also be useful to contributors so that they can get a better sense of which features are more likely to get merged. I'm curious if anybody else thinks that such a document would be useful, and if there are any guidelines that should be added. These guidelines would probabl
...
(https://github.com/bitcoin/bitcoin/issues/28062)
It would be useful to the project if there was a document detailing what features are considered "out of scope" of the Bitcoin Core Wallet. This would make evaluating wallet PRs that add features easier for reviewers. It would also be useful to contributors so that they can get a better sense of which features are more likely to get merged. I'm curious if anybody else thinks that such a document would be useful, and if there are any guidelines that should be added. These guidelines would probabl
...
💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629250320)
> Don't revert the commit; just remove the stray file and squash.
I think it should be fine now, can you please check once again?
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629250320)
> Don't revert the commit; just remove the stray file and squash.
I think it should be fine now, can you please check once again?
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629252705)
You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629252705)
You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629255377)
> You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
I think I did squash them, atleast that's what it shows in the commits tab.
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629255377)
> You need to squash the commits. See https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
I think I did squash them, atleast that's what it shows in the commits tab.
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629257234)
Apologies, you did indeed!
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629257234)
Apologies, you did indeed!
💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629260496)
> Apologies, you did indeed!
No worries, thanks for the guidance throughout this PR!
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629260496)
> Apologies, you did indeed!
No worries, thanks for the guidance throughout this PR!
👍 theStack approved a pull request: "Add support for RFC8439 variant of ChaCha20"
(https://github.com/bitcoin/bitcoin/pull/27985#pullrequestreview-1522431786)
Code-review ACK 0bf87476f55dceb106563156c7c8d6bfb8162e29
(https://github.com/bitcoin/bitcoin/pull/27985#pullrequestreview-1522431786)
Code-review ACK 0bf87476f55dceb106563156c7c8d6bfb8162e29
💬 ItIsOHM commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629302650)
> Apologies, you did indeed! The commit title is "Deleted stray file `vcpkg`" now, though.
Should I amend this commit or leave it as it is?
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629302650)
> Apologies, you did indeed! The commit title is "Deleted stray file `vcpkg`" now, though.
Should I amend this commit or leave it as it is?
👍 theStack approved a pull request: "test: Check expected_stderr after stop"
(https://github.com/bitcoin/bitcoin/pull/28028#pullrequestreview-1522532024)
ACK faf902858d38150caa8991b0ab9d7cfee2905684
Makes sense to do the stderr check at the very end after shutdown.
(https://github.com/bitcoin/bitcoin/pull/28028#pullrequestreview-1522532024)
ACK faf902858d38150caa8991b0ab9d7cfee2905684
Makes sense to do the stderr check at the very end after shutdown.
💬 sipa commented on pull request "[WIP] rpc: doc: Added `longpollid` and `data` params to `template_request` #27998":
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629352299)
Yes, amend the commit title to be accurate. You can rewrite the entire PR at any point.
(https://github.com/bitcoin/bitcoin/pull/28056#issuecomment-1629352299)
Yes, amend the commit title to be accurate. You can rewrite the entire PR at any point.