✅ fanquake closed an issue: "cmake: searching across directories for config files"
(https://github.com/bitcoin/bitcoin/issues/32938)
(https://github.com/bitcoin/bitcoin/issues/32938)
🚀 fanquake merged a pull request: "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`"
(https://github.com/bitcoin/bitcoin/pull/32943)
(https://github.com/bitcoin/bitcoin/pull/32943)
💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3069321057)
reACK 3afc73134bf652d01e25232701c2ef192b3fea0b
just a reminder that you don't need to rebase on master unless a silent merge conflict is detected or drahtbot yells at you about an issue merging
`git range-diff master 4ddc577953a7407ea44a3e3d8d9a4635929307ca 3afc73134bf652d01e25232701c2ef192b3fea0b`
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3069321057)
reACK 3afc73134bf652d01e25232701c2ef192b3fea0b
just a reminder that you don't need to rebase on master unless a silent merge conflict is detected or drahtbot yells at you about an issue merging
`git range-diff master 4ddc577953a7407ea44a3e3d8d9a4635929307ca 3afc73134bf652d01e25232701c2ef192b3fea0b`
💬 fanquake commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3069329852)
Backported to 29.x in #32863.
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3069329852)
Backported to 29.x in #32863.
💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069344912)
> How would you fix it, given IWYU is giving different recommendations, based on the architecture?
Easiest way is to use `// IWYU pragma: keep` when IWYU incorrectly tells you to remove an include that should be kept and `// IWYU pragma: no_include` when it tells you to add an include that shouldn't be added. The pragmas will suppress errors on whatever platform is complaining and be ignored on other platforms. I dealt with this recently in https://github.com/bitcoin-core/libmultiprocess/comm
...
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069344912)
> How would you fix it, given IWYU is giving different recommendations, based on the architecture?
Easiest way is to use `// IWYU pragma: keep` when IWYU incorrectly tells you to remove an include that should be kept and `// IWYU pragma: no_include` when it tells you to add an include that shouldn't be added. The pragmas will suppress errors on whatever platform is complaining and be ignored on other platforms. I dealt with this recently in https://github.com/bitcoin-core/libmultiprocess/comm
...
📝 fanquake opened a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/32969)
Backports:
* #32943
(https://github.com/bitcoin/bitcoin/pull/32969)
Backports:
* #32943
💬 fanquake commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3069385247)
Backported to 28.x in #32969.
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3069385247)
Backported to 28.x in #32969.
📝 maflcko opened a pull request: "ci: Enable more shellcheck"
(https://github.com/bitcoin/bitcoin/pull/32970)
shellcheck is often the main "reviewer" of CI code written in Bash, so it seems odd to disable it by putting commands into `bash -c "cmd..."`.
Fix that by removing `bash -c`, where it isn't intended.
(https://github.com/bitcoin/bitcoin/pull/32970)
shellcheck is often the main "reviewer" of CI code written in Bash, so it seems odd to disable it by putting commands into `bash -c "cmd..."`.
Fix that by removing `bash -c`, where it isn't intended.
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069459525)
> `// IWYU pragma: no_include` when it tells you to add an include that shouldn't be added. The pragmas will suppress errors on whatever platform is complaining and be ignored on other platforms.
I wouldn't recommend to do this, because this can lead to missing includes, as also mentioned earlier in https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2125793574. The correct fix is to just add the includes iwyu is asking for.
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069459525)
> `// IWYU pragma: no_include` when it tells you to add an include that shouldn't be added. The pragmas will suppress errors on whatever platform is complaining and be ignored on other platforms.
I wouldn't recommend to do this, because this can lead to missing includes, as also mentioned earlier in https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2125793574. The correct fix is to just add the includes iwyu is asking for.
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3069532178)
@User087 There are a few differences from the feature you linked. From the same guide, [the very last section](https://github.com/monero-project/monero/blob/master/docs/ANONYMITY_NETWORKS.md#i2ptor-stream-used-twice):
> I2P/Tor Stream Used Twice
`monerod` currently selects two outgoing connections every 5 minutes for transmitting transactions over I2P/Tor.
The proposed feature here does not use any existing connections for transmitting transactions. It creates a new connection to a random p
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3069532178)
@User087 There are a few differences from the feature you linked. From the same guide, [the very last section](https://github.com/monero-project/monero/blob/master/docs/ANONYMITY_NETWORKS.md#i2ptor-stream-used-twice):
> I2P/Tor Stream Used Twice
`monerod` currently selects two outgoing connections every 5 minutes for transmitting transactions over I2P/Tor.
The proposed feature here does not use any existing connections for transmitting transactions. It creates a new connection to a random p
...
💬 ryanofsky commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069550613)
> The correct fix is to just add the includes iwyu is asking for.
I think this is about cases where IWYU on one platform asks you to add (or remove) an include, and IWYU on other platform asks you to remove (or add) the same include. A reasonable way to resolve these is to decide which IWYU is correct and add pragmas to silence the other IWYU. Other ways to resolve it could be to define mappings, or submit fixes upstream, or choose a single platform to be your canonical IWYU platform and igno
...
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069550613)
> The correct fix is to just add the includes iwyu is asking for.
I think this is about cases where IWYU on one platform asks you to add (or remove) an include, and IWYU on other platform asks you to remove (or add) the same include. A reasonable way to resolve these is to decide which IWYU is correct and add pragmas to silence the other IWYU. Other ways to resolve it could be to define mappings, or submit fixes upstream, or choose a single platform to be your canonical IWYU platform and igno
...
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069564494)
Yes, the keep pragma seems useful, but I personally wouldn't recommend using the no_include pragma in this context
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069564494)
Yes, the keep pragma seems useful, but I personally wouldn't recommend using the no_include pragma in this context
💬 Sjors commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3069566904)
Better to point to #6722 as it has the discussion from back then.
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3069566904)
Better to point to #6722 as it has the discussion from back then.
💬 stratospher commented on pull request "test: fix intermittent failure in rpc_invalidateblock.py":
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2204928129)
yeah makes sense, I guess this was the reason some of the test was moved up in https://github.com/bitcoin/bitcoin/pull/30479#issuecomment-2892651335
(https://github.com/bitcoin/bitcoin/pull/32968#discussion_r2204928129)
yeah makes sense, I guess this was the reason some of the test was moved up in https://github.com/bitcoin/bitcoin/pull/30479#issuecomment-2892651335
🤔 instagibbs reviewed a pull request: "cluster mempool: add TxGraph work controls"
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3016273688)
reviewed via `git range-diff master 18e40b6cd5a11e23b7a0654bdd12f08e610ce980 4788c87f7c119f35c551da05e7c43bd5be51420c`
(https://github.com/bitcoin/bitcoin/pull/32263#pullrequestreview-3016273688)
reviewed via `git range-diff master 18e40b6cd5a11e23b7a0654bdd12f08e610ce980 4788c87f7c119f35c551da05e7c43bd5be51420c`
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204933624)
It will not "expend" iterations on oversized levels, so it can return "true" when doing no work.
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204933624)
It will not "expend" iterations on oversized levels, so it can return "true" when doing no work.
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204876202)
s/quality/QualityLevel/
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204876202)
s/quality/QualityLevel/
💬 instagibbs commented on pull request "cluster mempool: add TxGraph work controls":
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204911282)
The new logic means that you may not have enough to make it considered `ACCEPTABLE` BUT you may end up with `OPTIMAL` anyways and are able to continue until you hit a difficult cluster. In practice this may end up with a lot more clusters known as optimal when their topology is simple.
Might be worth a comment on the "if (!improved) return false;" line
(https://github.com/bitcoin/bitcoin/pull/32263#discussion_r2204911282)
The new logic means that you may not have enough to make it considered `ACCEPTABLE` BUT you may end up with `OPTIMAL` anyways and are able to continue until you hit a difficult cluster. In practice this may end up with a lot more clusters known as optimal when their topology is simple.
Might be worth a comment on the "if (!improved) return false;" line
💬 maflcko commented on issue "ci: don't pass GIT_EXEC_PATH inside test container":
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3069728608)
Maybe an easy fix could be to not pass any env vars, except those with a key specified in `ci/test/00_setup_env.sh`?
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3069728608)
Maybe an easy fix could be to not pass any env vars, except those with a key specified in `ci/test/00_setup_env.sh`?
🤔 instagibbs reviewed a pull request: "p2p: improve TxOrphanage denial of service bounds"
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3016494838)
reviewed through 790f6e7a72d31988c5be9f2d520a04d0c0edfa09
(https://github.com/bitcoin/bitcoin/pull/31829#pullrequestreview-3016494838)
reviewed through 790f6e7a72d31988c5be9f2d520a04d0c0edfa09