💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514131)
This is already included 3 lines higher.
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514131)
This is already included 3 lines higher.
💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514797)
```suggestion
constexpr int DEFAULT_TOR_CONTROL_PORT = 9051;
```
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297514797)
```suggestion
constexpr int DEFAULT_TOR_CONTROL_PORT = 9051;
```
📝 crywolf opened a pull request: "test: Minor fix in test - locale in terminal"
(https://github.com/bitcoin/bitcoin/pull/28286)
Running tests with non-English locale set in Linux terminal makes system tests (`system_tests/run_command`) fail. Setting the locale in failing test explicitly to 'C' fixes the failure.
_How to reproduce test failure:_
Setting LC_ALL=cs_CZ.UTF-8 (`export LC_ALL=cs_CZ.UTF-8`) in terminal and then running `make check` should be enough.
_Fix:_
My terminal is by default set to non-English locale and the test failed because it checks for English error message, but the terminal produces error
...
(https://github.com/bitcoin/bitcoin/pull/28286)
Running tests with non-English locale set in Linux terminal makes system tests (`system_tests/run_command`) fail. Setting the locale in failing test explicitly to 'C' fixes the failure.
_How to reproduce test failure:_
Setting LC_ALL=cs_CZ.UTF-8 (`export LC_ALL=cs_CZ.UTF-8`) in terminal and then running `make check` should be enough.
_Fix:_
My terminal is by default set to non-English locale and the test failed because it checks for English error message, but the terminal produces error
...
💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682682090)
> Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
rebased properly in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682682090)
> Please squash your commits according to https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits
rebased properly in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297526595)
thanks, addressed in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297526595)
thanks, addressed in [5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
💬 crywolf commented on pull request "test: Minor fix in test - locale in terminal":
(https://github.com/bitcoin/bitcoin/pull/28286#issuecomment-1682682796)
During the workshop wit Gloria @glozow on Dev/Hack/Day (part of BTC Prague conference) my tests failed. Later that day I found what the problem was and now I am finally presenting my fix as an attempt to make my first PR in this repo.
(https://github.com/bitcoin/bitcoin/pull/28286#issuecomment-1682682796)
During the workshop wit Gloria @glozow on Dev/Hack/Day (part of BTC Prague conference) my tests failed. Later that day I found what the problem was and now I am finally presenting my fix as an attempt to make my first PR in this repo.
💬 kevkevinpal commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297527127)
thanks didnt know that fixed in
[5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297527127)
thanks didnt know that fixed in
[5d94b1f](https://github.com/bitcoin/bitcoin/pull/28101/commits/5d94b1faecd8fc98017510d86f6ea5dc002ec0a3)
💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538)
Second sentence seems redundant:
```
$ ./src/bitcoind -h | grep -A3 torcontrol
-torcontrol=<ip>:<port>
Tor control host and port to use if onion listening enabled (default:
127.0.0.1:9051). If no port is specified, the default port will
be used (default: 9051).
```
```suggestion
+ argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will
...
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538)
Second sentence seems redundant:
```
$ ./src/bitcoind -h | grep -A3 torcontrol
-torcontrol=<ip>:<port>
Tor control host and port to use if onion listening enabled (default:
127.0.0.1:9051). If no port is specified, the default port will
be used (default: 9051).
```
```suggestion
+ argsman.AddArg("-torcontrol=<ip>:<port>", strprintf("Tor control host and port to use if onion listening enabled (default: %s). If no port is specified, the default port of %i will
...
💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682692907)
Approach ACK modulo https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538
(https://github.com/bitcoin/bitcoin/pull/28101#issuecomment-1682692907)
Approach ACK modulo https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297532538
💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1682703777)
> Had GCC segfault more than once when cross-compiling s390x.
I also ran into this on aarch64, and I presume it is just the CPU overheating, because it passed with `MAKEJOBS="-j1"`
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1682703777)
> Had GCC segfault more than once when cross-compiling s390x.
I also ran into this on aarch64, and I presume it is just the CPU overheating, because it passed with `MAKEJOBS="-j1"`
💬 MarcoFalke commented on pull request "ci: Disable --coverage temporarily":
(https://github.com/bitcoin/bitcoin/pull/28285#issuecomment-1682709592)
Looks like CI is red since https://github.com/bitcoin/bitcoin/commit/ecb20563b6a0cdd615628da9caa1e7389998119c, but on the pull, it passed last week: https://cirrus-ci.com/task/5524872076460032
(https://github.com/bitcoin/bitcoin/pull/28285#issuecomment-1682709592)
Looks like CI is red since https://github.com/bitcoin/bitcoin/commit/ecb20563b6a0cdd615628da9caa1e7389998119c, but on the pull, it passed last week: https://cirrus-ci.com/task/5524872076460032
💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1297560652)
Fixed, thank you.
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1297560652)
Fixed, thank you.
💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1682735790)
Updated per `git range-diff 6ce5e8f ce8faad 05143b2` to take review feedback by @ajtowns (thanks!) and rebase for CI.
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1682735790)
Updated per `git range-diff 6ce5e8f ce8faad 05143b2` to take review feedback by @ajtowns (thanks!) and rebase for CI.
📝 mzumsande opened a pull request: "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation"
(https://github.com/bitcoin/bitcoin/pull/28287)
This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer.
While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes.
Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged):
The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to
...
(https://github.com/bitcoin/bitcoin/pull/28287)
This adds a `sendmsgtopeer` rpc (for testing only) that allows a node to send a message (provided in hex) to a peer.
While we would usually use a `p2p` object instead of a node for this in the test framework, that isn't possible in situations where this message needs to trigger an actual interaction of multiple nodes.
Use this rpc to add test coverage for the bug fixed in #27981 (that just got merged):
The test lets two nodes (almost) simultaneously send a single large (4MB) p2p message to
...
💬 luke-jr commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1276773348)
Extraneous `\"` shouldn't be there, this is an array
(https://github.com/bitcoin/bitcoin/pull/27351#discussion_r1276773348)
Extraneous `\"` shouldn't be there, this is an array
💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297598582)
I believe that's true, though only due to optimistic send itself (which guarantees that as soon as something is added to `vSendMsg`, it's immediately at least attempted to be moved to the transport). Without it, if there were two consecutive `PushMessage` without intervening `SocketSendData`, `vSendMsg` would already have something in it on the second one.
Because of that, I think it's more "obviously correct" to do the full check here.
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297598582)
I believe that's true, though only due to optimistic send itself (which guarantees that as soon as something is added to `vSendMsg`, it's immediately at least attempted to be moved to the transport). Without it, if there were two consecutive `PushMessage` without intervening `SocketSendData`, `vSendMsg` would already have something in it on the second one.
Because of that, I think it's more "obviously correct" to do the full check here.
💬 sipa commented on pull request "net: transport abstraction":
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297627349)
Right, so I think `!node.m_sock` only happens when the node is already disconnected, or will never actually be connected to anything (in tests). I've added the following comment:
> There is no socket in case we've already disconnected, or in test cases without real connections. In these cases, we bail out immediately and just leave things in the send queue and transport.
Further up, above the `SetMessageToSend` call, I've added:
> This fails when there is an existing message still being
...
(https://github.com/bitcoin/bitcoin/pull/28165#discussion_r1297627349)
Right, so I think `!node.m_sock` only happens when the node is already disconnected, or will never actually be connected to anything (in tests). I've added the following comment:
> There is no socket in case we've already disconnected, or in test cases without real connections. In these cases, we bail out immediately and just leave things in the send queue and transport.
Further up, above the `SetMessageToSend` call, I've added:
> This fails when there is an existing message still being
...
📝 brunoerg opened a pull request: "test: fix 'unknown named parameter' test in `wallet_basic`"
(https://github.com/bitcoin/bitcoin/pull/28288)
This PR removes loop when testing an unknown named parameter. They don't have any effect.
(https://github.com/bitcoin/bitcoin/pull/28288)
This PR removes loop when testing an unknown named parameter. They don't have any effect.
💬 brunoerg commented on pull request "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation":
(https://github.com/bitcoin/bitcoin/pull/28287#issuecomment-1682814806)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28287#issuecomment-1682814806)
Concept ACK
💬 AndriiHlukhovskyi commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682836022)
Спасибо огромное очень познавательно я думаю что я тоже что то пропустил
!!! 🔥
чт, 17 авг. 2023 г., 12:58 mehran636311 ***@***.***>:
> Pellse full item my account open and stat
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BAMCSAPI2J4K356QRG4TQDLXVX2PDANCNFSM6AAAAAAW6JC3HY>
> .
> You are receiving this because you commented.M
...
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682836022)
Спасибо огромное очень познавательно я думаю что я тоже что то пропустил
!!! 🔥
чт, 17 авг. 2023 г., 12:58 mehran636311 ***@***.***>:
> Pellse full item my account open and stat
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BAMCSAPI2J4K356QRG4TQDLXVX2PDANCNFSM6AAAAAAW6JC3HY>
> .
> You are receiving this because you commented.M
...