💬 glozow commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153118703)
Question: what is the reason to put `m_recently_announced_invs` in `Peer` as opposed to `TxRelay` within `Peer`?
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153118703)
Question: what is the reason to put `m_recently_announced_invs` in `Peer` as opposed to `TxRelay` within `Peer`?
⚠️ vasild opened an issue: "Fix net grouping of non-IP networks"
(https://github.com/bitcoin/bitcoin/issues/27369)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`NetGroupManager::GetGroup()` was designed with IP networks in mind, where for example `10.20.30.40` and `10.20.30.45` are "close" and likely to belong to the same entity, while `100.20.30.40` is "distant" from them. This does not apply however for Tor, I2P and CJDNS where the address is "random" bytes. Actually, Tor, I2P and CJDNS addresses that have common prefix are _harder_ to obtain t
...
(https://github.com/bitcoin/bitcoin/issues/27369)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
`NetGroupManager::GetGroup()` was designed with IP networks in mind, where for example `10.20.30.40` and `10.20.30.45` are "close" and likely to belong to the same entity, while `100.20.30.40` is "distant" from them. This does not apply however for Tor, I2P and CJDNS where the address is "random" bytes. Actually, Tor, I2P and CJDNS addresses that have common prefix are _harder_ to obtain t
...
💬 vasild commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1490154556)
How? I opened https://github.com/bitcoin/bitcoin/issues/27369 to discuss ideas on how to approach this.
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1490154556)
How? I opened https://github.com/bitcoin/bitcoin/issues/27369 to discuss ideas on how to approach this.
💬 glozow commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1153139259)
https://cirrus-ci.com/task/4526522288046080
seems like the tidy job wants you to add this:
```suggestion
std::vector<uint256> txids_needed;
txids_needed.reserve(m_requested_outpoints_by_txid.size());
```
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1153139259)
https://cirrus-ci.com/task/4526522288046080
seems like the tidy job wants you to add this:
```suggestion
std::vector<uint256> txids_needed;
txids_needed.reserve(m_requested_outpoints_by_txid.size());
```
💬 stratospher commented on issue "Fix net grouping of non-IP networks":
(https://github.com/bitcoin/bitcoin/issues/27369#issuecomment-1490173773)
https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1490154556 - workaround it by not inserting tor/i2p/cjdns address in `setConnected` for now? so `GetGroup` wouldn't be called on these networks.
(https://github.com/bitcoin/bitcoin/issues/27369#issuecomment-1490173773)
https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1490154556 - workaround it by not inserting tor/i2p/cjdns address in `setConnected` for now? so `GetGroup` wouldn't be called on these networks.
💬 Sjors commented on pull request "refactor: remove unused param from legacy pubkey interface":
(https://github.com/bitcoin/bitcoin/pull/27274#discussion_r1153151060)
Oh you're right, I missed the last push.
In the majority of cases we don't use a space after `=*/`, but that's not worth touching the PR for imo.
(https://github.com/bitcoin/bitcoin/pull/27274#discussion_r1153151060)
Oh you're right, I missed the last push.
In the majority of cases we don't use a space after `=*/`, but that's not worth touching the PR for imo.
💬 Sjors commented on pull request "refactor: Drop no longer used `CNetMsgMaker` instances":
(https://github.com/bitcoin/bitcoin/pull/27368#issuecomment-1490183368)
ACK ea7ec7808745805c0a18513d7da271dedb2de3f1
(https://github.com/bitcoin/bitcoin/pull/27368#issuecomment-1490183368)
ACK ea7ec7808745805c0a18513d7da271dedb2de3f1
👍 Sjors approved a pull request: "refactor: remove unused param from legacy pubkey interface"
(https://github.com/bitcoin/bitcoin/pull/27274)
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
(https://github.com/bitcoin/bitcoin/pull/27274)
ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e
⚠️ osas90 opened an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)
I got a private key in bitcoin on my blockchain.com for one of my receiving addresses. When I went to import it, it imported another address but i already used it as receiving addresses now funds now available...
_Originally posted by @MarcoFalke in https://github.com/bitcoin/bitcoin/issues/17846#issuecomment-570241898_
(https://github.com/bitcoin/bitcoin/issues/27370)
I got a private key in bitcoin on my blockchain.com for one of my receiving addresses. When I went to import it, it imported another address but i already used it as receiving addresses now funds now available...
_Originally posted by @MarcoFalke in https://github.com/bitcoin/bitcoin/issues/17846#issuecomment-570241898_
✅ fanquake closed an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)
(https://github.com/bitcoin/bitcoin/issues/27370)
:lock: fanquake locked an issue: "Import in Bitcoin Core or a different software? What are the steps to reproduce?"
(https://github.com/bitcoin/bitcoin/issues/27370)
(https://github.com/bitcoin/bitcoin/issues/27370)
💬 ismaelsadeeq commented on pull request "test: refactor: dedup mempool_package_limits.py subtests via decorator":
(https://github.com/bitcoin/bitcoin/pull/27350#issuecomment-1490199581)
ACK e669833943bda13b2840a174dc8e45194187fc8e
(https://github.com/bitcoin/bitcoin/pull/27350#issuecomment-1490199581)
ACK e669833943bda13b2840a174dc8e45194187fc8e
💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490210669)
This is definitely nicer. When you doubleclick on the zip file it magically reveals the orange icon:
<img width="612" alt="Schermafbeelding 2023-03-30 om 14 15 54" src="https://user-images.githubusercontent.com/10217/228833150-647e3460-c174-43d0-b026-c9dbffb10b64.png">
<img width="845" alt="Schermafbeelding 2023-03-30 om 14 16 10" src="https://user-images.githubusercontent.com/10217/228833181-c82d0639-91a3-476f-a309-e9732577a657.png">
The only thing we need to remind the user of, is t
...
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490210669)
This is definitely nicer. When you doubleclick on the zip file it magically reveals the orange icon:
<img width="612" alt="Schermafbeelding 2023-03-30 om 14 15 54" src="https://user-images.githubusercontent.com/10217/228833150-647e3460-c174-43d0-b026-c9dbffb10b64.png">
<img width="845" alt="Schermafbeelding 2023-03-30 om 14 16 10" src="https://user-images.githubusercontent.com/10217/228833181-c82d0639-91a3-476f-a309-e9732577a657.png">
The only thing we need to remind the user of, is t
...
💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490218629)
I'd like to test the Guix build too. @achow101 can you sign it? Or is there a way to self-sign it?
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1490218629)
I'd like to test the Guix build too. @achow101 can you sign it? Or is there a way to self-sign it?
💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#discussion_r1153198925)
💿
(https://github.com/bitcoin/bitcoin/pull/27099#discussion_r1153198925)
💿
💬 dergoegge commented on pull request "refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer":
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153208873)
No reason, it should be in `TxRelay`, which also results in a small resource optimization because that filter now only exists for tx relay peers. Thanks!
(https://github.com/bitcoin/bitcoin/pull/26140#discussion_r1153208873)
No reason, it should be in `TxRelay`, which also results in a small resource optimization because that filter now only exists for tx relay peers. Thanks!
🚀 fanquake merged a pull request: "refactor: Drop no longer used `CNetMsgMaker` instances"
(https://github.com/bitcoin/bitcoin/pull/27368)
(https://github.com/bitcoin/bitcoin/pull/27368)
👍 stickies-v approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
re-ACK 30efd6d36
I think my comments aren't too controversial to address but they only affect documentation and style/readability so I'm happy with the status quo and a follow-up, too. We've had quite a bit of back and forth already on this PR.
(https://github.com/bitcoin/bitcoin/pull/26261)
re-ACK 30efd6d36
I think my comments aren't too controversial to address but they only affect documentation and style/readability so I'm happy with the status quo and a follow-up, too. We've had quite a bit of back and forth already on this PR.
💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140315012)
There's another such update needed for the `LookupNumeric` docstring which currently reads:
```
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
* for additional parameter descriptions.
```
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140315012)
There's another such update needed for the `LookupNumeric` docstring which currently reads:
```
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
* for additional parameter descriptions.
```
💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153145784)
```suggestion
const CService ui_proxy{ui_proxy_netaddr.value_or(CNetAddr{}), ui->proxyPort->text().toUShort()};
```
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1153145784)
```suggestion
const CService ui_proxy{ui_proxy_netaddr.value_or(CNetAddr{}), ui->proxyPort->text().toUShort()};
```