💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2560911214)
`5766bbefa9...b448b01494`: avoid moving `GetRandomNodeEvictionCandidates()` as suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731 and https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
(https://github.com/bitcoin/bitcoin/pull/30205#issuecomment-2560911214)
`5766bbefa9...b448b01494`: avoid moving `GetRandomNodeEvictionCandidates()` as suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1858648731 and https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
💬 L508726 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150696862)
Oky my bro some how it will detamine
(https://github.com/bitcoin/bitcoin/commit/ce6df7df9bab2405cfe7d6e382f5682cf30de476#r150696862)
Oky my bro some how it will detamine
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896603609)
Dropped the move.
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896603609)
Dropped the move.
💬 vasild commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896604403)
done, also suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1896604403)
done, also suggested in https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410
💬 maflcko commented on pull request "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)":
(https://github.com/bitcoin/bitcoin/pull/31468#issuecomment-2560956747)
> I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.
For fun, I had written another async alternative that reads the next block while the current block is submitted, which was faster than this pull by a few seconds, but I think trivial code is more important in tests than the fastest solution.
(https://github.com/bitcoin/bitcoin/pull/31468#issuecomment-2560956747)
> I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.
For fun, I had written another async alternative that reads the next block while the current block is submitted, which was faster than this pull by a few seconds, but I think trivial code is more important in tests than the fastest solution.
💬 maflcko commented on issue "Stale code (that has no effect)":
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560984394)
Removing the code yields a test failure for negative feerates for me:
```
test/amount_tests.cpp(64): error: in "amount_tests/GetFeeTest": check feeRate.GetFee(8) == CAmount(-1) has failed [0 != -1]
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
```
I've documented that in fa2da2cb607ba359231fccc9635abe7c8616de56 .
Though, I find it a bit odd to treat negative and positives feerates differently. So the code should probably still be removed and the ceil shou
...
(https://github.com/bitcoin/bitcoin/issues/31558#issuecomment-2560984394)
Removing the code yields a test failure for negative feerates for me:
```
test/amount_tests.cpp(64): error: in "amount_tests/GetFeeTest": check feeRate.GetFee(8) == CAmount(-1) has failed [0 != -1]
*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
```
I've documented that in fa2da2cb607ba359231fccc9635abe7c8616de56 .
Though, I find it a bit odd to treat negative and positives feerates differently. So the code should probably still be removed and the ceil shou
...
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2561017841)
`d8cd80e...48843ec`: `s/=/!=/` :facepalm:
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2561017841)
`d8cd80e...48843ec`: `s/=/!=/` :facepalm:
💬 maflcko commented on issue "Please provide 32 bits builds again":
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561026041)
The 32-bit Windows builds were removed in 2019, which is now a few years ago. Since then, I think you are the second person to ask for the Windows 32-bit build to be added back. Given such a low demand and given that Windows 10 will start to go EOL next year, I don't think there is any value in the effort of trying to add the build back in. (In fact, testing could only be done manually on a Windows 32-bit machine, which again had no interest from anyone volunteering to do, given the nonexistent/
...
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561026041)
The 32-bit Windows builds were removed in 2019, which is now a few years ago. Since then, I think you are the second person to ask for the Windows 32-bit build to be added back. Given such a low demand and given that Windows 10 will start to go EOL next year, I don't think there is any value in the effort of trying to add the build back in. (In fact, testing could only be done manually on a Windows 32-bit machine, which again had no interest from anyone volunteering to do, given the nonexistent/
...
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896679919)
Given that you no longer kill tcpdump, the code should work on all CI tasks, except for the GHA macos tasks?
If there are only a few tasks that do not work, it may be better to just carve them out, instead of enumerating all the ones that work or dealing with code to ensure that at least one works for each config (fuzz/tidy/tests)?
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896679919)
Given that you no longer kill tcpdump, the code should work on all CI tasks, except for the GHA macos tasks?
If there are only a few tasks that do not work, it may be better to just carve them out, instead of enumerating all the ones that work or dealing with code to ensure that at least one works for each config (fuzz/tidy/tests)?
🤔 rkrux reviewed a pull request: "descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404#pullrequestreview-2521878191)
make, functional tests successful.
(https://github.com/bitcoin/bitcoin/pull/31404#pullrequestreview-2521878191)
make, functional tests successful.
💬 rkrux commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896687544)
```
assert_equal('multisig', rpc_result['type'])
```
Nit: This assertion can also be added. I modified the test case to have 21 keys, and the `type` changed to `nonstandard`, which is correct. `p2sh` and `desc` values are present in that output.
Do you think it's worth adding the test case with 21 pubkeys as well to cover the `nonstandard` scenario?
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896687544)
```
assert_equal('multisig', rpc_result['type'])
```
Nit: This assertion can also be added. I modified the test case to have 21 keys, and the `type` changed to `nonstandard`, which is correct. `p2sh` and `desc` values are present in that output.
Do you think it's worth adding the test case with 21 pubkeys as well to cover the `nonstandard` scenario?
💬 rkrux commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896688596)
+1 for this doc change.
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896688596)
+1 for this doc change.
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896697274)
Done! Much better.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896697274)
Done! Much better.
💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896713349)
The table in the OP describes each task and its status wrt to this. There are 18 tasks. It is supposed to work on the top 5+4=9 tasks. The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.
The intention here is to have at least one task that runs this. But it will not hurt to enforce it on more. Do you want to add `CI_FAIL_IF_NO_TCPDUMP_FILE=1` to more tasks? Or reverse the logic to `C
...
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896713349)
The table in the OP describes each task and its status wrt to this. There are 18 tasks. It is supposed to work on the top 5+4=9 tasks. The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.
The intention here is to have at least one task that runs this. But it will not hurt to enforce it on more. Do you want to add `CI_FAIL_IF_NO_TCPDUMP_FILE=1` to more tasks? Or reverse the logic to `C
...
💬 brunoerg commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896714825)
I'm not sure about this. I think `Assume(ctx != ParseScriptContext::P2WPKH)` is shorter and easier to understand.
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896714825)
I'm not sure about this. I think `Assume(ctx != ParseScriptContext::P2WPKH)` is shorter and easier to understand.
💬 maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896719730)
> The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.
If no tests are run, then no exclusion is needed as well. Though, if this is not expected to run on tidy, I wonder why `traffic_monitor_begin "tidy"` exists? This leaves macOS, which can be excluded. The other tasks, which do not use the CI infra anyway, so don't need to be excluded as well.
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896719730)
> The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.
If no tests are run, then no exclusion is needed as well. Though, if this is not expected to run on tidy, I wonder why `traffic_monitor_begin "tidy"` exists? This leaves macOS, which can be excluded. The other tasks, which do not use the CI infra anyway, so don't need to be excluded as well.
💬 ytrezq commented on issue "Please provide 32 bits builds again":
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561085564)
@maflcko except bitcoin 0.17 can’t read the `wallet.dat` file I created with a more recent version of Bitcoin back when I had working 64 bits computer.
**An alternative would be to support 32 bits builds until an import solution come for android apps**.
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561085564)
@maflcko except bitcoin 0.17 can’t read the `wallet.dat` file I created with a more recent version of Bitcoin back when I had working 64 bits computer.
**An alternative would be to support 32 bits builds until an import solution come for android apps**.
⚠️ pinheadmz opened an issue: "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD"
(https://github.com/bitcoin/bitcoin/issues/31561)
Running master (specifically 318b2a2f90dfd8ae20beca58e55e6d240a7f3d27 which is HEAD of #31415) on Ubuntu 24.10 on a digital ocean droplet with 8GB RAM. Crashing regularly during IBD. Logs always end like this:
```
2024-12-23T20:39:07Z [validation] Enqueuing BlockConnected: block hash=000000000000000000008364a7396ba08c5ab141d10136e3ad0d9fa37899d7b0 block height=814565
2024-12-23T20:39:07Z [validation] Enqueuing UpdatedBlockTip: new block hash=000000000000000000008364a7396ba08c5ab141d10136e
...
(https://github.com/bitcoin/bitcoin/issues/31561)
Running master (specifically 318b2a2f90dfd8ae20beca58e55e6d240a7f3d27 which is HEAD of #31415) on Ubuntu 24.10 on a digital ocean droplet with 8GB RAM. Crashing regularly during IBD. Logs always end like this:
```
2024-12-23T20:39:07Z [validation] Enqueuing BlockConnected: block hash=000000000000000000008364a7396ba08c5ab141d10136e3ad0d9fa37899d7b0 block height=814565
2024-12-23T20:39:07Z [validation] Enqueuing UpdatedBlockTip: new block hash=000000000000000000008364a7396ba08c5ab141d10136e
...