💬 fanquake commented on pull request "depends: build zeromq with CMake":
(https://github.com/bitcoin/bitcoin/pull/29723#issuecomment-2237161800)
> is non-deterministic across architectures.

Should be fixed now.

👍 theuni approved a pull request: "depends: build FreeType with CMake"
(https://github.com/bitcoin/bitcoin/pull/29880#pullrequestreview-2186515247)
ACK ff4f3deb7b8adfcc90fb745440ce4be1176552ca

Guix build (x86_64):
```
12c161c6b94ccb15d420fe27f7a812c95247a2614db5f8c6e666e04b5ef0a7d3 aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu-debug.tar.gz
99c6d258715f38c7b56eefd0b08e80552012e8903f26d28225b690f432839545 aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu.tar.gz
bd6c916cc5fdd322bf9e7e3cecbe45a68551c0135d63b35709b32fccde289834 aarch64-linux-gnu/SHA256SUMS.part
18d7fb6f45b585a4313d115ac95f049dfc27b66857571933a3f5851f
...

📝 mzumsande opened a pull request: "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates"
(https://github.com/bitcoin/bitcoin/pull/30479)
When we call `reconsiderblock` for some block,  `Chainstate::ResetBlockFailureFlags` puts the descendants of that block into `setBlockIndexCandidates` (if they meet the criteria, i.e. have more work than the tip etc.), but never put any ancestors into the set even though we do clear their failure flags.

I think that this is wrong, because `setBlockIndexCandidates` should always contain all eligible indexes that have at least as much work as the current tip, which can include ancestors of the
...

💬 mzumsande commented on issue "Assertion `setBlockIndexCandidates.count(pindex)' failed":
(https://github.com/bitcoin/bitcoin/issues/16444#issuecomment-2237175337)
While #16849 fixed inconsistencies in `invalidateblock`, failures continued because similar inconsistencies can also happen during reconsiderblock. See #30479 for an explanation and a fix.

💬 brunoerg commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#discussion_r1683274753)
```suggestion
// We don't care about these types because they are not spendable
```

💬 hebasto commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1683279748)
Please see https://github.com/hebasto/bitcoin/pull/269.

💬 ryanofsky commented on pull request "kernel, refactor: return error status on all fatal errors":
(https://github.com/bitcoin/bitcoin/pull/29700#issuecomment-2237212741)
Rebased ad1e73590d102edf3738986c1382b5e36a0ed727 -> e04e259146238f9a733edf3fd7b192db95f66e71 ([`pr/fatalresult.18`](https://github.com/ryanofsky/bitcoin/commits/pr/fatalresult.18) -> [`pr/fatalresult.19`](https://github.com/ryanofsky/bitcoin/commits/pr/fatalresult.19), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/fatalresult.18-rebase..pr/fatalresult.19)) to fix conflicts with #29668 and #30428

💬 luisschwab commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2237247765)
> So I don't think there is a strong benefit in removing it?

Yes, I agree.

🤔 ryanofsky reviewed a pull request: "refactor: Improve assumeutxo state representation"
(https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-2186500256)
re: https://github.com/bitcoin/bitcoin/pull/30214#pullrequestreview-2177291599

Thanks for reviewing! Will rebase and try to address comments. I'm especially interested if you have ideas on splitting up documentation and code changes into separate PRs.

> I am not so deep into the current state of the kernel work though, so I can not comment on that part, for example the newly introduced typing file. I guess @TheCharlatan could give his nod to that here.

Sure, for background kernel/types.
...

💬 ryanofsky commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1683253240)
re: https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1677573399

> nit: In other places similar checks are inlined without a helper method. That might be actually a bit more clear as well.

Good point, at least at first glance this looks reasonable to inline.

💬 ryanofsky commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1683252990)
re: https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1677640152

> Not sure why we need this when in another commit we remove `GetAll()` and instead use `m_chainstates`. Seems kind of inconsistent but maybe I am missing something.

Is this comment about the `Validity()` method? This is just supposed to be a public accessor for code that cannot access the private m_validity member/

💬 ryanofsky commented on pull request "refactor: Improve assumeutxo state representation":
(https://github.com/bitcoin/bitcoin/pull/30214#discussion_r1683252537)
> Why do you add this function around here instead of using `m_chainstates` as well?

I need to double-check but IIRC, the issue is that m_chainstates requires a lock to access, and it would require larger changes and more verbosity in the tests if they had to be changed to access m_chainstates directly. Will follow up and add a comment describing purpose of this function.

💬 maflcko commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2237344411)
While touching this, another thing to add could be the number of confirmations? I understand that this wouldn't help machine consumers of the interface, but human callers may find it useful?

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#issuecomment-2237345861)
@petertodd I'll give a non-TRUC version another try and report back. I may have been over-thinking things.

💬 luisschwab commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2237347759)
Sure, should also be trivial to add.

📝 Psc544 opened a pull request: "6fe04302f75958ca1ad398e637e9e44c680d3a2eCreate devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/30480)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "6fe04302f75958ca1ad398e637e9e44c680d3a2eCreate devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/30480)

📝 fanquake locked a pull request: "6fe04302f75958ca1ad398e637e9e44c680d3a2eCreate devcontainer.json"
(https://github.com/bitcoin/bitcoin/pull/30480)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2186655119)
Light code review ACK 52c80dff8e26c359d13c4b33e3c75a8519cdcee7, since I still didn't look very closely at merkle path function yet

💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1683349167)
In commit "Have createNewBlock return BlockTemplate interface" (6536b345a346670b8733e5372f988452727a2554)

I don't see any way this condition could be true, now or previously. Would suggest changing this to assert(block_template) or CHECK_NONFATAL(block_template)

Same comment also applies to createNewBlock calls below on lines 374 and 817.

The check [here](https://github.com/bitcoin/bitcoin/blob/20ccb30b7af1c30cece2b0579ba88aa101583f07/src/node/miner.cpp#L108-L111) also appears to be dea
...