👋 mzumsande's pull request is ready for review: "test: fix intermittent failure in p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/27864)

💬 TheCharlatan commented on pull request "kernel: Remove shutdown globals from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1587883295)
I opened #27861 to introduce the interrupt class and implement the `fatalError` notification. Also opened https://github.com/bitcoin/bitcoin/pull/27866 to start tackling proper error return types when a function calls abort.

💬 Xekyo commented on pull request "doc: update comment in policy.cpp to refer to virtual bytes":
(https://github.com/bitcoin/bitcoin/pull/27726#discussion_r1227098886)
Yeah, this should be **vbytes** instead of bytes. The figure is also inaccurate, a P2WPKH input is [67.75 vbytes (for low r) or 68.0 vbytes for (high r)](https://bitcoin.stackexchange.com/a/100160/5406).

💬 achow101 commented on pull request "wallet: Migrate wallets that are not in a wallet dir":
(https://github.com/bitcoin/bitcoin/pull/26740#discussion_r1227130800)
Done as suggested

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#issuecomment-1587940978)
Addressed the feedback and rebased, which seems to have fixed the CI issues finally.

🤔 ryanofsky reviewed a pull request: "validation: Return on abort"
(https://github.com/bitcoin/bitcoin/pull/27866#pullrequestreview-1475602374)
I'm not sure about all the changes in this PR because it seems like some validation code is intentionally written to ignore flush errors, and now they are returning early and skipping other work. It also seems reasonable that some failed flushes like periodic flushes might not need to be fatal errors.

But adding return values and nodiscard attributes could at least make it clear if errors are being ignored intentionally or not.

💬 ryanofsky commented on pull request "validation: Return on abort":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1227107197)
In commit "blockstorage: Return on fatal flush errors" (1fe06a811bf8be941bef5336af97688f48885828)

This method is not marked nodiscard, and it seems like there are some cases where this error is ignored and not returned to the caller.

💬 ryanofsky commented on pull request "validation: Return on abort":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1227119951)
In commit "chainstate: Return on ValidatedSnapshotCleanup fatal failure" (360fe5a47f75514621ef62bc4c906f2112d3308f)

I think this should return FAILURE not INTERRUPTED so caller can know that an error happened, not just an early shutdown request.

But I'd actually suggest dropping this commit from the PR. #27862 should cover this case, and I opened it earlier because I think AbortNode calls in assumeutxo code and in flush code seem different. There should be less of a rationale for ignoring
...

👍 ryanofsky approved a pull request: "wallet: Migrate wallets that are not in a wallet dir"
(https://github.com/bitcoin/bitcoin/pull/26740#pullrequestreview-1475669026)
Code review ACK a1e653828bc59351b2a0dd5a70f519e6b61199bc

Just applied suggested change since last review

🤔 Xekyo reviewed a pull request: "Use `int32_t` type for most transaction size/weight values"
(https://github.com/bitcoin/bitcoin/pull/23962#pullrequestreview-1475650641)
codereview ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f

💬 Xekyo commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#discussion_r1227136627)
Re-reviewing this basically from scratch, I stumble here. `int32_t` should be big enough to represent the weight of more than 5,000 max standard weight transactions. Are we actually bumping into overflow issues with `int32_t` somewhere for size with descendants?

I see that this was brought up before, so I assume it was left a 64-bit value to limit the scope of this PR. Is that right?

💬 Xekyo commented on pull request "Use `int32_t` type for most transaction size/weight values":
(https://github.com/bitcoin/bitcoin/pull/23962#discussion_r1227137928)
As above

💬 TheBlueMatt commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1587954425)
> In terms of RPC calls, this is of course possible. With stratum v2 some of the goals are around improving performance and reducing latency. JSON RPC can fall short in speed and efficiency.

Its not just latency and overhead, however - one of the important goals of replacing GBT in Bitcoin Core broadly is to move towards everything being push, rather than polling. In general, block templates should come out of Bitcoin Core very aggressively - when a new block comes in and has been validated,
...

📝 supernormand opened a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/27867)
<!--
*** 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
...

✅ pinheadmz closed a pull request: "Initial commit"
(https://github.com/bitcoin/bitcoin/pull/27867)

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1227151300)
Addressed in #26152 by introducing a method to update the Ancestor set state.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1227152852)
Implemented in #26152

👍 ryanofsky approved a pull request: "Use `int32_t` type for most transaction size/weight values"
(https://github.com/bitcoin/bitcoin/pull/23962#pullrequestreview-1475680958)
Code review ACK 3ef756a5b558a1dd2fcb93bc0d4237707aa04f3f. Since last review, just rebased with more type changes in test and tracing code

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226752830)
unused

💬 instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226794309)
I don't think this is true?

We're only returning `TxValidationResult::TX_MEMPOOL_POLICY` for package size of 1(which becomes a Single Accept), so and subsequent subpackage relying on this prior tx will infer a result of `TX_MISSING_INPUTS`, `invalid-tx-dependency` in `AcceptPackageWrappingSingle`.

Wondering if it might be worth it to have a specific `TxValidationResult` that is precisely for the reasons that we may allow re-evaluation(early-ish abort due to low (package) feerate), and no o
...