💬 jonatack commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513962484)
> Perhaps it's time to encourage people to build their own asmap database instead of just dowloading it?
That might be tangential to this pull but feel free to propose a concrete diff or a follow-up.
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1513962484)
> Perhaps it's time to encourage people to build their own asmap database instead of just dowloading it?
That might be tangential to this pull but feel free to propose a concrete diff or a follow-up.
💬 amitiuttarwar commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1514035359)
added an extra commit to ensure the non-deterministic test will not fail intermittently. will incorporate the outstanding feedback separately
(https://github.com/bitcoin/bitcoin/pull/27214#issuecomment-1514035359)
added an extra commit to ensure the non-deterministic test will not fail intermittently. will incorporate the outstanding feedback separately
💬 vasild commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170892009)
I think that's a bit too verbose.
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170892009)
I think that's a bit too verbose.
💬 vasild commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170894501)
Named args at call sites look too verbose to me and here a short `name` is ok because the scope if very limited - only used on one line.
(https://github.com/bitcoin/bitcoin/pull/25390#discussion_r1170894501)
Named args at call sites look too verbose to me and here a short `name` is ok because the scope if very limited - only used on one line.
💬 gruve-p commented on pull request "kernel: chainparams updates for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1514238275)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
(https://github.com/bitcoin/bitcoin/pull/27482#issuecomment-1514238275)
ACK https://github.com/bitcoin/bitcoin/pull/27482/commits/a2bef805c11a4ab391f4ea635bfde7dd2ec9ce81
💬 gruve-p commented on pull request "[24.1] Bump version to 24.1rc2":
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1514240526)
ACK https://github.com/bitcoin/bitcoin/pull/27486/commits/03a16a1da09f45721c799c719e48350dfe12ae88
(https://github.com/bitcoin/bitcoin/pull/27486#issuecomment-1514240526)
ACK https://github.com/bitcoin/bitcoin/pull/27486/commits/03a16a1da09f45721c799c719e48350dfe12ae88
💬 MarcoFalke commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514259259)
You should be able to see it in the tsan CI task or with libc++ locally?
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514259259)
You should be able to see it in the tsan CI task or with libc++ locally?
💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170929813)
Yeah, I think it is good to know of false positives and then report them upstream. Otherwise they are less likely to be fixed.
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170929813)
Yeah, I think it is good to know of false positives and then report them upstream. Otherwise they are less likely to be fixed.
🚀 fanquake merged a pull request: "[24.1] Bump version to 24.1rc2"
(https://github.com/bitcoin/bitcoin/pull/27486)
(https://github.com/bitcoin/bitcoin/pull/27486)
👍 MarcoFalke approved a pull request: "refactor: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1391499392)
review ACK ec51c252de42e11906f114ac05e476fd88ca2176 🦅
<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: review ACK ec51c252de4
...
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1391499392)
review ACK ec51c252de42e11906f114ac05e476fd88ca2176 🦅
<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: review ACK ec51c252de4
...
💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170972002)
Any reason for a separate section for this header?
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170972002)
Any reason for a separate section for this header?
💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170983576)
Can you explain this change?
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170983576)
Can you explain this change?
💬 MarcoFalke commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170994610)
nit: The comment is incomplete (missing AnyPtr stuff etc), and I don't think it is a good use of time to make it complete. Also, the file has less than 10 functions and we don't put those comments in other files, so might as well remove it.
(https://github.com/bitcoin/bitcoin/pull/27419#discussion_r1170994610)
nit: The comment is incomplete (missing AnyPtr stuff etc), and I don't think it is a good use of time to make it complete. Also, the file has less than 10 functions and we don't put those comments in other files, so might as well remove it.
💬 MarcoFalke commented on pull request "depends: Remove `_LIBCPP_DEBUG` from depends DEBUG mode":
(https://github.com/bitcoin/bitcoin/pull/27447#issuecomment-1514364903)
This should also fix a segfault with `DEBUG=1`. To reproduce on a fresh install of Ubuntu Jammy:
* `export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl g++-multilib libtool binutils bsdmainutils pkg-config python3 patch biso
...
(https://github.com/bitcoin/bitcoin/pull/27447#issuecomment-1514364903)
This should also fix a segfault with `DEBUG=1`. To reproduce on a fresh install of Ubuntu Jammy:
* `export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl g++-multilib libtool binutils bsdmainutils pkg-config python3 patch biso
...
💬 fanquake commented on issue "Replace all of the RecursiveMutex instances with the Mutex ones":
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514372323)
> You should be able to see it in the tsan CI task or with libc++ locally?
I don't currently reproduce on x86_64 with master +:
```diff
--- a/test/sanitizer_suppressions/tsan
+++ b/test/sanitizer_suppressions/tsan
@@ -12,9 +12,6 @@ race:DatabaseBatch
race:zmq::*
race:bitcoin-qt
-# deadlock (TODO fix)
-deadlock:Chainstate::ConnectTip
-
# Intentional deadlock in tests
deadlock:sync_tests::potential_deadlock_detected
```
and
```bash
time FILE_ENV="./ci/test/00_setup_env
...
(https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514372323)
> You should be able to see it in the tsan CI task or with libc++ locally?
I don't currently reproduce on x86_64 with master +:
```diff
--- a/test/sanitizer_suppressions/tsan
+++ b/test/sanitizer_suppressions/tsan
@@ -12,9 +12,6 @@ race:DatabaseBatch
race:zmq::*
race:bitcoin-qt
-# deadlock (TODO fix)
-deadlock:Chainstate::ConnectTip
-
# Intentional deadlock in tests
deadlock:sync_tests::potential_deadlock_detected
```
and
```bash
time FILE_ENV="./ci/test/00_setup_env
...
💬 willcl-ark commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1171059750)
Thanks, I did write it like this intentially as I felt that the final 3 are all variants of network/CIDR, but writing them as a sub-list didn't read well to me...
Thinking more about it I think they should be called wildcard addresses, so perhaps we go with:
```cpp
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), or wildc
...
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1171059750)
Thanks, I did write it like this intentially as I felt that the final 3 are all variants of network/CIDR, but writing them as a sub-list didn't read well to me...
Thinking more about it I think they should be called wildcard addresses, so perhaps we go with:
```cpp
argsman.AddArg("-rpcallowip=<ip>", "Allow JSON-RPC connections from specified source. Valid for <ip> are a single IP (e.g. 1.2.3.4), a network/netmask (e.g. 1.2.3.4/255.255.255.0), a network/CIDR (e.g. 1.2.3.4/24), or wildc
...
💬 Sjors commented on issue "support BIP39 mnemonic in descriptors":
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-1514419208)
Briefly looked at this again. Importing with private key functionality seems doable (not just watch-only).
1. We need to store the extended private key of the root node (easier after #26728): internally our wallet does this by storing the full extended public key and a regular `CKey` for the private key.
2. We don't have to store the mnemonic. But if we wanted to, we have to make sure it gets encrypted along with other private key material.
3. Both seed and passphrase would (initially) be c
...
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-1514419208)
Briefly looked at this again. Importing with private key functionality seems doable (not just watch-only).
1. We need to store the extended private key of the root node (easier after #26728): internally our wallet does this by storing the full extended public key and a regular `CKey` for the private key.
2. We don't have to store the mnemonic. But if we wanted to, we have to make sure it gets encrypted along with other private key material.
3. Both seed and passphrase would (initially) be c
...
💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions":
(https://github.com/bitcoin/bitcoin/pull/26088#issuecomment-1514450115)
@aureleoules @fanquake
I think this could be re-opened to solve https://github.com/bitcoin/bitcoin/issues/25270 if @aureleoules is happy to pick it up again, otherwise I'd be happy to?
Whilst #17127 has given us more sane and secure defaults, permitting group members to access the cookie without resorting to `-startupnotify` wortkarounds or similar seems reasonable to me.
One the one hand any processes with access to the cookie could just as well be running as the same user as `bitcoin
...
(https://github.com/bitcoin/bitcoin/pull/26088#issuecomment-1514450115)
@aureleoules @fanquake
I think this could be re-opened to solve https://github.com/bitcoin/bitcoin/issues/25270 if @aureleoules is happy to pick it up again, otherwise I'd be happy to?
Whilst #17127 has given us more sane and secure defaults, permitting group members to access the cookie without resorting to `-startupnotify` wortkarounds or similar seems reasonable to me.
One the one hand any processes with access to the cookie could just as well be running as the same user as `bitcoin
...
💬 willcl-ark commented on issue "large wallet: Bitcoind freezes":
(https://github.com/bitcoin/bitcoin/issues/15148#issuecomment-1514458533)
@RomanBelkov is this issue still present on the current release of Bitcoin Core (v24.0.1), as there have been many wallet improvements in the years since this issue was opened?
(https://github.com/bitcoin/bitcoin/issues/15148#issuecomment-1514458533)
@RomanBelkov is this issue still present on the current release of Bitcoin Core (v24.0.1), as there have been many wallet improvements in the years since this issue was opened?
💬 RomanBelkov commented on issue "large wallet: Bitcoind freezes":
(https://github.com/bitcoin/bitcoin/issues/15148#issuecomment-1514465381)
@willcl-ark unfortunately I'm unable to comment on issue as I'm not working with Bitcoin Core anymore for quite a while. I guess the issue can be closed as there was no development/reports from other people for 3 years :)
(https://github.com/bitcoin/bitcoin/issues/15148#issuecomment-1514465381)
@willcl-ark unfortunately I'm unable to comment on issue as I'm not working with Bitcoin Core anymore for quite a while. I guess the issue can be closed as there was no development/reports from other people for 3 years :)