💬 glozow commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542734986)
> note that the 💎 issue still exists in https://github.com/bitcoin/bitcoin/pull/26711

Yeah. Rebasing it on top of this.

💬 brunoerg commented on pull request "net, refactor: net_processing, add `ProcessCompactBlockTxns`":
(https://github.com/bitcoin/bitcoin/pull/26969#issuecomment-1542743968)
Rebased

💬 brunoerg commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#issuecomment-1542744503)
Concept ACK

💬 glozow commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1190350142)
Thanks, fixed

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1190350382)
Ooooo thank you! C++ rookie here :smile:

Fixed in f5c8b5f78f93bea96e6846ef9d5b787269029b50

🤔 mzumsande reviewed a pull request: "refactor, kernel: Decouple ArgsManager from blockstorage"
(https://github.com/bitcoin/bitcoin/pull/27125#pullrequestreview-1421312759)
Code Review ACK 5ff63a09a9edd1204b2cc56cf6f48a44adab7bb3

💬 mzumsande commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1190350876)
nit (if you repush): blockman should be added to the doc, also it would be nice to keep the out param at the end according to dev notes.

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#discussion_r1190362272)
This is a great point. I think I need to take a closer look at some of the refactoring that's going on in `net` in hopes of finding other examples/decisions that support making `NetStats` a member variable in `CNode`. I think the comment you linked to is very relevant.

💬 satsie commented on pull request "rpc: add 'getnetmsgstats', new rpc to view network message statistics":
(https://github.com/bitcoin/bitcoin/pull/27534#issuecomment-1542767255)
Thank you for testing and taking a peek at this @ccdle12 :hugs: Appreciate the review and will definitely be taking you up on the offer for a more in depth review as this evolves! :wink:

💬 achow101 commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#issuecomment-1542843029)
re-ACK 72efc26439da9a1344a19569fb0cab01f82ae7d1

🚀 achow101 merged a pull request: "util: improve FindByte() performance"
(https://github.com/bitcoin/bitcoin/pull/19690)

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437947)
Re-added

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190437970)
Yeah. Added some docs to the class (also mentioned BIP331) so maybe it's more clear? I used to call it `Packageifier` because it can potentially build a package out of any random list of transactions. But then it's weird because we packageify a `Package`. Open to naming improvements 😅

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438085)
Elaborated on the comment, hopefully it's better now

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438226)
> I think hashing these bytes would defeat this since it can't generate a valid tx with that txid?

Did this one, thanks 👍

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

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438618)
Changed to always use `package[i]`, and added a comment about this unstable sort

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1190438675)
Aha, thanks!

📝 ismaelsadeeq opened a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622)
Fixes #27555

The issue arises during initialization where an old `fee_estimates.dat` file is sometimes read during initialization.
Or after an unclean shutdown, the latest fee estimates are not flushed to `fee_estimates.dat`.
If the fee estimates in the old file are old, they can cause transactions to become stuck in the mempool.
This PR ensures that nodes do not use stale estimates from the old file during initialization. If `fee_estimates.dat`
has not been updated for 12 hours or mo
...

💬 theuni commented on pull request "build: LLVM 16 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1542911418)
I've experimented and implemented with the above here: https://github.com/theuni/bitcoin/commits/21778

@fanquake I think we want https://github.com/theuni/bitcoin/commit/a0427fc986294ef81605b4c44b54c3ec9825bf48 regardless, which fixes the libbitcoinconsensus linking issue. tl;dr: It makes lld linking work like ld64 by skipping a busted test. The `-r` was a red herring as a result of that busted test.

Shall I go ahead and PR that commit separately?