π¬ stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134245231)
done.
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134245231)
done.
π¬ stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134253991)
adding addresses from different networks wouldn't cause these kind of collisions because [`GetGroup()`](https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/netgroup.cpp#L17) would differ right?
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134253991)
adding addresses from different networks wouldn't cause these kind of collisions because [`GetGroup()`](https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/netgroup.cpp#L17) would differ right?
π¬ MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1466466171)
lgtm. While it would be good to be able to point to docs, I don't think this is required. Happy to review if you move this out of draft.
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1466466171)
lgtm. While it would be good to be able to point to docs, I don't think this is required. Happy to review if you move this out of draft.
π¬ TheCharlatan commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466482704)
Updated [4aa0ec6](https://github.com/bitcoin/bitcoin/pull/27238/commits/4aa0ec6a8617ac77a19e49d8cf081b7a7f6404ae) -> aaced5633b81b2f08b993106a527e2a0e1d663c1 ([splitSystemLogging_0](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_0) -> [splitSystemLogging_1](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemLogging_0..splitSystemLogging_1)) to fix header inclusion order addressing @MarcoFalke
...
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466482704)
Updated [4aa0ec6](https://github.com/bitcoin/bitcoin/pull/27238/commits/4aa0ec6a8617ac77a19e49d8cf081b7a7f6404ae) -> aaced5633b81b2f08b993106a527e2a0e1d663c1 ([splitSystemLogging_0](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_0) -> [splitSystemLogging_1](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemLogging_0..splitSystemLogging_1)) to fix header inclusion order addressing @MarcoFalke
...
π¬ instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1466487189)
Switched to OP_TRUE, as it was less confusing for at least a few people, and only requires light test editing. Had to rebase on top of master for trivial merge conflict.
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1466487189)
Switched to OP_TRUE, as it was less confusing for at least a few people, and only requires light test editing. Had to rebase on top of master for trivial merge conflict.
π¬ MarcoFalke commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466487829)
re-ACK aaced5633b81b2f08b993106a527e2a0e1d663c1 π
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK aaced5633b81b2f08b99
...
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466487829)
re-ACK aaced5633b81b2f08b993106a527e2a0e1d663c1 π
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK aaced5633b81b2f08b99
...
π¬ instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#discussion_r1134315587)
suggested in other PR: "switch the ordering? else if (whichType != ANCHOR && IsDust())"
(https://github.com/bitcoin/bitcoin/pull/26403#discussion_r1134315587)
suggested in other PR: "switch the ordering? else if (whichType != ANCHOR && IsDust())"
β
glozow closed an issue: "Error messages for invalid address"
(https://github.com/bitcoin/bitcoin/issues/21741)
(https://github.com/bitcoin/bitcoin/issues/21741)
π glozow merged a pull request: "Improve address decoding errors"
(https://github.com/bitcoin/bitcoin/pull/26514)
(https://github.com/bitcoin/bitcoin/pull/26514)
π hernanmarino approved a pull request: "Mask values on Transactions View"
(https://github.com/bitcoin-core/gui/pull/708)
ACK 4492de1be11f4131811f504a037342c78f480e20
(https://github.com/bitcoin-core/gui/pull/708)
ACK 4492de1be11f4131811f504a037342c78f480e20
π glozow merged a pull request: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)
(https://github.com/bitcoin/bitcoin/pull/27235)
π¬ achow101 commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1466603230)
ACK 4492de1be11f4131811f504a037342c78f480e20
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1466603230)
ACK 4492de1be11f4131811f504a037342c78f480e20
π¬ mzumsande commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134455743)
I don't think so - even with the guarantee that `GetGroup()` is different, there is still a small chance that two addresses happen to hash to the same bucket in `GetNewBucket()`.
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134455743)
I don't think so - even with the guarantee that `GetGroup()` is different, there is still a small chance that two addresses happen to hash to the same bucket in `GetNewBucket()`.
π pinheadmz's pull request is ready for review: "Add wallet method to detect if a key is "active""
(https://github.com/bitcoin/bitcoin/pull/27216)
(https://github.com/bitcoin/bitcoin/pull/27216)
π¬ ishaanam commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1134513177)
Oh yes, `walletlock` never gets called here. However, implementing this change results in another `CannotSendRequest`.
(https://github.com/bitcoin/bitcoin/pull/27199#discussion_r1134513177)
Oh yes, `walletlock` never gets called here. However, implementing this change results in another `CannotSendRequest`.
π¬ hernanmarino commented on issue "Confusing/misleading "Dust:" label in coin selection dialog":
(https://github.com/bitcoin-core/gui/issues/699#issuecomment-1466831109)
+1 on removing it
(https://github.com/bitcoin-core/gui/issues/699#issuecomment-1466831109)
+1 on removing it
π¬ mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1466870416)
> Hmm, this is maybe ok. Another approach would be to try to open to e.g. Tor while still having < 8 outbound connections. To counter the issue of staying with < 8 connections and not being able to open to Tor because e.g. the Tor proxy is broken, then we could give up after a few attempts and then fall back to the old way of filling with random (aka IPv4 ;-)) addresses. If we overshoot the 8 connections (like described above), then for some time we will have more, until eviction kicks in. How m
...
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1466870416)
> Hmm, this is maybe ok. Another approach would be to try to open to e.g. Tor while still having < 8 outbound connections. To counter the issue of staying with < 8 connections and not being able to open to Tor because e.g. the Tor proxy is broken, then we could give up after a few attempts and then fall back to the old way of filling with random (aka IPv4 ;-)) addresses. If we overshoot the 8 connections (like described above), then for some time we will have more, until eviction kicks in. How m
...
π¬ mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1466872077)
> > Why it doesn't make sense? I think it is an extra effort to obtain IPv4 addresses if one has IPv6 addresses and the other way around. Why impossible in some network environments? After all you know if you are going to attempt to connect to 5.6.7.8 or 2030:c0fe:....
>
> I also not sure if I agree with counting IPv4 and IPv6 as one network and I think this could need a bit more reasoning. Is it because they are both clearnet? Is it a workaround to don't have 8 protected peers if all network
...
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1466872077)
> > Why it doesn't make sense? I think it is an extra effort to obtain IPv4 addresses if one has IPv6 addresses and the other way around. Why impossible in some network environments? After all you know if you are going to attempt to connect to 5.6.7.8 or 2030:c0fe:....
>
> I also not sure if I agree with counting IPv4 and IPv6 as one network and I think this could need a bit more reasoning. Is it because they are both clearnet? Is it a workaround to don't have 8 protected peers if all network
...
π¬ Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1134606456)
Thanks, thatβs unexpected. I would also expect that `CalculateTotalBumpFees(β¦)` always is equal or smaller to the sum of individual bump fees. Iβll check it out.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1134606456)
Thanks, thatβs unexpected. I would also expect that `CalculateTotalBumpFees(β¦)` always is equal or smaller to the sum of individual bump fees. Iβll check it out.
π¬ willcl-ark commented on issue "untrust vs unsafe ?":
(https://github.com/bitcoin/bitcoin/issues/18625#issuecomment-1466951596)
I think a breaking change to the RPC argument keys, seeing as we accept named arguments via rpc, would likely not be considered worth it just to harmonise vocabulary.
@achow101 I'd be curious of your thoughts here? If we aren't going to change this then perhaps we can close the issue.
(https://github.com/bitcoin/bitcoin/issues/18625#issuecomment-1466951596)
I think a breaking change to the RPC argument keys, seeing as we accept named arguments via rpc, would likely not be considered worth it just to harmonise vocabulary.
@achow101 I'd be curious of your thoughts here? If we aren't going to change this then perhaps we can close the issue.