✅ maflcko closed a pull request: "Update .style.yapf"
(https://github.com/bitcoin/bitcoin/pull/33583)

💬 maflcko commented on pull request "refactor: throw `std::string_view` instead of `const char*` in constexpr/consteval functions":
(https://github.com/bitcoin/bitcoin/pull/33569#discussion_r2416218945)
nit: I guess a more minimal one-line temporary(?) workaround would be to just add `&` here? Ref https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381846204

```suggestion
BOOST_CHECK_EXCEPTION(CheckNumFormatSpecifiers<WrongNumArgs>(wrong_fmt), const char*&, HasReason{error});
```

> I don't mind adding such a check, but it seemed to me contrib/devtools/bitcoin-tidy isn't actively developed.

In theory we could add a tidy check for this, but I wonder how much it is worth i
...

💬 maflcko commented on pull request "doc: bump the template macOS version since 14 is now the minimum supported version":
(https://github.com/bitcoin/bitcoin/pull/33573#discussion_r2416237758)
```suggestion
placeholder: e.g. "MacOS 26.0" or "Ubuntu 26.04 LTS"
```

nit: Those are just placeholders, so the value doesn't really matter. Though, if updating them, it may be best to update them in such a way that they won't need to be touched again for the longest time.

I understand that Ubuntu 26.04 doesn't exist yet, but there will be a daily ISO starting next month, which seems good enough.

✅ maflcko closed a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)

💬 maflcko commented on pull request "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`":
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3385146910)
Re-open to trigger the newly added GHA CI tasks

📝 maflcko reopened a pull request: "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator{h,cpp}`"
(https://github.com/bitcoin/bitcoin/pull/33218)
This PR is a simple refactoring that does four things:

1. Renames `test/policy_fee_tests.cpp` to `test/feerounder_tests.cpp`.
2. Renames `policy/fees.{h,cpp}` to `policy/fees/block_policy_estimator.{h,cpp}`.
3. Renames `policy/fees_args.cpp` to `policy/fees/block_policy_estimator_args.cpp`.
4. Modifies `estimateSmartFee` to return the block height at which the estimate was made by adding a `best_height` unsigned int value to the `FeeCalculation` struct.

**Motivation**

In preparation
...

👍 hebasto approved a pull request: "randomenv: Fix MinGW dllimport warning for `environ`"
(https://github.com/bitcoin/bitcoin/pull/33570#pullrequestreview-3318292404)
ACK 97762b1fcf5d389b4f9c06ae46d6408fb78627b0.

📝 MamunC0der opened a pull request: "Upgrade GitHub Action to download-artifact@v5"
(https://github.com/bitcoin/bitcoin/pull/33584)
Release notes:https://github.com/actions/download-artifact/releases/tag/v5.0.0

Change:
uses: actions/download-artifact@v4 -> uses: actions/download-artifact@v5

🤔 ismaelsadeeq reviewed a pull request: "kernel: Introduce initial C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3317819837)
Thanks for the addressing comments quickly and the thread safety clarification.

First Pass

- Overall the commits look solid. I've left a few more comments mostly non-blocking nice to haves.
This PR currently does three things, introduces the C header API, adds a C++ wrapper and tests, updates `bitcoin-chainstate` to use `libbitcoinkernel` wrapper

Maybe split this into two PRs:

- C header API, C++ wrapper here, then bitcoin-chainstate usage of the wrapper, other things can also be added befo
...

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416004410)
In "kernel: Add functions for the block validation state to C header" 43d64194940028071b9f3774f938936d5c6b57d7


nit: this makes sense here, but won't makes sense when you are reading the logs I think, perhaps just remove the "excluding any below reasons" phrase?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416094811)
In "kernel: Add functions to read block from disk to C header" 8653972e2b0c3c698c4fc426ebfd23d44be14983

I think it will be worth indicating the several scenarios that the user should guard here not just a single example.
or provide a one fit all example of how that inconsistency can be prevented.
```suggestion
* chainstate manager, e.g. processing blocks, will change the chain.
```

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416130383)
In "kernel: Add function to read block undo data from disk to C header"

Nice that we are calling this data structure spent outputs 👍🏾

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416058549)
In "kernel: Add block validation to C header" f97a2428fe252c8eea4841487c89ae52ec981833

For some invalid blocks they are not new and we won't set this to 1.

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416281630)
This makes sense, thanks for the explanation :)

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416281267)
Maybe add a comment saying that?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416148293)
In "kernel: Add function to read block undo data from disk to C header" c32db779023c4e0f5384174cb6269ff50f1acc41

Another usecase for the undo data is non-assumevalid swiftsync.

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416281014)
Fair point

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2416282679)
> We previously had similar data structs and managed to eliminate them over time,

Why, it seems okay to me to have those structs ?

👍 hebasto approved a pull request: "Upgrade GitHub Action to download-artifact@v5"
(https://github.com/bitcoin/bitcoin/pull/33584#pullrequestreview-3318331270)
ACK b35341b9ba63a0108596e56e9eecc851a4558d98, I have reviewed the code and it looks OK.

🤔 hebasto reviewed a pull request: "Upgrade GitHub Action to download-artifact@v5"
(https://github.com/bitcoin/bitcoin/pull/33584#pullrequestreview-3318357238)
I think this might be update as well: https://github.com/bitcoin/bitcoin/blob/b510893d00760083ac36948747aa6ebd84656192/.github/actions/configure-docker/action.yml#L22

cc @willcl-ark