💬 TheCharlatan commented on pull request "guix: remove errant leftover from #29648":
(https://github.com/bitcoin/bitcoin/pull/29787#issuecomment-2035556508)
Re https://github.com/bitcoin/bitcoin/pull/29787#issuecomment-2035514900
> When you say you are running a guix build, do you mean you are running the guix operating system on a separate machine?
Guix builds describe this project's method for compiling reproducible binaries (see https://reproducible-builds.org/). It is the same method that is used for Bitcoin Core release binaries. Guix is the package and build environment manager used for this process. In this context guix is installed on
...
(https://github.com/bitcoin/bitcoin/pull/29787#issuecomment-2035556508)
Re https://github.com/bitcoin/bitcoin/pull/29787#issuecomment-2035514900
> When you say you are running a guix build, do you mean you are running the guix operating system on a separate machine?
Guix builds describe this project's method for compiling reproducible binaries (see https://reproducible-builds.org/). It is the same method that is used for Bitcoin Core release binaries. Guix is the package and build environment manager used for this process. In this context guix is installed on
...
💬 murchandamus commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1550494056)
Thanks! Great, I’ll attempt another more thorough review at a later time.
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1550494056)
Thanks! Great, I’ll attempt another more thorough review at a later time.
💬 kosuodhmwa commented on issue "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?":
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2035593511)
Now i saw that i also have message like that:
`2024-04-03T20:43:47Z Socks5() connect to 77.7.121.181:8333 failed: general failure`
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2035593511)
Now i saw that i also have message like that:
`2024-04-03T20:43:47Z Socks5() connect to 77.7.121.181:8333 failed: general failure`
💬 kosuodhmwa commented on issue "Tons of Socks5() connect to x.x.x.x:8333 failed: connection refused-messages when i use TOR - why?":
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2035617849)
**_"Do you get only IP addresses in those messages (x.x.x.x) or also onion addresses (e.g. ydvbxdzs6w5wefifiqsqntpbd7tliofenqih5hlnz34546fvy4ab7iid.onion)?"_**
No, only for IP addresses as shown on post above. @vasild
(https://github.com/bitcoin/bitcoin/issues/29759#issuecomment-2035617849)
**_"Do you get only IP addresses in those messages (x.x.x.x) or also onion addresses (e.g. ydvbxdzs6w5wefifiqsqntpbd7tliofenqih5hlnz34546fvy4ab7iid.onion)?"_**
No, only for IP addresses as shown on post above. @vasild
💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2035711744)
Thanks for having a look @tdb3!
As mentioned in the summary, running without `--jobs=16` makes the tests succeed. I guess it is not a common parameter (even less common in conjunction with `--extended`).
Your questions prompted me to perform the tests on older release branches to see if behavior changed recently. (No other tests failed but I didn't let `feature_dbcrash.py` and complete since it takes around 45 minutes (occasionally skipped some other stragglers as well)).
*27.x* - Only
...
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2035711744)
Thanks for having a look @tdb3!
As mentioned in the summary, running without `--jobs=16` makes the tests succeed. I guess it is not a common parameter (even less common in conjunction with `--extended`).
Your questions prompted me to perform the tests on older release branches to see if behavior changed recently. (No other tests failed but I didn't let `feature_dbcrash.py` and complete since it takes around 45 minutes (occasionally skipped some other stragglers as well)).
*27.x* - Only
...
🤔 pablomartin4btc requested changes to a pull request: "Don't permit port in proxy IP option"
(https://github.com/bitcoin-core/gui/pull/813#pullrequestreview-1978210897)
Concept ACK 94173eac6e89f374f1a98b5d0e449a0a07ed832d
I'd suggest to use a regex validator for the IP address, it's much simpler I think and the user won't be allow to enter any other symbols or chars (typo?) that aren't digits (plus user can't type more than 3 subnets separated by dots where currently you could and any symbols):
```diff
diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index dd654a7ab..1fabee7f2 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsd
...
(https://github.com/bitcoin-core/gui/pull/813#pullrequestreview-1978210897)
Concept ACK 94173eac6e89f374f1a98b5d0e449a0a07ed832d
I'd suggest to use a regex validator for the IP address, it's much simpler I think and the user won't be allow to enter any other symbols or chars (typo?) that aren't digits (plus user can't type more than 3 subnets separated by dots where currently you could and any symbols):
```diff
diff --git a/src/qt/optionsdialog.cpp b/src/qt/optionsdialog.cpp
index dd654a7ab..1fabee7f2 100644
--- a/src/qt/optionsdialog.cpp
+++ b/src/qt/optionsd
...
💬 cbergqvist commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2035716197)
@itornaza subtly prompted me to do some tests to find the sweet spot for the number of jobs on my hardware, which luckily helps motivate why running with 16 jobs is not unreasonable.
This is wall time running on a [RAM disk](https://github.com/bitcoin/bitcoin/blob/master/test/README.md#speed-up-test-runs-with-a-ram-disk) **without `--extended`**.
```
jobs duration (s) results
4 816 all passed
8 454 all passed
12 342 all passed
16 300 all pas
...
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2035716197)
@itornaza subtly prompted me to do some tests to find the sweet spot for the number of jobs on my hardware, which luckily helps motivate why running with 16 jobs is not unreasonable.
This is wall time running on a [RAM disk](https://github.com/bitcoin/bitcoin/blob/master/test/README.md#speed-up-test-runs-with-a-ram-disk) **without `--extended`**.
```
jobs duration (s) results
4 816 all passed
8 454 all passed
12 342 all passed
16 300 all pas
...
💬 fjahr commented on pull request "ThreadSanitizer: Fix #29767":
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2035727894)
Code review ACK bbe82c116e72ca0638751e063bf564cd1fe5c4d5
This looks like the correct fix to me. CI failure is unrelated.
(https://github.com/bitcoin/bitcoin/pull/29776#issuecomment-2035727894)
Code review ACK bbe82c116e72ca0638751e063bf564cd1fe5c4d5
This looks like the correct fix to me. CI failure is unrelated.
💬 achow101 commented on pull request "wallet: Construct ScriptPubKeyMans with all data rather than loaded progressively":
(https://github.com/bitcoin/bitcoin/pull/28333#issuecomment-2035728966)
Added a commit so that the fuzzer will skip the type of inputs that was causing the previous failure. This should be safe as that failure is not reachable in normal usage. It can only be reached if the user's wallet is corrupted, and in that case, the runtime error would propagate up to them so there would not be a crash.
(https://github.com/bitcoin/bitcoin/pull/28333#issuecomment-2035728966)
Added a commit so that the fuzzer will skip the type of inputs that was causing the previous failure. This should be safe as that failure is not reachable in normal usage. It can only be reached if the user's wallet is corrupted, and in that case, the runtime error would propagate up to them so there would not be a crash.
✅ GopherJ closed an issue: "Always exit silently"
(https://github.com/bitcoin/bitcoin/issues/29783)
(https://github.com/bitcoin/bitcoin/issues/29783)
💬 GopherJ commented on issue "Always exit silently":
(https://github.com/bitcoin/bitcoin/issues/29783#issuecomment-2035808444)
closing as it's now more related to system instead of bitcoin. thanks for your help!
(https://github.com/bitcoin/bitcoin/issues/29783#issuecomment-2035808444)
closing as it's now more related to system instead of bitcoin. thanks for your help!
💬 tdb3 commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550751325)
Thank you for reviewing. **Would you be ok with a simplified approach of writing all unit test output to stdout, with this output being shown when unit tests fail?** This seems a bit cleaner and more consistent than selectively placing all unit test output to stdout or stderr depending on test pass/fail.
### More info
`TextTestRunner` lets us print unit test output to one stream. Previously (before this PR), `TextTestRunner` wrote all output to stderr regardless of pass/fail but doing t
...
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550751325)
Thank you for reviewing. **Would you be ok with a simplified approach of writing all unit test output to stdout, with this output being shown when unit tests fail?** This seems a bit cleaner and more consistent than selectively placing all unit test output to stdout or stderr depending on test pass/fail.
### More info
`TextTestRunner` lets us print unit test output to one stream. Previously (before this PR), `TextTestRunner` wrote all output to stderr regardless of pass/fail but doing t
...
💬 sipa commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770682)
Done.
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770682)
Done.
💬 sipa commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770817)
Done.
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770817)
Done.
💬 sipa commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770899)
Done.
(https://github.com/bitcoin/bitcoin/pull/29757#discussion_r1550770899)
Done.
💬 pablomartin4btc commented on issue "Node shutting down immediately":
(https://github.com/bitcoin-core/gui/issues/809#issuecomment-2036016180)
Shouldn't the title of the issue be updated? Perhaps adding " _- root cause: incorrect proxy IP address input_" or something? It'll be more useful navigating thru the issues list I think.
(https://github.com/bitcoin-core/gui/issues/809#issuecomment-2036016180)
Shouldn't the title of the issue be updated? Perhaps adding " _- root cause: incorrect proxy IP address input_" or something? It'll be more useful navigating thru the issues list I think.
💬 tdb3 commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2036073972)
>As mentioned in the summary, running without --jobs=16 makes the tests succeed
>
Ok, this gives me a bit more confidence that this may just be a simple case of resource contention from high concurrency slowing things down. Omitting `--jobs` causes the default value of 4 jobs to be used. In general, IIRC jobs != threads of execution (since some tests run multiple nodes, etc.). I was also able to reproduce the failure of `feature_index_prune.py` with `--jobs=18` (on master).
> Your questi
...
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2036073972)
>As mentioned in the summary, running without --jobs=16 makes the tests succeed
>
Ok, this gives me a bit more confidence that this may just be a simple case of resource contention from high concurrency slowing things down. Omitting `--jobs` causes the default value of 4 jobs to be used. In general, IIRC jobs != threads of execution (since some tests run multiple nodes, etc.). I was also able to reproduce the failure of `feature_index_prune.py` with `--jobs=18` (on master).
> Your questi
...
💬 maflcko commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550988846)
> **Would you be ok with a simplified approach of writing all unit test output to stdout, with this output being shown when unit tests fail?**
Sure. That'd be fine as well
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550988846)
> **Would you be ok with a simplified approach of writing all unit test output to stdout, with this output being shown when unit tests fail?**
Sure. That'd be fine as well
💬 maflcko commented on pull request "test: Run framework unit tests in parallel":
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550989523)
I was thinking about calling the script (passing) directly:
```
python3 ./test/functional/feature_framework_unit_tests.py
(https://github.com/bitcoin/bitcoin/pull/29771#discussion_r1550989523)
I was thinking about calling the script (passing) directly:
```
python3 ./test/functional/feature_framework_unit_tests.py
💬 willcl-ark commented on pull request "Don't permit port in proxy IP option":
(https://github.com/bitcoin-core/gui/pull/813#issuecomment-2036336106)
> I'd suggest to use a regex validator for the IP address, it's much simpler I think and the user won't be allow to enter any other symbols or chars (typo?) that aren't digits (plus user can't type more than 3 subnets separated by dots where currently you could and any symbols)
I'm not convinced that a large regex is "simpler" than checking if a ":" is in the string?
Thanks for the other suggestions, but I don't feel that interested in picking them here with the QML work going on, and them
...
(https://github.com/bitcoin-core/gui/pull/813#issuecomment-2036336106)
> I'd suggest to use a regex validator for the IP address, it's much simpler I think and the user won't be allow to enter any other symbols or chars (typo?) that aren't digits (plus user can't type more than 3 subnets separated by dots where currently you could and any symbols)
I'm not convinced that a large regex is "simpler" than checking if a ":" is in the string?
Thanks for the other suggestions, but I don't feel that interested in picking them here with the QML work going on, and them
...