💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2671300966)
> Can you add the note about `FUZZ_LIBS` usage to the PR description?

Sure! Added.

💬 janb84 commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2671302403)
tACK [f43b6ba](https://github.com/bitcoin/bitcoin/pull/31907/commits/f43b6ba27690442721623a9fa57b3c59e55d8397)

- Ran unit and functional tests related to assumeutxo (test/functional/feature_assumeutxo.py).
- Ran all the functional tests
- Validated the code change.

I also wrote a [review note](https://github.com/janb84/Bitcoin-Core_review-notes/blob/0f6634af718c2acedba9de74884cb149c2a68e2a/reviews/PR-%2331907.md) on the PR

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1963455462)
I agree.

I've picked that commit from one of @theuni's branch. Perhaps it fell out of the correct context.

💬 fanquake commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2671319664)
> but I don't see how this can be achieved without possibly breaking someones build-setup.

Given we are already breaking all build setups in the CMake switch, it seems like the right time to make any further changes?

It's pretty clear the code here is convoluted/overcomplicated. Just the fact that the release process derives the same version number from two different places is odd (and is what hides the bug here).

👍 ryanofsky approved a pull request: "mining: drop unused -nFees and sigops from CBlockTemplate"
(https://github.com/bitcoin/bitcoin/pull/31897#pullrequestreview-2629670115)
Code review ACK 587ad67bf3bee03a7088e1dd5be2828b7a4f4fd4. Only change since last review is removing another dummy vTxSigOpsCost assignment.

IMO, it would be probably be more ideal to drop the dummy values from the `CBlockTemplate::vTxFees` and `CBlockTemplate::vTxSigOpsCost` vectors. I don't think it matters if they have different indexing than `CBlock::vtx` because that is a different vector in a different class, and it would be confusing for that vector *not* to contain a placeholder transa
...

👍 ryanofsky approved a pull request: "Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods"
(https://github.com/bitcoin/bitcoin/pull/31785#pullrequestreview-2629683943)
Code review ACK 9725cd6a5c20302da3dcae8fc1156458537ce1df. Just minor suggestions implemented since last review: changing comments and swapping current_block / block in waitfornewblock.

💬 BrandonOdiwuor commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1963477222)
fixed the remaining instances

💬 BrandonOdiwuor commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1963480997)
reverted the extraction of `dummy_testshell_file` to avoid global variables

💬 BrandonOdiwuor commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1963484163)
The reason for the extraction was to share the variable with the `reset` function, I have however reverted the extraction to avoid using global variables

💬 BrandonOdiwuor commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#discussion_r1963486075)
I have reverted the extraction, is this still worth removing?

💬 maflcko commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2671378631)
As a workaround, reverting 4b527fa93b9763a33842069bc07446313cbf5e0f could be investigated. See also https://github.com/bitcoin/bitcoin/pull/30193#discussion_r1632994981

💬 BrandonOdiwuor commented on pull request "test: fix TestShell initialization and reset()":
(https://github.com/bitcoin/bitcoin/pull/31415#issuecomment-2671379166)
@l0rinc I have added instructions on how to test on the PR description

The reason for the extraction of the `dummy_testshell_file` initially was to share it with the `reset()` function to fix https://github.com/bitcoin/bitcoin/issues/31131, I have however reverted the change to avoid using global variable

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1963498435)
Unrelated changes have been dropped.

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1963499301)
Thanks! Reworked.

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2671390521)
Addressed the latest @TheCharlatan's feedback.

👍 rkrux approved a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2629756653)
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02

💬 maflcko commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2671429607)
Given that the commit id is hardcoded into the `git archive`, it could make sense to hardcode everything else as well and then just prefer that over any external environment influences. This should allow to drop `BITCOIN_GENBUILD_NO_GIT`, because whatever result it will calculate will be ignored.

In theory, it could be implemented with a small helper script to inject the hardcoded values into the source code (along with the *current* commit id), then create a new "dummy" commit, then call `git
...

💬 laanwj commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2671469502)
> @laanwj did you get a chance to look into this? Else i guess it falls on me to take care of this before deadline..

Whoops. No i didn't, i lost track of the deadline (which, assuming this is considered a bugfix is 2025-03-06, according to #31029), sorry.
i will have a look at it today and tomorrow, if i don't manage to figure out this week, it would be great if you can take it over.

💬 darosior commented on issue "Gracefully handle dropped UPnP support ":
(https://github.com/bitcoin-core/gui/issues/843#issuecomment-2671477287)
Sounds good, thanks for looking into it.

-------- Original Message --------
On 2/20/25 8:17 AM, laanwj wrote:

>> ***@***.***(https://github.com/laanwj) did you get a chance to look into this? Else i guess it falls on me to take care of this before deadline..
>
> Whoops. No i didn't, i lost track of the deadline (which, assuming this is considered a bugfix is 2025-03-06, according to #31029), sorry.
> i will have a look at it today and tomorrow, if i don't manage to figure out this week, it wo
...

💬 maflcko commented on pull request "ci: Switch to gcr.io mirror to avoid rate limits":
(https://github.com/bitcoin/bitcoin/pull/31906#issuecomment-2671483472)
> > An alternative would probably be to configure the docker deamons of the self-hosted runners to use the mirror?
>
> I haven't tested this

I tested it and your suggestion failed locally for me:

```
# podman pull docker.io/ubuntu/24.04
Error: loading drop-in registries configuration "/etc/containers/registries.conf.d/gcr.conf": invalid location 'https://mirror.gcr.io': URI schemes are not supported
# cat /etc/containers/registries.conf.d/gcr.conf
[[registry]]
location = "https://m
...