💬 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 :)
✅ RomanBelkov closed an issue: "large wallet: Bitcoind freezes"
(https://github.com/bitcoin/bitcoin/issues/15148)
(https://github.com/bitcoin/bitcoin/issues/15148)
💬 TheCharlatan commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1514467370)
Thank you for the reviews, decided to address the left over nits here:
Updated ec51c252de42e11906f114ac05e476fd88ca2176 -> be55f545d53d44fdcf2d8ae802e9eae551d120c6 ([splitSystemArgs_3](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_3) -> [splitSystemArgs_4](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_3..splitSystemArgs_4)):
* Addressed @ryanofsky's [comment](https://github.com/b
...
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1514467370)
Thank you for the reviews, decided to address the left over nits here:
Updated ec51c252de42e11906f114ac05e476fd88ca2176 -> be55f545d53d44fdcf2d8ae802e9eae551d120c6 ([splitSystemArgs_3](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_3) -> [splitSystemArgs_4](https://github.com/TheCharlatan/bitcoin/commits/splitSystemArgs_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemArgs_3..splitSystemArgs_4)):
* Addressed @ryanofsky's [comment](https://github.com/b
...
⚠️ batriskaweb3 opened an issue: "Compiling a bitcoin core version that accepts transactions over 100vkb"
(https://github.com/bitcoin/bitcoin/issues/27490)
### Please describe the feature you'd like to see added.
i couldnt manage to find any label that fits this,
when people try to inscribe ordinals that are bigger than 400kb they can not be able to because bitcoin core doesnt allow transactions over 100kvb
even though that would require the person to be a miner or pay miners for it but it still would be nice to guide us on how to compile such bitcoin core version
### Is your feature related to a problem, if so please describe it.
_No response
...
(https://github.com/bitcoin/bitcoin/issues/27490)
### Please describe the feature you'd like to see added.
i couldnt manage to find any label that fits this,
when people try to inscribe ordinals that are bigger than 400kb they can not be able to because bitcoin core doesnt allow transactions over 100kvb
even though that would require the person to be a miner or pay miners for it but it still would be nice to guide us on how to compile such bitcoin core version
### Is your feature related to a problem, if so please describe it.
_No response
...
📝 TheCharlatan opened a pull request: "Kernel chain type"
(https://github.com/bitcoin/bitcoin/pull/27491)
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is also a follow up to #26177.
It replaces pull request https://github.com/bitcoin/bitcoin/pull/27294, which just moved the constants to a new file, but did not re-declare them as enums.
The code move of the chain name constants out of the `cha
...
(https://github.com/bitcoin/bitcoin/pull/27491)
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is also a follow up to #26177.
It replaces pull request https://github.com/bitcoin/bitcoin/pull/27294, which just moved the constants to a new file, but did not re-declare them as enums.
The code move of the chain name constants out of the `cha
...
💬 TheCharlatan commented on pull request "refactor: Move chain names to the util library":
(https://github.com/bitcoin/bitcoin/pull/27294#issuecomment-1514490390)
Re https://github.com/bitcoin/bitcoin/pull/27294#pullrequestreview-1376476610
> However I think as long as every usage of the constants is changing anyway, it would be better to turn this into to enum and not introduce an odd namespace with string symbols. Suggestion would be to add a new `src/util/chain_types.h` file with:
I initially wanted to implement the constants as an enum, but then saw in the git history, that they were originally an enum and converted to string constants in https://
...
(https://github.com/bitcoin/bitcoin/pull/27294#issuecomment-1514490390)
Re https://github.com/bitcoin/bitcoin/pull/27294#pullrequestreview-1376476610
> However I think as long as every usage of the constants is changing anyway, it would be better to turn this into to enum and not introduce an odd namespace with string symbols. Suggestion would be to add a new `src/util/chain_types.h` file with:
I initially wanted to implement the constants as an enum, but then saw in the git history, that they were originally an enum and converted to string constants in https://
...
❤1
✅ TheCharlatan closed a pull request: "refactor: Move chain names to the util library"
(https://github.com/bitcoin/bitcoin/pull/27294)
(https://github.com/bitcoin/bitcoin/pull/27294)