📝 MarcoFalke opened a pull request: "ci: Invalidate Cirrus CI docker cache"
(https://github.com/bitcoin/bitcoin/pull/27838)
Currently the Cirrus CI seems to fail for some reason. No idea why, but maybe invalidating the Docker image cache fixes it?

The failure is:

```
Failed to start an instance! Failed to pull null image! Repository does not exist or may require authentication.
Container errored with 'ImagePullBackOff: Back-off pulling image "gcr.io/cirrus-ci-community/bitcoin/bitcoin/ci/test_imagefile:b3e086572130d8954f84bb90778d02e2cfbb6dc624c01e2f74ee17335a9c453e"'
```

https://cirrus-ci.com/tas
...

💬 ismaelsadeeq commented on pull request "ci: Invalidate Cirrus CI docker cache":
(https://github.com/bitcoin/bitcoin/pull/27838#issuecomment-1582140841)
Concept ACK

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1582165674)

> I'd really prefer we have a regtest-only option to skip the check to impact integration tests the least.

I added a support for skipping the check only on regtest in 2fec7f362d8540cda5ff7e3e4dfb2cb751364e06 by passing -acceptstaleestimates option and it's tested in dea3accb1f2eeef74f293262c168d68c0ec444cb

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1582194359)
`35fc849412...bedbdf4a15`: address https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219413577

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1222705608)
Done. Now it is the same as in `process_message.cpp`.

It seems inefficient to me - if `LIMIT_TO_MESSAGE_TYPE` is set to e.g. `filterclear` the fuzzer has to brute force all possible strings with length 11 to find it? That is 2⁸⁸ possibilities.

🤔 glozow reviewed a pull request: "ci: Nuke Android APK task, Use credits for tsan"
(https://github.com/bitcoin/bitcoin/pull/27834#pullrequestreview-1469402645)
ACK fa22538e481fa2c4f0b5d6f91166335e60b67fe9
Agree with removing this task (which doesn't run any tests) to make room for a more useful task (which does run tests).

💬 MarcoFalke commented on pull request "ci: Nuke Android APK task, Use credits for tsan":
(https://github.com/bitcoin/bitcoin/pull/27834#issuecomment-1582213817)
I think it should be fine to re-add this the next time it finds an issue, or when the (unit) tests are run, or some other reason appears to re-add it.

💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1222713396)
A modern fuzz engine will read the `LIMIT_TO_MESSAGE_TYPE` and inject it into the fuzz input, so it shouldn't take more than a few seconds to guess. In any case it shouldn't take 2^88 tries

💬 MarcoFalke commented on pull request "ci: Invalidate Cirrus CI docker cache":
(https://github.com/bitcoin/bitcoin/pull/27838#issuecomment-1582219284)
I think it is fine to merge this? Seems like it fixed the issue and there is no risk or downside, right?

💬 achow101 commented on pull request "ci: Invalidate Cirrus CI docker cache":
(https://github.com/bitcoin/bitcoin/pull/27838#issuecomment-1582231514)
ACK fac7f4ab5e93e2cd1469dfcda215c7e20d9fe1fb

CI seems to be working here

🚀 achow101 merged a pull request: "ci: Invalidate Cirrus CI docker cache"
(https://github.com/bitcoin/bitcoin/pull/27838)

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1222736555)
We won't be able to get the private key since we don't know the descriptor id in order to do the lookup.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1222737605)
I don't think that's necessary as wallet such as ones without private keys or those with only imports won't have a global hd key.

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1222738220)
This follows the same pattern for the other upgrading methods.

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222738052)
2fec7f362d8540cda5ff7e3e4dfb2cb751364e06

this should be const

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222736738)
in 2fec7f362d8540cda5ff7e3e4dfb2cb751364e06
Please also include here (1) the default value (2) how old "stale" is specifically, using the constants.

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222741414)
nit in dea3accb1f2eeef74f293262c168d68c0ec444cb

maybe replace this `hour` and the one in the other tests with a constant at the top of the file, `SECONDS_PER_HOUR`

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222723313)
in 2fec7f362d8540cda5ff7e3e4dfb2cb751364e06:

prefix with `DEFAULT_`, make doxygen compatible, use braced initialization. Documentation about where the option can be used belongs where that code lives, not here.

```suggestion
/** Whether we allow importing a fee_estimates file older than MAX_FILE_AGE. */
static constexpr bool DEFAULT_ACCEPT_STALE_FEE_ESTIMATES{false};
```

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222742962)
in dea3accb1f2eeef74f293262c168d68c0ec444cb

Given that you comment "there are no blocks" you should probably also assert that is true, e.g. `assert_equal` the result from `getbestblockhash` before and after

🤔 glozow reviewed a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1469429394)
Looks pretty good, thanks for adding the option.
Please clean up the commit messages. dea3accb1f2eeef74f293262c168d68c0ec444cb claims to add the regtest option, but that is done in 2fec7f362d8540cda5ff7e3e4dfb2cb751364e06.