💬 naumenkogs commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#issuecomment-1649481055)
ACK f5d17e1a3f2856483ad3a6abcf3ea415f2d05fde

🚀 fanquake merged a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499)

👍 fanquake approved a pull request: "fuzz: Re-enable symbolize=1 in ASAN_OPTIONS"
(https://github.com/bitcoin/bitcoin/pull/28124#pullrequestreview-1545063455)
ACK faa8c1be265d2344a3bc0932455b0182ec7d64c7

🚀 fanquake merged a pull request: "fuzz: Re-enable symbolize=1 in ASAN_OPTIONS"
(https://github.com/bitcoin/bitcoin/pull/28124)

💬 stickies-v commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273298422)
I think this catch now needs to go?

💬 naumenkogs commented on pull request "net: Add new permission `forceinbound` to evict a random unprotected connection if all slots are otherwise full":
(https://github.com/bitcoin/bitcoin/pull/27600#issuecomment-1649517478)
Consider when a malicious Internet provider or something can request the `forceinbound` IP address.

Does this PR open any fundamentally new risk in this case? I'm thinking of disrupting the network by causing many disconnections if the `forceinbound` practice is broad.

This probably depends on whether it's possible to take multiple slots from the same `forceinbound` IP (or, rather, initiate multiple evictions), which I'm not 100% sure about.

If my suspicions are correct, this might bet
...

💬 MarcoFalke commented on pull request "ci: Start with clean env":
(https://github.com/bitcoin/bitcoin/pull/27976#issuecomment-1649519608)
Split out an easy-to-test bugfix: https://github.com/bitcoin/bitcoin/pull/28138 . So please review that one first.

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273305607)
I'll test this.

⚠️ MarcoFalke opened an issue: "error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed"
(https://github.com/bitcoin/bitcoin/issues/28146)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

check failed

### Expected behaviour

not fail

### Steps to reproduce

?

### Relevant log output

https://cirrus-ci.com/task/4769037708689408?logs=ci#L3768

```
test/validationinterface_tests.cpp(93): error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed
```



### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitc
...

💬 MarcoFalke commented on issue "error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed":
(https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1649540097)
`TestInterface::Call();` should be blocking on the destructor, which sets `destroyed`, so I have no idea how this could happen.

💬 MarcoFalke commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273346189)
You forgot to read `-blocksonly` in this function

💬 fanquake commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#issuecomment-1649599987)
I think this PR can now be closed given #28139?

📝 fanquake converted_to_draft a pull request: "build: pass sanitize flags to instrument libsecp256k1 code"
(https://github.com/bitcoin/bitcoin/pull/27991)
secp256k1 functions are now instrumented:
```bash
# objdump --disassemble-symbols=_secp256k1_xonly_pubkey_serialize src/test/fuzz/fuzz | grep sanitizer_cov
100eb5244: 73 5d 01 94 bl 0x100f0c810 <___sanitizer_cov_trace_const_cmp8>
100eb53f0: cc 5c 01 94 bl 0x100f0c720 <___sanitizer_cov_trace_pc_indir>
100eb5418: c2 5c 01 94 bl 0x100f0c720 <___sanitizer_cov_trace_pc_indir>
100eb5444: b7 5c 01 94 bl 0x100f0c720 <___sanitizer_cov_trace_pc_indir>
```

Not sure if this is the best solutio
...

💬 stickies-v commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273361119)
I think this is on purpose since we use `ignore_incoming_txs` for multiple purposes, see also my suggested diff in https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1268400386 (although I now see it can be simplified even further by not passing `ignores_incoming_txs` to the `Options` constructor at all). So, I don't think it's forgotten, but room for improvement in follow-up? Happy to open a pull for this if you think it's an improvement.

💬 fanquake commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1273370949)
Added, but had to use `-m` for macOS users.

💬 MarcoFalke commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1273370954)
Yeah, I don't see what is wrong with your diff (apart from it not compiling).

I just don't get the point of adding the `ApplyArgsManOptions` helper when it is not used consistently. Might as well not add it in the first place?

💬 MarcoFalke commented on pull request "doc: cleanup release process doc":
(https://github.com/bitcoin/bitcoin/pull/28003#discussion_r1273372415)
My assumption was to run the `git` command on the machine that is running guix. I don't think macOS can run guix?

💬 MarcoFalke commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1649628432)
> which may reduce build size and speed up build times.

May be good to add numbers?

💬 MarcoFalke commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#issuecomment-1649629599)
Thanks, yes.

✅ MarcoFalke closed a pull request: "test: Bump walletpassphrase timeout to avoid intermittent issue"
(https://github.com/bitcoin/bitcoin/pull/28089)