💬 dergoegge commented on pull request "net: use interruptible async getaddrinfo wrapper from libevent for DNS":
(https://github.com/bitcoin/bitcoin/pull/27505#issuecomment-1517517368)
Afaik we currently do not expose libevent on any public facing interface (e.g. p2p) and only use it for things that aren't supposed to be exposed (e.g. rpc or rest). By using it for DNS queries we would be changing that (e.g. a malicious DNS seeder) and I'm not sure if that is the best idea given that libevent is pretty archaic (I think the MSan, ASan, TSan and LSan failures in the CI are kind of proving my point).

💬 MarcoFalke commented on pull request "ci: use LLVM/clang-16 in native_asan job":
(https://github.com/bitcoin/bitcoin/pull/27360#discussion_r1173544917)
Hmm, 2110 worked last time I checked: https://github.com/bitcoin/bitcoin/pull/25528#discussion_r918013975

💬 MarcoFalke commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1517534970)
Reverted the last commit, should be good to go now.

💬 dergoegge commented on pull request "refactor: Introduce EvictionManager and use it for the inbound eviction logic":
(https://github.com/bitcoin/bitcoin/pull/25572#issuecomment-1517535588)
Rebased, have not addressed Martin's comments yet

👍 dergoegge approved a pull request: "fuzz: re-enable prioritisetransaction & analyzepsbt RPC"
(https://github.com/bitcoin/bitcoin/pull/27464#pullrequestreview-1395428483)
utACK faa7144d3cf41e6410d942a3c485982ee65b3c6e

📝 fanquake opened a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507)
Type stubs are now available as of 0.13.0.
See https://github.com/lief-project/LIEF/issues/650.

💬 fanquake commented on pull request "lint: stop ignoring LIEF imports":
(https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517561645)
> ERROR: Could not find a version that satisfies the requirement lief==0.13.0 (from versions: 0.8.0.post7, 0.8.1.post1, 0.8.2.post1, 0.8.3.post3, 0.9.0, 0.10.0, 0.10.1, 0.11.0, 0.11.1, 0.11.2, 0.11.3, 0.11.4, 0.11.5, 0.12.0, 0.12.1, 0.12.2, 0.12.3)
ERROR: No matching distribution found for lief==0.13.0

Looks like [LIEF requires 3.8+](https://pypi.org/project/lief/), which is why this is failing to install. Guess this will be solved after #27483.

👍 hebasto approved a pull request: "move-only: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419#pullrequestreview-1395476027)
ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged.

👍 TheCharlatan approved a pull request: "Bump python minimum version to 3.8"
(https://github.com/bitcoin/bitcoin/pull/27483#pullrequestreview-1395478975)
utACK fa6eb6516727a8675dc6e46634d8343e282528ab

💬 hebasto commented on pull request "move-only: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1517589863)
> Hoping this moveonly change could be reviewed and merged quickly since it conflicts with various PR's I have open

So am I (meaning cmake staging branch) :D

🤔 dergoegge reviewed a pull request: "mempool: keep CPFP'd transactions when loading from mempool.dat"
(https://github.com/bitcoin/bitcoin/pull/27476#pullrequestreview-1395486586)
Approach ACK - importing all and then trimming seems like the least complex approach to me.

💬 dergoegge commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#discussion_r1173589155)
Afaict we currently import mempool txs from disk in the background and allow the node to make connections and sync the chain in parallel. Seems like this commit changes that by locking cs_main for the entire import duration. Not sure about the consequences.

Potentially relevant for #27460 as well.

👍 hebasto approved a pull request: "test: Remove unused sanitizer suppressions"
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1395494099)
ACK fa15a9934ee1d331737c631e6ffc2ddfafaddb7f

Nice to see the updated comments with TSan logs.

🤔 stickies-v reviewed a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1395462615)
Concept ACK

💬 stickies-v commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1173574801)
```suggestion
RPCResult::Type::OBJ_DYN, "prioritisation-map", "prioritisation keyed by txid",
```

💬 stickies-v commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1173597419)
Slight rephrasing suggestion:
```suggestion
/** Return a map of all entries in mapDeltas with as value their fee delta and whether the transaction is present in the mempoool. */
```

💬 batriskaweb3 commented on issue "Compiling a bitcoin core version that accepts transactions over 100vkb":
(https://github.com/bitcoin/bitcoin/issues/27490#issuecomment-1517608382)
thanks alot
any idea why this error could be occurring?
`Making all in src
make[1]: Entering directory '/home/doodles/bitcoin/src'
make[2]: Entering directory '/home/doodles/bitcoin/src'
CXX bitcoind-bitcoind.o
CXX init/bitcoind-bitcoind.o
CXX libbitcoin_node_a-addrdb.o
CXX libbitcoin_node_a-addrman.o
CXX libbitcoin_node_a-banman.o
CXX libbitcoin_node_a-blockencodings.o
In file included from ./txmempool.h:24,
from blockencodi
...

🚀 fanquake merged a pull request: "move-only: Extract common/args from util/system"
(https://github.com/bitcoin/bitcoin/pull/27419)

👍 fanquake approved a pull request: "test: Remove unused sanitizer suppressions"
(https://github.com/bitcoin/bitcoin/pull/27498#pullrequestreview-1395513861)
ACK fa15a9934ee1d331737c631e6ffc2ddfafaddb7f

💬 TheCharlatan commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1173586914)
Why is the `banman` moved into the options?