💬 Sjors commented on issue "ASN-based bucketing of the network nodes":
(https://github.com/bitcoin/bitcoin/issues/16599#issuecomment-1496367967)
During Advancing Bitcoin 2023 @virtu showed some simulations for the number of inbound connections nodes on "obscure" ASN's would get. Perhaps there's a way to reduce the bandwidth burden for these nodes.

It's good in general to connect to a diverse set of ASN's so we don't get eclipsed. But for downloading blocks it seems unnecessary, since we know exactly what we're looking for. Perhaps during IBD (and long catchup) we could make additional outbound connections to a fast ASN. Hardcoding Ama
...

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157609917)
hmm why?
`warning: 'const' type qualifier on return type has no effect`

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157613803)
pushed

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1157613941)
pushed

💬 kouloumos commented on issue "ASN-based bucketing of the network nodes":
(https://github.com/bitcoin/bitcoin/issues/16599#issuecomment-1496408484)
> During Advancing Bitcoin 2023 @virtu showed some simulations for the number of inbound connections nodes on "obscure" ASN's would get.

For more context: https://github.com/virtu/talks/tree/master/2023-03-02-advancing-bitcoin

💬 martinus commented on pull request "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings":
(https://github.com/bitcoin/bitcoin/pull/27401#issuecomment-1496410442)
> Maybe add a check to configure.ac whether ignoring of -Wgnu-zero-variadic-macro-arguments is https://github.com/bitcoin/bitcoin/issues/26916#issuecomment-1400156218 required?

Sounds good, but I have no idea how to do this with `configure.ac`... But it's also possible to do this in code level, I've now added a `__cplusplus < 202002L` so disabling the warning only happens when compiling below c++20

💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1496413288)
Thanks all for the feedback :). [Update Diff](https://github.com/bitcoin/bitcoin/compare/ba7970c793f3e8ddb54a02256e7ae52e88e47f08..488dbeab56b2cd62e381883e5fe347b446ae7d2c):

* Renamed `max_weight` to `max_inputs_weight`.
* Deduced change size from the max allowed weight for Knapsack and SRD (not from BnB as it's specialized to not create change).
* Adapted doxygen comment to current sources.

💬 RandyMcMillan commented on pull request "build: remove ancient unused define":
(https://github.com/bitcoin/bitcoin/pull/27420#issuecomment-1496440854)
utACK 9fbc5fc

💬 RandyMcMillan commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1496446116)
Concept ACK

💬 hebasto commented on pull request "ci: Remove second user account":
(https://github.com/bitcoin/bitcoin/pull/27376#issuecomment-1496449588)
Unfortunately, I've noticed a regression introduced in this PR.

On Ubuntu 22.04, running `FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh` locally fails with
```
...
IWYU edited 30 files on your behalf.

warning: Not a git repository. Use --no-index to compare two paths outside a working tree
usage: git diff --no-index [<options>] <path> <path>
...
```

👍 hebasto approved a pull request: "compat: move (win) S_* defines into bdb"
(https://github.com/bitcoin/bitcoin/pull/26832)
ACK 54e406118926c2a526455a60f67961520dfb65da, I have reviewed the code and it looks OK.

nit: Two commits _"refactor: don't avoid sys/types.h on when building for Windows"_ and _"compat: move (win) S\_* defines into bdb"_ were squashed but the commit message of the latter was lost.

👍 Ayush170-Future approved a pull request: "build: remove ancient unused define"
(https://github.com/bitcoin/bitcoin/pull/27420)
ACK

💬 MarcoFalke commented on pull request "More verbose warning for multiple network argument error.":
(https://github.com/bitcoin/bitcoin/pull/26028#issuecomment-1496533357)
Closing for now due to lack of activity. Let us know when the should be reopened.

✅ MarcoFalke closed a pull request: "More verbose warning for multiple network argument error."
(https://github.com/bitcoin/bitcoin/pull/26028)

💬 brunoerg commented on pull request "logging, net: add ASN from peers on logs":
(https://github.com/bitcoin/bitcoin/pull/27412#issuecomment-1496568490)
> Would be nice to log this for inbound connections too, I guess the receive version message: is most suitable for that?

Yes, it would be great to have this for both ones, just updated it!

💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1157735880)
No I cannot, because it doesn't make sense. Will revert.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157712788)
Missing brackets here (we require brackets with conditionals over one line due to CVE-2014-1266, the Apple "goto fail" vulnerability, see also https://dwheeler.com/essays/apple-goto-fail).

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157714528)
nit, I prefer to read left-to-right rather than up-to-down, when these can all be on one line

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157715369)
Could set only the first enum value for smaller diffs/less churn in future changes.

💬 jonatack commented on pull request "Add warnings for non-active addresses in receive tab and address book":
(https://github.com/bitcoin-core/gui/pull/723#discussion_r1157718871)
(Idem brackets here and lines 118-119)