🤔 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

🚀 achow101 merged a pull request: "init: completely remove `-zapwallettxes` (remaining hidden option)"
(https://github.com/bitcoin/bitcoin/pull/28787)

👍 kristapsk approved a pull request: "rpc: keep `.cookie` file if it was not generated"
(https://github.com/bitcoin/bitcoin/pull/28784#pullrequestreview-1720723346)
utACK 7cb9367157eb42ee06bc6fa024522cc14a80138d

💬 pinheadmz commented on issue "whiteconnections should be re-added":
(https://github.com/bitcoin/bitcoin/issues/8798#issuecomment-1802198296)
Ping @theymos @asoltys take a look at #27600 please and leave a comment if this is still an issue that concerns you

📝 willcl-ark opened a pull request: "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes"
(https://github.com/bitcoin/bitcoin/pull/28824)
Closes: #27795
Closes: #7996

Previously `ScriptToAsmStr` returned hex-encoded integers, except if data length was <= 4 bytes, in which case it displayed using decimal encoding.

Remove the decimal encoding carve-out for small pushes and always display hex results from `decodescript` RPC.

There were [other suggestions](https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1692870456) for approaches in #27795, but IMO moving to hex-only-always makes the most sense.

💬 luke-jr commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1802220054)
Maybe snapshots should include it?

💬 luke-jr commented on pull request "Embedding ASMap files as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-1802223047)
Why is the option of loading it from a file even in dev builds, not considered?

💬 maflcko commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#discussion_r1386888196)
For reference, this whole assert block is likely wrong, see the thread at https://github.com/bitcoin/bitcoin/pull/28562#discussion_r1350470707