💬 S3RK commented on pull request "Disable and uncheck blank when private keys are disabled":
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1610881553)
@ryanofsky no no, I don't think this change is making it worse.
From my perspective, it's a in improvement for core developers, because it provides clarity on the wallet flags usage.
UX wise it's ~0 as the flags are implementation details. Both pre- and post-PR whether blank checkbox is disabled or automatically checked is equally not explained to the users.
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1610881553)
@ryanofsky no no, I don't think this change is making it worse.
From my perspective, it's a in improvement for core developers, because it provides clarity on the wallet flags usage.
UX wise it's ~0 as the flags are implementation details. Both pre- and post-PR whether blank checkbox is disabled or automatically checked is equally not explained to the users.
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244795380)
I tried extracting the mocked sockets out from `test/util/net.h` into a separate `test/util/mocked_sockets.h` and then compared the re-compilation time if that is modified (e.g. something in `DynSock` declaration is changed). In both cases recompilation takes ~28 seconds, so it makes no difference (in practice).
Here is the change (on top of this PR):
<details>
<summary>mocked_sockets.h</summary>
```diff
commit 398a2d5717b6ef6760e48eac92aa92127157a67a (HEAD -> e2e_tests)
Parent: 4557
...
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244795380)
I tried extracting the mocked sockets out from `test/util/net.h` into a separate `test/util/mocked_sockets.h` and then compared the re-compilation time if that is modified (e.g. something in `DynSock` declaration is changed). In both cases recompilation takes ~28 seconds, so it makes no difference (in practice).
Here is the change (on top of this PR):
<details>
<summary>mocked_sockets.h</summary>
```diff
commit 398a2d5717b6ef6760e48eac92aa92127157a67a (HEAD -> e2e_tests)
Parent: 4557
...
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244823822)
(No opinion on the change), just
> In both cases recompilation takes ~28 seconds
Seems odd that re-compilation of all tests takes only 28 seconds when `mocked_sockets.h` is modified, which is included in `setup_common.h`, which is included in all test files, no?
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244823822)
(No opinion on the change), just
> In both cases recompilation takes ~28 seconds
Seems odd that re-compilation of all tests takes only 28 seconds when `mocked_sockets.h` is modified, which is included in `setup_common.h`, which is included in all test files, no?
👍 Dezzj approved a pull request: "Tests: Fill out dust limit unit test for known types except bare multisig"
(https://github.com/bitcoin/bitcoin/pull/26875#pullrequestreview-1502509586)
Approved
(https://github.com/bitcoin/bitcoin/pull/26875#pullrequestreview-1502509586)
Approved
📝 vasild opened a pull request: "test: remove race in the user-agent reception check"
(https://github.com/bitcoin/bitcoin/pull/27986)
In `add_p2p_connection()` we connect to `bitcoind` from the Python client and check that it has received our version string.
This check looked up the last/newest entry from `getpeerinfo` RPC, assuming that it must be the connection we have just opened. But this will not be the case if a new inbound or outbound connection is made to/from `bitcoind` in the meantime.
Instead of the last entry in `getpeerinfo`, check all and find the one which corresponds to our connection using our outgoing a
...
(https://github.com/bitcoin/bitcoin/pull/27986)
In `add_p2p_connection()` we connect to `bitcoind` from the Python client and check that it has received our version string.
This check looked up the last/newest entry from `getpeerinfo` RPC, assuming that it must be the connection we have just opened. But this will not be the case if a new inbound or outbound connection is made to/from `bitcoind` in the meantime.
Instead of the last entry in `getpeerinfo`, check all and find the one which corresponds to our connection using our outgoing a
...
💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1610998377)
I stumbled on this playing with https://github.com/bitcoin/bitcoin/pull/27509:
https://cirrus-ci.com/task/6722465808777216?logs=ci#L8761-L8771
<details>
<summary>CI log</summary>
```
2023-06-27T09:50:26.796000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/tmp/cirrus-ci-build/ci/scratc
...
(https://github.com/bitcoin/bitcoin/pull/27986#issuecomment-1610998377)
I stumbled on this playing with https://github.com/bitcoin/bitcoin/pull/27509:
https://cirrus-ci.com/task/6722465808777216?logs=ci#L8761-L8771
<details>
<summary>CI log</summary>
```
2023-06-27T09:50:26.796000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()
File "/tmp/cirrus-ci-build/ci/scratc
...
💬 MarcoFalke commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244887087)
Why would this ever be not found, given that the previously the connection was sync_with_ping'd?
In any case, you can use `next()` to do the assertion. Roughly:
```py
next([p if p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}" p in self.getpeerinfo()])
assert_equal(p["subver"], P2P_SUBVERSION)
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244887087)
Why would this ever be not found, given that the previously the connection was sync_with_ping'd?
In any case, you can use `next()` to do the assertion. Roughly:
```py
next([p if p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}" p in self.getpeerinfo()])
assert_equal(p["subver"], P2P_SUBVERSION)
💬 willcl-ark commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1611033313)
> I get this when compiling with both gcc and clang-16. I am also using libevent 2.1.12. Is there anything unique about your setup @willcl-ark ? Also, did you Ctrl-C the `waitforblockheight` command before trying to stop bitcoind?
I've had more reports of this deadlock so come back to try and repro it again, but don't seem to be able to no matter what I try 🤷🏼♂️
I am on Ubuntu 23.04, also using Clang 16 and libevent 2.1.12-stable-8ubuntu3. It doesn't look like there are any debian patc
...
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1611033313)
> I get this when compiling with both gcc and clang-16. I am also using libevent 2.1.12. Is there anything unique about your setup @willcl-ark ? Also, did you Ctrl-C the `waitforblockheight` command before trying to stop bitcoind?
I've had more reports of this deadlock so come back to try and repro it again, but don't seem to be able to no matter what I try 🤷🏼♂️
I am on Ubuntu 23.04, also using Clang 16 and libevent 2.1.12-stable-8ubuntu3. It doesn't look like there are any debian patc
...
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244922368)
Compiling everything from scratch (after `git clean -fdx`) takes about 1m 30sec (including autogen and ./configure which are single-threaded). The CPU on the machine is:
> AMD Ryzen 9 7950X 16-Core Processor (4491.70-MHz K8-class CPU)
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244922368)
Compiling everything from scratch (after `git clean -fdx`) takes about 1m 30sec (including autogen and ./configure which are single-threaded). The CPU on the machine is:
> AMD Ryzen 9 7950X 16-Core Processor (4491.70-MHz K8-class CPU)
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244937645)
For a useful benchmark, you'll have to measure the total CPU time, not the CPU time of just the translation unit that takes the longest.
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244937645)
For a useful benchmark, you'll have to measure the total CPU time, not the CPU time of just the translation unit that takes the longest.
💬 vasild commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244943695)
> Why would this ever be not found
Well, I do not know, maybe some typo in `p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}"` or some other unexpected thing. That is just another sanity check.
Taking a step back I wonder if this check as much value - why would `bitcoind` not have received our user agent string? I wouldn't object if somebody wants to drop the check altogether.
> p=next([p if p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}" p in self.getpeerinfo()])
I find this compl
...
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244943695)
> Why would this ever be not found
Well, I do not know, maybe some typo in `p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}"` or some other unexpected thing. That is just another sanity check.
Taking a step back I wonder if this check as much value - why would `bitcoind` not have received our user agent string? I wouldn't object if somebody wants to drop the check altogether.
> p=next([p if p["addr"] == f"{us_addrport[0]}:{us_addrport[1]}" p in self.getpeerinfo()])
I find this compl
...
📝 fanquake opened a pull request: "ci: remove duplicate bsdmainutils from CI configs"
(https://github.com/bitcoin/bitcoin/pull/27987)
`bsdmainutils` is included in `CI_BASE_PACKAGES`.
(https://github.com/bitcoin/bitcoin/pull/27987)
`bsdmainutils` is included in `CI_BASE_PACKAGES`.
💬 MarcoFalke commented on pull request "test: remove race in the user-agent reception check":
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244951363)
> That is just another sanity check.
Might be better to check the length is one (unique), if the check is kept?
(https://github.com/bitcoin/bitcoin/pull/27986#discussion_r1244951363)
> That is just another sanity check.
Might be better to check the length is one (unique), if the check is kept?
💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244969873)
```
PR:
real 0m29.623s
user 10m58.337s
sys 0m51.187s
real 0m29.829s
user 10m58.359s
sys 0m50.912s
mocked_sockets.h:
real 0m29.528s
user 11m12.208s
sys 1m0.406s
real 0m28.831s
user 10m45.615s
sys 0m49.680s
```
I guess the "total CPU time" is real+sys from above. It all seems within noise. In both cases all tests would be recompiled, maybe I got @jonatack's comment wrong?
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244969873)
```
PR:
real 0m29.623s
user 10m58.337s
sys 0m51.187s
real 0m29.829s
user 10m58.359s
sys 0m50.912s
mocked_sockets.h:
real 0m29.528s
user 11m12.208s
sys 1m0.406s
real 0m28.831s
user 10m45.615s
sys 0m49.680s
```
I guess the "total CPU time" is real+sys from above. It all seems within noise. In both cases all tests would be recompiled, maybe I got @jonatack's comment wrong?
💬 hebasto commented on pull request "ci: remove duplicate bsdmainutils from CI configs":
(https://github.com/bitcoin/bitcoin/pull/27987#issuecomment-1611106896)
What about the python3 package?
(https://github.com/bitcoin/bitcoin/pull/27987#issuecomment-1611106896)
What about the python3 package?
💬 TheCharlatan commented on pull request "kernel: Add interrupt class":
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1611113265)
Rebased 001acf37dff40688d82c3b1a7ff9ed3901addb33 -> 6eb33bd0c21b3e075fbab596351cacafdc947472 ([kernelInterrupt_8](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_8) -> [kernelInterrupt_9](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInterrupt_8..kernelInterrupt_9)).
* Fixed conflict with #27896
(https://github.com/bitcoin/bitcoin/pull/27861#issuecomment-1611113265)
Rebased 001acf37dff40688d82c3b1a7ff9ed3901addb33 -> 6eb33bd0c21b3e075fbab596351cacafdc947472 ([kernelInterrupt_8](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_8) -> [kernelInterrupt_9](https://github.com/TheCharlatan/bitcoin/commits/kernelInterrupt_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInterrupt_8..kernelInterrupt_9)).
* Fixed conflict with #27896
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244981230)
> I guess the "total CPU time" is real+sys from above
user+sys, but thanks for re-checking.
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1244981230)
> I guess the "total CPU time" is real+sys from above
user+sys, but thanks for re-checking.
💬 tansanDOTeth commented on issue "Keep getting errors after a while of syncing":
(https://github.com/bitcoin/bitcoin/issues/27972#issuecomment-1611123854)
> Has anyone ever put the leveldb directory successfully on an external drive, I am not sure if this is even supported by leveldb?
>
> Edit: Alternatively, leveldb may not support your filesystem?
This looks like the fix. I am at 35% now and haven't had issues since reformatting as APFS. Seems like ExFAT is the culprit. I'm going to close this now, thank you!
(https://github.com/bitcoin/bitcoin/issues/27972#issuecomment-1611123854)
> Has anyone ever put the leveldb directory successfully on an external drive, I am not sure if this is even supported by leveldb?
>
> Edit: Alternatively, leveldb may not support your filesystem?
This looks like the fix. I am at 35% now and haven't had issues since reformatting as APFS. Seems like ExFAT is the culprit. I'm going to close this now, thank you!
✅ tansanDOTeth closed an issue: "Keep getting errors after a while of syncing"
(https://github.com/bitcoin/bitcoin/issues/27972)
(https://github.com/bitcoin/bitcoin/issues/27972)
💬 hebasto commented on pull request "ci: remove duplicate bsdmainutils from CI configs":
(https://github.com/bitcoin/bitcoin/pull/27987#issuecomment-1611145723)
> What about the `python3` package?
In `ci/test/00_setup_env_i686_centos.sh` as well?
(https://github.com/bitcoin/bitcoin/pull/27987#issuecomment-1611145723)
> What about the `python3` package?
In `ci/test/00_setup_env_i686_centos.sh` as well?