💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1801958804)
Force-pushed covering more functions. Left it running for a long time and didn't see any downside.

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1386694325)
Did that for fun, and it prints the error message:

```
test_framework.authproxy.JSONRPCException: Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order when using assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height 299 (-4)
```

Hacky diff:

```diff
diff --git a/test/functional/feature_assumeutxo.py b/test/fun
...

🚀 fanquake merged a pull request: "fuzz: Avoid timeout and bloat in fuzz targets"
(https://github.com/bitcoin/bitcoin/pull/28815)

💬 maflcko commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1801990207)
Another idea would be to start a new wallet test. For now it could only check that it throws an error. See the example hacky diff I wrote, which will need to be moved to a new test file: https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1386694325

📝 fanquake opened a pull request: "ci: remove note re M1 usage"
(https://github.com/bitcoin/bitcoin/pull/28823)
M1 is now available in GitHub CI, but we don't currently have a plan to use it, so remove the comment.

💬 jsarenik commented on issue "v26.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/28718#issuecomment-1802095699)
Running two 26rc2 nodes, using binaries from bitcoincore.org, one on aarch64, one on amd64. Both are also on Tor and v2transport-enabled, connected via v2 at least to one another node running daily master.

Apart from local bitcoin-cli connections keeping bitcoind from restarting, which is present for some time since around v25 on master, everything works fine. Thanks.

💬 stratospher commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1802144035)
Nice! Concept ACK.

> "Before, a global -v2transport provided to the test would be dropped when restarting the node within a test."

@mzumsande, wanted to confirm which scenario you're talking about. i tried it out in [this file](https://github.com/stratospher/bitcoin/blob/ab72033bdf9e3e2155fe645a1c4b71d0d6249a57/test/functional/test_v2transport.py).

1. when test is run with `--v2transport` option and `TestNode` (with no `extra_args` passed in the test) is restarted? `TestNode` restarted
...

🤔 glozow reviewed a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1720647559)
LGTM aedfef28af6e8c5bd8833fb63e2ef4f3264f934c, just a couple introductions of uint256 that should probably be the type-safe ones instead

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386805310)
6903d8aa37e01d16aea0b53c3729f8f92c7d2bf0
Could you use `Txid` instead? or `GenTxid`

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386815017)
Wow TIL there's a `get`!

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386820528)
aedfef28af6e8c5bd8833fb63e2ef4f3264f934c
could use `Txid` for this as well

💬 glozow commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386821034)
same

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#issuecomment-1802147662)
Force pushed from 09436b21e84cc6cd979fe4942ba1c415c4bc91be to 969f5ec4a8
- Addressed @glozow review comments
- Added a commit that `SyncWithValidationInterfaceQueue` at the start of fee estimation RPC's
- The relationship between `RemovedMempoolTransactionInfo`, `NewMempoolTransactionInfo`, and `TransactionInfo` has been updated to use composition. `TransactionInfo` object is now a member of `RemovedMempoolTransactionInfo` and `NewMempoolTransactionInfo`.

💬 maflcko commented on pull request "ci: remove note re M1 usage":
(https://github.com/bitcoin/bitcoin/pull/28823#issuecomment-1802149189)
lgtm ACK 8cbb6196913b22006dac75f719a2834ab0d6c94f

🤔 glozow reviewed a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1720677427)
drahty pong

👍 luke-jr approved a pull request: "rpc: keep `.cookie` file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28784#pullrequestreview-1720677514)
utACK, though it might be better to do something like keep the file open and detect if it's the same file/content before we delete it.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1386826811)
> `SyncState` yeah that would work if you come up with a good enum values.

Maybe `Synced`, `RecentStale`, `Stale`?
Named the second and third "stale" because these states have no in-flight block requests.

Still, this could also be easily changed in the future if we come up with better names.

💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1378898930)
clang-format new code?

```diff
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
index 1b188e0f49..d323a320b5 100644
--- a/src/primitives/transaction.h
+++ b/src/primitives/transaction.h
@@ -189,13 +189,12 @@ public:

struct CMutableTransaction;

-struct TransactionSerParams
-{
+struct TransactionSerParams {
const bool allow_witness;
SER_PARAMS_OPFUNC
};
-static constexpr TransactionSerParams TX_WITH_WITNESS{.allow_witness=true};
-static co
...

💬 luke-jr commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1802153356)
>Note that using an older version of the kernel headers inside Guix, is not a "hack" for compatibility, and is explicitly recommended against by glibc:

Yet boost doesn't make the same compatibility guarantees, and needs to be checked.

💬 achow101 commented on pull request "init: completely remove `-zapwallettxes` (remaining hidden option)":
(https://github.com/bitcoin/bitcoin/pull/28787#issuecomment-1802168625)
ACK 5039c346ca87d6112ea1eb124bdc622ba9e9a513