💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2108700980)
```suggestion
# At the end the old fork is revalidated to cause reorg, and all of the created
```

💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109024282)
nit in 2b202e9db56487e658fc41089178f31ef4259a0d: Could force named args, while touching this?

```py
... size, *, n_tx_to...

💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109020817)
nit in 05abcc86998f6649b48ed3a2eaf2a024ba42af5c: Remove the comment, or add "coins to test", or "coins for test"?

💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109025020)
(nit in https://github.com/bitcoin/bitcoin/commit/2b202e9db56487e658fc41089178f31ef4259a0d:)

💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109025763)
nit in https://github.com/bitcoin/bitcoin/commit/2b202e9db56487e658fc41089178f31ef4259a0d: Any reason to remove this here?

💬 maflcko commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2109038105)
nit in https://github.com/bitcoin/bitcoin/commit/2b202e9db56487e658fc41089178f31ef4259a0d: could clarify the exact error string?

```
too-long-mempool-chain, too many unconfirmed ancestors [limit: 100]

💬 maflcko commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#discussion_r2109053322)
thx, I'll leave as-is for now to make it possible to review via `--color-moved=dimmed-zebra`. Seems better to be done as a repo-wide follow-up anyway, because this affects all docs.

💬 l0rinc commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#issuecomment-2912331467)
ACK fa2bf6bdb7e5f8940fe4bce1aaab29bf2e063b49

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2912362918)
https://cirrus-ci.com/task/4713767570767872?logs=ci#L3220

💬 maflcko commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986)
why is this needed? Isn't the chain already `MAIN`?

🤔 TheCharlatan reviewed a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981#pullrequestreview-2870950393)
Concept ACK

💬 TheCharlatan commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109107660)
In commit 981853ab25dc6f967f393ecba57965afb8a822f3

I would have suggested the same. It is not clear to me why you are introducing a new `blockCoins` (camel case?) view here. Maybe do some of these `TestBlockValidity` refactors and documentation in a separate commit. It is not quite clear to me why this should be changed.

💬 maflcko commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109111837)
“into a OP_RETURN output.”
-> “into an OP_RETURN output.” [article before vowel sound “O”]


I am also thinking about splitting up the "make this arg optional". Seems like a small and easy preparatory pull request?

💬 maflcko commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2109115291)

participatns -> participants [correct misspelling in error message]

💬 brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109117905)
Just removed it. Redudant.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#issuecomment-2912418699)
08b574dde2c5f13a4c9dadcb9d8bded158c59623..5f2fb4dd333f8d1db27c78414afc84454e6ee4a5: Address https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2109086986

🤔 naiyoma reviewed a pull request: "rpc: generateblock to allow multiple outputs"
(https://github.com/bitcoin/bitcoin/pull/32468#pullrequestreview-2870982878)

A summary of my understanding of this PR so far, regarding the `generateblock` changes: :
- [x] Outputs and transactions are now optional.
- [x] `generateblock` can now take an array of addresses and distribute the reward equally among them.
- [x] `generateblock` with an empty array - in this case fallback to OP_TRUE
- [ ] I think there's an intention to add a remainder — that way, it's possible to specify part of the reward to specific addresses and then split the remainder among the oth
...

💬 maflcko commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109129254)
https://cirrus-ci.com/task/4825274484785152?logs=ci#L3624

```
in sync_mempools
[23:01:19.556] raise AssertionError("Mempool sync timed out after {}s:{}".format(
[23:01:19.556] AssertionError: Mempool sync timed out after 2400s:
[23:01:19.556]

💬 instagibbs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2109129595)
weight / WITNESS_SCALE_FACTOR = vbytes

A bit confusing

💬 naiyoma commented on pull request "rpc: generateblock to allow multiple outputs":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2109130072)
Delete this instead of commenting it out.