💬 hebasto commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298378816)
Same change in `cirrus.sh`?

💬 MarcoFalke commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298380304)
In theory yes, but in practise, no, because:

* Cirrus already runs on amd64, so this won't change anything.
* The config will be removed in another pull, so changing it will cause merge conflicts.

📝 MarcoFalke opened a pull request: "ci: Add missing ${CI_RETRY_EXE} before curl"
(https://github.com/bitcoin/bitcoin/pull/28296)
GitHub is frequently down and this is causing many intermittent issues. For example, from today: https://cirrus-ci.com/task/5740122163904512?logs=ci#L398


Try to fix it with a retry.

💬 hebasto commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298383590)
Does the following log:
```
#3 [internal] load metadata for docker.io/library/ubuntu:jammy
```
look OK?

👍 hebasto approved a pull request: "ci: Add missing ${CI_RETRY_EXE} before curl"
(https://github.com/bitcoin/bitcoin/pull/28296#pullrequestreview-1584467464)
ACK fa968ef6a32bb66ca9fa3b84807f1c960f6a9833

💬 MarcoFalke commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298389926)
Yes, the log looks green

💬 MarcoFalke commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#issuecomment-1683842523)
Force pushed to do the same for android

📝 Sjors converted_to_draft a pull request: "getblocktemplate improvements for segwit and sigops"
(https://github.com/bitcoin/bitcoin/pull/27433)
**Sigops**

Two recent F2Pool blocks violated the sigops limit. Both by 3.

I suspect they were not using `getblocktemplate`. If you look at the [template](https://miningpool.observer/template-and-block/00000000000000000004e0ec4f27bd3347381e8e19ed98d7f918e8c1c292ae97) right before the valid block at the same height, which was produced two minutes later, you'll see that it matches the block with 3 small transactions difference. In other words, the valid block producer likely did use `getblock
...

💬 hebasto commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298400895)
I was expecting to see:
```
#3 [internal] load metadata for docker.io/amd64/ubuntu:22.04
```

The PR description claims that this is a bugfix, but changes do not affect CI builds at all, according to the logs.

I'm confused a bit about that.

💬 MarcoFalke commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298404126)
It is possible to run the CI locally. For example, on a local `riscv64` machine you can type:

```
MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh
```

And then observe the failure.

The failure is fixed by this patch.

💬 darosior commented on pull request "Wallet: estimate the size of signed inputs using descriptors":
(https://github.com/bitcoin/bitcoin/pull/26567#issuecomment-1683861802)
Rebased after #28244.

💬 hebasto commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#issuecomment-1683868538)
> Currently the task will fail if run on non-`x86_64`.

I expect it still fail with this PR with a non-`x86_64` Cirrus worker.

💬 hebasto commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298411691)
> Currently the task will fail if run on non-`x86_64`.

I expect it still fail with this PR with a non-`x86_64` Cirrus worker.

💬 MarcoFalke commented on pull request "ci: Add missing amd64 to win64-cross task":
(https://github.com/bitcoin/bitcoin/pull/28295#discussion_r1298414958)
> I expect it still fail with this PR with a non-`x86_64` Cirrus worker.

No, it shouldn't, unless the switch is done incorrectly. If you switch to a non-`x86_64` Cirrus-hosted worker (you shouldn't do that), you should also change the docker label to match this one at the same time.

💬 fanquake commented on pull request "refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate":
(https://github.com/bitcoin/bitcoin/pull/28218#issuecomment-1683882791)
This will need to be rebased after #27460:
```bash
CXX rpc/libbitcoin_node_a-signmessage.o
CXX rpc/libbitcoin_node_a-txoutproof.o
rpc/mempool.cpp:757:28: error: no member named 'IsInitialBlockDownload' in 'Chainstate'
if (chainstate.IsInitialBlockDownload()) {
~~~~~~~~~~ ^
1 error generated.
gmake[2]: *** [Makefile:10829: rpc/libbitcoin_node_a-mempool.o] Error 1
```

🚀 fanquake merged a pull request: "ci: Add missing ${CI_RETRY_EXE} before curl"
(https://github.com/bitcoin/bitcoin/pull/28296)

💬 brunoerg commented on pull request "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation":
(https://github.com/bitcoin/bitcoin/pull/28287#discussion_r1298424645)
It works. But in the case it would be `target=node1.sendmsgtopeer` instead of `target=node1.send_message`.

👍 MarcoFalke approved a pull request: "refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate"
(https://github.com/bitcoin/bitcoin/pull/28218#pullrequestreview-1584529348)
ACK ebda557e5ebe7f754984a34ddfd33d2540c0303b9 🦈

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK ebda557e5ebe7f754984a34dd
...

💬 MarcoFalke commented on pull request "refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1298425994)
Same?

💬 MarcoFalke commented on pull request "refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1298425415)
Can use `auto` to avoid using the same type twice on the same line?
```suggestion
auto& chainman = static_cast<TestChainstateManager&>(*g_setup->m_node.chainman);
```

💬 MarcoFalke commented on pull request "refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate":
(https://github.com/bitcoin/bitcoin/pull/28218#discussion_r1298425581)
Same?