Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1458015010)
Concept ACK.

This was somewhere on my TODO, thanks for picking it up!

> The automatic connections to alternate networks is done defensively, by first filling all outbound slots with random addresses (as in the status quo) and then adding additional peers from reachable networks the node is currently not connected to. This approach ensures that outbound slots are not left unfilled while attempting to connect to a network that may be unavailable due to a technical issue or misconfiguration t
...
💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1127730689)
```suggestion
const auto& address = addrman.Select(/*new_only=*/false, NET_I2P);
```
💬 brunoerg commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1127731998)
Yes, can leave this way. This suggestion I made while reviewing the first commit but considering the whole context, you can leave it.
💬 MarcoFalke commented on pull request "ci: Use clang-15 and IWYU v0.19 in "tidy" task":
(https://github.com/bitcoin/bitcoin/pull/26766#discussion_r1127740280)
Another alternative would be to use https://apt.llvm.org/
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1458076661)
Added myself as co-author to some commits per invitation of @glozow
👍 hebasto approved a pull request: "util: Work around ParseHex gcc cross compiler bug"
(https://github.com/bitcoin/bitcoin/pull/27218)
ACK fa8481b05fae6a7e44ca526d930993f02549234e, tested on Ubuntu 22.04, gcc 11.3 for the `riscv64-linux-gnu` host.,
💬 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-1458118175)
rebased, conflicts solved.
👍 gruve-p approved a pull request: "util: Work around ParseHex gcc cross compiler bug"
(https://github.com/bitcoin/bitcoin/pull/27218)
💬 gruve-p commented on pull request "util: Work around ParseHex gcc cross compiler bug":
(https://github.com/bitcoin/bitcoin/pull/27218#issuecomment-1458151432)
ACK https://github.com/bitcoin/bitcoin/pull/27218/commits/fa8481b05fae6a7e44ca526d930993f02549234e

Tested cross compiling through depends on Ubuntu Jammy (22.04) with GCC 11 on these hosts:
`i686-pc-linux-gnu`
`x86_64-pc-linux-gnu`
`x86_64-w64-mingw32`
`x86_64-apple-darwin`
`arm64-apple-darwin`
`arm-linux-gnueabihf`
`aarch64-linux-gnu`
`powerpc64-linux-gnu`
`powerpc64le-linux-gnu`
`riscv32-linux-gnu`
`riscv64-linux-gnu`
`s390x-linux-gnu`
💬 fanquake commented on pull request "util: Work around ParseHex gcc cross compiler bug":
(https://github.com/bitcoin/bitcoin/pull/27218#issuecomment-1458200190)
Does anyone have a link to an upstream issue? Is this a regression in GCC 11, that has been fixed in 12/13?
💬 MarcoFalke commented on pull request "util: Work around ParseHex gcc cross compiler bug":
(https://github.com/bitcoin/bitcoin/pull/27218#issuecomment-1458210175)
It is not fixed in gcc-12, see https://github.com/bitcoin/bitcoin/pull/25227#issuecomment-1456215491. gcc-13 exists, so it may be possible to test. I have not attempted to reduce the test case, nor did I dig deeper to find out if this is a compiler bug or bug in our code, because I don't think we have a choice here: If there are two equivalent versions of the same code, but one doesn't compile, we should pick the one that compiles.
💬 MarcoFalke commented on pull request "util: Work around ParseHex gcc cross compiler bug":
(https://github.com/bitcoin/bitcoin/pull/27218#issuecomment-1458213858)
To clarify, even if the workaround is not needed in gcc-13 or gcc-14, I think we shouldn't revert it. I see it as a permanent workaround.
💬 fanquake commented on pull request "util: Work around ParseHex gcc cross compiler bug":
(https://github.com/bitcoin/bitcoin/pull/27218#issuecomment-1458234945)
> gcc-13 exists, so it may be possible to test.

Still broken in GCC 13.0.1. `riscv64-linux-gnu-g++-13 13.0.1 20230215`
🚀 fanquake merged a pull request: "util: Work around ParseHex gcc cross compiler bug"
(https://github.com/bitcoin/bitcoin/pull/27218)
💬 fanquake commented on pull request "Add wallet method to detect if a key is "fresh"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1127978416)
https://cirrus-ci.com/task/6502838134636544:
```bash
wallet/test/scriptpubkeyman_tests.cpp:142:48: error: argument name 'index' in comment does not match parameter name 'pos' [bugprone-argument-comment,-warnings-as-errors]
descriptor.descriptor->ExpandFromCache(/*index=*/10, descriptor.cache, scripts3, provider);
^
./script/descriptor.h:137:38: note: 'pos' declared here
virtual bool ExpandFromCache(int pos, const DescriptorCache& r
...
💬 fanquake commented on pull request "Add wallet method to detect if a key is "fresh"":
(https://github.com/bitcoin/bitcoin/pull/27216#issuecomment-1458300719)
https://github.com/bitcoin/bitcoin/pull/27216/checks?check_run_id=11804958852
```bash
Assertion failed: lock cs_wallet not held in ./wallet/wallet.h:444; locks held:
'cs_KeyStore' in wallet/scriptpubkeyman.cpp:1185 (in thread 'test')
```
💬 fanquake commented on pull request "wallet: 26032 followups":
(https://github.com/bitcoin/bitcoin/pull/27180#issuecomment-1458303075)
> Followups for https://github.com/bitcoin/bitcoin/pull/26032. So far nothing major.

Are you expecting there to be more? I really only see the one followup comment in #26032.
📝 pablomartin4btc opened a pull request: "doc: update broken links on developer-notes"
(https://github.com/bitcoin/bitcoin/pull/27220)
References to `utilstrencodings` and `lint-locale-dependence.sh` where incorrect, updating them accordingly.

Also, adding another reference to util function [`LocaleIndependentAtoi`](https://github.com/bitcoin/bitcoin/blob/master/src/util/strencodings.h#L108-L118), which is related with the updated section of the guide.
💬 pinheadmz commented on pull request "Add wallet method to detect if a key is "fresh"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1128006272)
oooh thanks, didn't realize those comments were parsed by anything!
👍 willcl-ark approved a pull request: "util: Use steady clock in SeedStrengthen, FindBestImplementation, FlushStateToDisk"
(https://github.com/bitcoin/bitcoin/pull/27189)
ACK fa1b4e5c3