🤔 danielabrozzoni reviewed a pull request: "validation: Improve input script check error reporting"
(https://github.com/bitcoin/bitcoin/pull/31097#pullrequestreview-2382244315)
post merge tACK 86e2a6b749c7fecbd086b361806ac9f6e9426d79

💬 sipa commented on pull request "Improve parallel script validation error debug logging":
(https://github.com/bitcoin/bitcoin/pull/31112#issuecomment-2426831357)
Rebased after the merge of #31097.

💬 davidgumberg commented on pull request "coins: Add move operations to Coin and CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/30643#discussion_r1808928993)
Is the reason that all of these default constructors can be declared noexcept that all of `Coin`'s members' respective default constructors that get invoked are noexcept?

💬 instagibbs commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1808939633)
good point, could be used elsewhere too in a follow-up e.g., `testmempoolaccept`, `submitpackage`, etc. done.

💬 instagibbs commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1808939884)
changed thanks

💬 instagibbs commented on pull request "rpc: Add support to populate PSBT input utxos via rpc":
(https://github.com/bitcoin/bitcoin/pull/30886#discussion_r1808940077)
fixed

💬 sipa commented on pull request "coins: Add move operations to Coin and CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/30643#discussion_r1808941268)
According to https://en.cppreference.com/w/cpp/language/noexcept_spec:

> [default constructors](https://en.cppreference.com/w/cpp/language/default_constructor), [copy constructors](https://en.cppreference.com/w/cpp/language/copy_constructor), [move constructors](https://en.cppreference.com/w/cpp/language/move_constructor) that are implicitly-declared or defaulted on their first declaration unless
>
> * a constructor for a base or member that the implicit definition of the constructor woul
...

🤔 danielabrozzoni reviewed a pull request: "[refactor] Cleanup BlockAssembler mempool usage"
(https://github.com/bitcoin/bitcoin/pull/28843#pullrequestreview-2382356336)
re-ACK 192dac1

👍 stickies-v approved a pull request: "[refactor] Cleanup BlockAssembler mempool usage"
(https://github.com/bitcoin/bitcoin/pull/28843#pullrequestreview-2382365246)
ACK 192dac1d3370edd579db235d69c034726f37c8da

💬 fjahr commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#issuecomment-2426954572)
re-ACK a16917fb5981d1465ffd4c036586f8729e683b73

Thanks for addressing it!

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2426960596)
@fanquake thanks, I'll look into that along with a rebase - might be after TABConf.

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2426964142)
cc @theuni any takes?

🤔 marcofleon reviewed a pull request: "test: Fuzz the human-readable part of bech32 as well"
(https://github.com/bitcoin/bitcoin/pull/30623#pullrequestreview-2382423311)
Code review ACK 9b7023d31a3ec95f66b45f0ecb47e79762d74854. The separation into two targets and the new `GenerateRandomHRP` seem fine to me.

Decode [coverage](https://marcofleon.github.io/coverage/bech32decode/)
Roundtrip [coverage](https://marcofleon.github.io/coverage/bech32roundtrip/)

💬 Sjors commented on issue "Mining Interface doesn't allow for Bitcoin Core to create blocks when it wants":
(https://github.com/bitcoin/bitcoin/issues/31109#issuecomment-2426976364)
> This includes the ability to create a new block in between validating a new block and updating the mempool for a full CNB run.

The interface isn't final so that could be added. But aren't we holding `cs_main` throughout the process? cc @TheCharlatan, @ryanofsky

🤔 jonatack reviewed a pull request: "rpc: net: follow-ups for #30062"
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2382439587)
LGTM but the commit could be more accurately renamed to "Improve mapped AS documentation" and perhaps touch up this doc in the same commit:

```diff
--- a/src/netgroup.h
+++ b/src/netgroup.h
@@ -35,9 +35,9 @@ public:
std::vector<unsigned char> GetGroup(const CNetAddr& address) const;

/**
- * Get the autonomous system on the BGP path to address.
+ * Get the autonomous system number at the end of the BGP path to the IP address.
*
- * The ip->AS mapping
...

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1809026225)
That might make sense, though I'm not sure how much these round trips matter when running on the same machine.

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1809027825)
I don't think we should be trying to make the IPC interface DoS resistant. We don't do that for RPC either.

💬 laanwj commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2426998599)
Lightly tested native builds on Debian 12.7 and Ubuntu 24.04 using the qt6 packages described in build-unix.md. Found no regression compared to Qt5. Will test guix builds next.

fwiw: this can be trivially rebased by dropping the MacOS 12 commit, all the conflicts are in that.

💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2427009621)
> Do you think the lack of desire for review (compared to reviewing the capnproto protocol instead) comes from the encryption, the framing logic, the connman management, etc?

The "instead" misses the mark: reviewing capnproto was already necessary to achieve all the other use cases of multiprocess. The mining interface just provided an easier opportunity to make progress there.

Trying to redesign the interface in a way that makes it DoS-safe brings back the need for a lot of the review tha
...

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1809048162)
done, previously this would have allowed non-dust-spending children and invalidate the invariant checks, now it shouldn't matter

💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1809048220)
done