💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766)
So far it looks like this can only happen with gcc-13 (or older) *and* GLIBCXX debug mode. I don't think any real user is running in production with the std lib debug mode, so an alternative would be to just fixup the CI?
For reference, the end-user setting `-D_GLIBCXX_ASSERTIONS` passed 25 times: https://cirrus-ci.com/task/5739615073599488
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766)
So far it looks like this can only happen with gcc-13 (or older) *and* GLIBCXX debug mode. I don't think any real user is running in production with the std lib debug mode, so an alternative would be to just fixup the CI?
For reference, the end-user setting `-D_GLIBCXX_ASSERTIONS` passed 25 times: https://cirrus-ci.com/task/5739615073599488
💬 laanwj commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890480867)
> so an alternative would be to just fixup the CI?
Sure. If this is unlikely to cause issues for real users, that makes more sense, the point of the CI is not to keep us patching the code to make the CI happy 😄
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890480867)
> so an alternative would be to just fixup the CI?
Sure. If this is unlikely to cause issues for real users, that makes more sense, the point of the CI is not to keep us patching the code to make the CI happy 😄
📝 maflcko opened a pull request: "ci: Move DEBUG=1 to centos task"
(https://github.com/bitcoin/bitcoin/pull/32560)
The glibcxx debug mode has many bugs in prior gcc releases:
* https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766
* https://github.com/bitcoin/bitcoin/issues/31436#issuecomment-2530717875
* ...
Instead of working around all of them, just use the existing `ci_native_centos` task with gcc-14 to have it enabled. This also follows the logic of other sanitizers (tsan, asan, ubsan, msan, valgrind, ...) to generally prefer the latest version of the sanitizer for the latests
...
(https://github.com/bitcoin/bitcoin/pull/32560)
The glibcxx debug mode has many bugs in prior gcc releases:
* https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890411766
* https://github.com/bitcoin/bitcoin/issues/31436#issuecomment-2530717875
* ...
Instead of working around all of them, just use the existing `ci_native_centos` task with gcc-14 to have it enabled. This also follows the logic of other sanitizers (tsan, asan, ubsan, msan, valgrind, ...) to generally prefer the latest version of the sanitizer for the latests
...
💬 fanquake commented on issue "intermittent issue in qt addressBookTests: (table_view->model()->rowCount()): [3 != 2] Loc: [./qt/test/addressbooktests.cpp(183)]":
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890508864)
https://github.com/bitcoin/bitcoin/runs/42411027184
(https://github.com/bitcoin/bitcoin/issues/32558#issuecomment-2890508864)
https://github.com/bitcoin/bitcoin/runs/42411027184
📝 maflcko opened a pull request: "doc: Adjust stale MSVC bug url"
(https://github.com/bitcoin/bitcoin/pull/32561)
The old url is stale, so use the current one. See https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2889188342
(https://github.com/bitcoin/bitcoin/pull/32561)
The old url is stale, so use the current one. See https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2889188342
💬 maflcko commented on pull request "refactor: Remove workaround for resolved MSVC bug":
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2890510445)
> > It would be good to adjust the URL to avoid confusion and duplicate work and pull requests
>
> Sure. I'll wait until it's been properly triaged.
See https://github.com/bitcoin/bitcoin/pull/32561
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2890510445)
> > It would be good to adjust the URL to avoid confusion and duplicate work and pull requests
>
> Sure. I'll wait until it's been properly triaged.
See https://github.com/bitcoin/bitcoin/pull/32561
💬 maflcko commented on pull request "refactor: Remove workaround for resolved MSVC bug":
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2890515265)
If contributors here have a microsoft account, they may want to upvote the issue report, so that microsoft is aware that there is some interest in having it fixed.
(https://github.com/bitcoin/bitcoin/pull/32552#issuecomment-2890515265)
If contributors here have a microsoft account, they may want to upvote the issue report, so that microsoft is aware that there is some interest in having it fixed.
👍 theStack approved a pull request: "signet: omit commitment for some trivial challenges"
(https://github.com/bitcoin/bitcoin/pull/29032#pullrequestreview-2850301319)
re-ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8
(as per `$ git range-diff 2be93f7b...6ee32aaa`)
(https://github.com/bitcoin/bitcoin/pull/29032#pullrequestreview-2850301319)
re-ACK 6ee32aaaca4a46d42594bc16479868c76cc67fd8
(as per `$ git range-diff 2be93f7b...6ee32aaa`)
✅ laanwj closed a pull request: "subprocess: replace `fs::directory_iterator` with `readdir`"
(https://github.com/bitcoin/bitcoin/pull/32529)
(https://github.com/bitcoin/bitcoin/pull/32529)
💬 laanwj commented on pull request "subprocess: replace `fs::directory_iterator` with `readdir`":
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2890559685)
This isn't necessary if it's worked around some other way https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890480867
(https://github.com/bitcoin/bitcoin/pull/32529#issuecomment-2890559685)
This isn't necessary if it's worked around some other way https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890480867
💬 crStiv commented on pull request "docs: clarify RPC credentials security boundary":
(https://github.com/bitcoin/bitcoin/pull/32424#issuecomment-2890561971)
> Seems to contradict the existence of `-rpcwhitelist`
@luke-jr now seems to be better, what are your thoughts?
(https://github.com/bitcoin/bitcoin/pull/32424#issuecomment-2890561971)
> Seems to contradict the existence of `-rpcwhitelist`
@luke-jr now seems to be better, what are your thoughts?
🤔 hebasto reviewed a pull request: "ci: Move DEBUG=1 to centos task"
(https://github.com/bitcoin/bitcoin/pull/32560#pullrequestreview-2850329125)
Changes look correct. Waiting for CI.
(https://github.com/bitcoin/bitcoin/pull/32560#pullrequestreview-2850329125)
Changes look correct. Waiting for CI.
👍 hebasto approved a pull request: "doc: Adjust stale MSVC bug url"
(https://github.com/bitcoin/bitcoin/pull/32561#pullrequestreview-2850334992)
ACK fa330a5e38a8e9937778eeb53f06390d943bc42c.
(https://github.com/bitcoin/bitcoin/pull/32561#pullrequestreview-2850334992)
ACK fa330a5e38a8e9937778eeb53f06390d943bc42c.
🚀 hebasto merged a pull request: "doc: Adjust stale MSVC bug url"
(https://github.com/bitcoin/bitcoin/pull/32561)
(https://github.com/bitcoin/bitcoin/pull/32561)
💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout)":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890582680)
I'd say finding compiler bugs is a valid use-case of the CI, but this one seems to be fixed already, so there is probably not much value in bisecting the exact gcc commit, especially given that it would be extremely tedious without a minimal non-intermittent reproducer.
(The time would be better spent on reducing other compiler bugs, such as https://github.com/bitcoin/bitcoin/issues/32276#issuecomment-2807267578)
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2890582680)
I'd say finding compiler bugs is a valid use-case of the CI, but this one seems to be fixed already, so there is probably not much value in bisecting the exact gcc commit, especially given that it would be extremely tedious without a minimal non-intermittent reproducer.
(The time would be better spent on reducing other compiler bugs, such as https://github.com/bitcoin/bitcoin/issues/32276#issuecomment-2807267578)
💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2095447036)
Why `libtool`?
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2095447036)
Why `libtool`?
💬 hebasto commented on pull request "doc: add alpine build instructions":
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2095449704)
I'd add `pkgconf`.
(https://github.com/bitcoin/bitcoin/pull/32559#discussion_r2095449704)
I'd add `pkgconf`.
🤔 janb84 reviewed a pull request: "wallet: Fix logging of wallet version"
(https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2850352360)
ACK https://github.com/bitcoin/bitcoin/commit/4b2cd0b41ff4800c8801f2c44883eaec60a035fa
Fixes regression, improves code readability, adds a test to check if the wallet file version is correctly logged.
- Code review ✅
- Build & run tests ✅
- Ran the added tests against master (fails as expected):
```
wallet_createwallet.py | ✖ Failed | 3 s
wallet_createwallet.py --usecli | ✖ Failed | 3 s
```
- looked into #26021
(https://github.com/bitcoin/bitcoin/pull/32553#pullrequestreview-2850352360)
ACK https://github.com/bitcoin/bitcoin/commit/4b2cd0b41ff4800c8801f2c44883eaec60a035fa
Fixes regression, improves code readability, adds a test to check if the wallet file version is correctly logged.
- Code review ✅
- Build & run tests ✅
- Ran the added tests against master (fails as expected):
```
wallet_createwallet.py | ✖ Failed | 3 s
wallet_createwallet.py --usecli | ✖ Failed | 3 s
```
- looked into #26021
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2095459011)
`MAX_TX_LEGACY_SIGOPS`?
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2095459011)
`MAX_TX_LEGACY_SIGOPS`?
💬 Sjors commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2095462982)
`GetSigOpCount(/*fAccurate=*/true)`
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2095462982)
`GetSigOpCount(/*fAccurate=*/true)`
💬 hebasto commented on pull request "Set minimum supported Windows version to 1903 (May 2019 Update)":
(https://github.com/bitcoin/bitcoin/pull/32537#issuecomment-2890611969)
Friendly ping @laanwj @davidgumberg @hodlinator @sipsorcery :)
(https://github.com/bitcoin/bitcoin/pull/32537#issuecomment-2890611969)
Friendly ping @laanwj @davidgumberg @hodlinator @sipsorcery :)