💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2900903169)
I am out of ideas on how to proceed here. I tried this again on Ubuntu and the gcc-13 failed frequently (https://cirrus-ci.com/task/4846423977492480), whereas the previous commit with gcc-14 passed 100 times (https://cirrus-ci.com/task/5508081775280128)
* I suspect a downgrade from `DEBUG=1` to `-D_GLIBCXX_ASSERTIONS` should also work on centos (c.f. https://cirrus-ci.com/task/5739615073599488)
* Alternatively, we take https://github.com/bitcoin/bitcoin/pull/32529
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2900903169)
I am out of ideas on how to proceed here. I tried this again on Ubuntu and the gcc-13 failed frequently (https://cirrus-ci.com/task/4846423977492480), whereas the previous commit with gcc-14 passed 100 times (https://cirrus-ci.com/task/5508081775280128)
* I suspect a downgrade from `DEBUG=1` to `-D_GLIBCXX_ASSERTIONS` should also work on centos (c.f. https://cirrus-ci.com/task/5739615073599488)
* Alternatively, we take https://github.com/bitcoin/bitcoin/pull/32529
📝 maflcko opened a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586)
to work around https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2900903169
closes #32524
(https://github.com/bitcoin/bitcoin/pull/32586)
to work around https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2900903169
closes #32524
💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102389616)
@hebasto thanks, I see. I will look at using the top commit hash for cache invalidation. If I've understood you correct the current cache key is:
`hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json')`
we think this will be sufficient:
`hashFiles('vcpkg_commit_hash', 'vcpkg.json')`
Which then means we can drop the vswhere tool finding.
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102389616)
@hebasto thanks, I see. I will look at using the top commit hash for cache invalidation. If I've understood you correct the current cache key is:
`hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json')`
we think this will be sufficient:
`hashFiles('vcpkg_commit_hash', 'vcpkg.json')`
Which then means we can drop the vswhere tool finding.
💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102395677)
> I will look at using the top commit hash for cache invalidation. If I've understood you correct the current cache key is:
>
> `hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json')`
>
> we think this will be sufficient:
>
> `hashFiles('vcpkg_commit_hash', 'vcpkg.json')`
Yes, I thin so.
> Which then means we can drop the vswhere tool finding.
Right. However, we still need to locate `mt.exe`.
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2102395677)
> I will look at using the top commit hash for cache invalidation. If I've understood you correct the current cache key is:
>
> `hashFiles('cmake_version', 'msbuild_version', 'toolset_version', 'vcpkg.json')`
>
> we think this will be sufficient:
>
> `hashFiles('vcpkg_commit_hash', 'vcpkg.json')`
Yes, I thin so.
> Which then means we can drop the vswhere tool finding.
Right. However, we still need to locate `mt.exe`.
💬 juanmigdr commented on issue "estimateSmartFee error: "Insufficient data or no feerate found"":
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2900990650)
I am currently experiencing this same issue again on testnet and testnet4
(https://github.com/bitcoin/bitcoin/issues/31116#issuecomment-2900990650)
I am currently experiencing this same issue again on testnet and testnet4
💬 maflcko commented on pull request "depends: hard-code necessary c(xx)flags rather than setting them per-host":
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-2900995568)
> The per-host flag values now represent the mandatory flags that cannot be overridden by the environment. Additionally, these flags (-pipe and -std=xx) will no longer be passed into the CMake build.
Would be nice to explain this with commands to test and observe the difference in behavior. This would make it easier to understand the goals and test them.
(https://github.com/bitcoin/bitcoin/pull/32584#issuecomment-2900995568)
> The per-host flag values now represent the mandatory flags that cannot be overridden by the environment. Additionally, these flags (-pipe and -std=xx) will no longer be passed into the CMake build.
Would be nice to explain this with commands to test and observe the difference in behavior. This would make it easier to understand the goals and test them.
💬 1440000bytes commented on pull request "log: Additional compact block logging":
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2901057685)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32582#issuecomment-2901057685)
Concept ACK
💬 maflcko commented on issue "Support `Accept` HTTP header in REST API":
(https://github.com/bitcoin/bitcoin/issues/32583#issuecomment-2901076206)
Not sure. This means there will be two conflicting? ways to specify the same. I don't see the benefit either.
(https://github.com/bitcoin/bitcoin/issues/32583#issuecomment-2901076206)
Not sure. This means there will be two conflicting? ways to specify the same. I don't see the benefit either.
💬 TheCharlatan commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2901082433)
@fjahr can you rebase this, I'm still interested in this change.
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2901082433)
@fjahr can you rebase this, I'm still interested in this change.
💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901105674)
> Looks like CI failed
Thanks, I had missed that. Seems to have been a silent merge conflict. Kind of weird that only one environment failed but I have come up with an easier way to produce the corrupted db failure case and added some documentation on that as well.
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901105674)
> Looks like CI failed
Thanks, I had missed that. Seems to have been a silent merge conflict. Kind of weird that only one environment failed but I have come up with an easier way to produce the corrupted db failure case and added some documentation on that as well.
👍 laanwj approved a pull request: "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task"
(https://github.com/bitcoin/bitcoin/pull/32586#pullrequestreview-2861096204)
ACK fa079538e32d20aec6786c93e1117da1e8ea0cab
(https://github.com/bitcoin/bitcoin/pull/32586#pullrequestreview-2861096204)
ACK fa079538e32d20aec6786c93e1117da1e8ea0cab
💬 brunoerg commented on pull request "fuzz: doc: add info about `afl-system-config` for macOS":
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2901123554)
> I noticed that a couple lines above, the link to "selecting the best AFL compiler..." is invalid and has instead moved to https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#a-selecting-the-best-afl-compiler-for-instrumenting-the-target. I can open a PR to fix the doc link.
Cool, go ahead.
(https://github.com/bitcoin/bitcoin/pull/32175#issuecomment-2901123554)
> I noticed that a couple lines above, the link to "selecting the best AFL compiler..." is invalid and has instead moved to https://github.com/AFLplusplus/AFLplusplus/blob/stable/docs/fuzzing_in_depth.md#a-selecting-the-best-afl-compiler-for-instrumenting-the-target. I can open a PR to fix the doc link.
Cool, go ahead.
👋 glozow's pull request is ready for review: "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs"
(https://github.com/bitcoin/bitcoin/pull/31829)
(https://github.com/bitcoin/bitcoin/pull/31829)
💬 fanquake commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901167009)
Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901167009)
Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
💬 maflcko commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901167688)
> Kind of weird that only one environment failed
There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901167688)
> Kind of weird that only one environment failed
There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2901169323)
This is ready for review again. Main changes from February: it includes the rewrite as a boost::multi_index container, removes `-maxorphantxs` entirely, and drops the benches that weren't demonstrating anything. I've updated the PR description.
Rewrite and eviction changes are in the same commit. I didn't think it made sense to first reimplement the old design as a multi_index, because the old eviction strategy requires twice as many indexes. If you are familiar with the old design and just w
...
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-2901169323)
This is ready for review again. Main changes from February: it includes the rewrite as a boost::multi_index container, removes `-maxorphantxs` entirely, and drops the benches that weren't demonstrating anything. I've updated the PR description.
Rewrite and eviction changes are in the same commit. I didn't think it made sense to first reimplement the old design as a multi_index, because the old eviction strategy requires twice as many indexes. If you are familiar with the old design and just w
...
💬 maflcko commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901183220)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
Of course there is no right or wrong answer here, but I'd say that the two libc++ debug builds are probably enough and redundant with this one anyway, so one could even fully remove the debug setting here. (Also, there is other, possibly more important stuff, only run in nightly CIs outside this rep
...
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2901183220)
> Not sure; now we wouldn't have any (non-msan, non-32 bit task) CI using `DEBUG=1`? It'd at least be good to note in the CI config, why this is being changed this way / when it could be removed.
Of course there is no right or wrong answer here, but I'd say that the two libc++ debug builds are probably enough and redundant with this one anyway, so one could even fully remove the debug setting here. (Also, there is other, possibly more important stuff, only run in nightly CIs outside this rep
...
💬 hebasto commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2102535478)
A new https://git.guix.gnu.org/guix.git seems up.
(https://github.com/bitcoin/bitcoin/pull/32439#discussion_r2102535478)
A new https://git.guix.gnu.org/guix.git seems up.
💬 fjahr commented on pull request "test: properly check for per-tx sigops limit":
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2901192518)
tACK 7bc64a8
Confirmed that the test fails if the limit is not exceeded.
(https://github.com/bitcoin/bitcoin/pull/32533#issuecomment-2901192518)
tACK 7bc64a8
Confirmed that the test fails if the limit is not exceeded.
💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901197602)
> > Kind of weird that only one environment failed
>
> There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.
Good to know, thanks!
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2901197602)
> > Kind of weird that only one environment failed
>
> There are not enough CI resources to re-run every task on every pull request every few days. The CI resources would have to be ~doubled for that. For now, only the lint and previous-releases task are re-run.
Good to know, thanks!