💬 brunoerg commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803693573)
> Would be good to explain this a bit more, to make sure this is not a bug.

I think fuzz can generate subnets with a zone identifier, like: "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121", which is a valid one. However, the lookup call might change the zone identifier. In my machine (macOS), "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121" becomes "676:c962:7962:b787:b392:fed8:7058:c500%31097/121" after lookup and it makes the assertion fails.

💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-1803694990)
Concept ACK

💬 hebasto commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387909280)
Thanks! Done.

💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#issuecomment-1803710156)
lgtm ACK 9f208c017174132dbefbc917aa9824c279382597

💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803717471)
Ah ok. And about the efficiency, is it possible to keep the fuzz input format unchanged instead of using a string representation (Maybe via a string-lookup roundtrip)? I wonder if that'd be more efficient.

🚀 fanquake merged a pull request: "ci: Switch IWYU to `clang_17` branch"
(https://github.com/bitcoin/bitcoin/pull/28826)

👍 ismaelsadeeq approved a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1722502299)
ACK 2be5e799ba623f969f5ffc59667a5bc6799df290 🥘

📝 maflcko opened a pull request: "test: Avoid intermittent failures in feature_init"
(https://github.com/bitcoin/bitcoin/pull/28831)
The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures.

Fix the intermittent test failures by reverting https://github.com/bitcoin/bitcoin/commit/5ab6419f380cc0a8cde78b125f3eeee5fcba43ae .

🤔 ryanofsky reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1722504219)
Thanks for the reviews.

Updated d7477b1c9cde1e795e006d1788c79b8985970eee -> 52ecb6547dd2b59643483e43531d1201941c0df0 ([`pr/noshut.16`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.16) -> [`pr/noshut.17`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noshut.16..pr/noshut.17)) with suggested changes

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387982956)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377678893

> note for other reviewers (and future self): `NodeImpl::startShutDown()` does have [this additional code block](https://github.com/bitcoin/bitcoin/blob/d51fb9caa622add96c6b594e162da5584eb927fc/src/node/interfaces.cpp#L124-L128)

There is a change in behavior here but the new behavior should be more correct. RPC calls should be interrupted whenever the GUI is shut down, regardless of how it is shut down. Calls to `Star
...

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983151)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1364373784

> NIt: s/namespaces/namespace/

Thanks, fixed

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983370)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1379378726

> I don't think we need to expose those internal error details. Also, I know we exclude `server.cpp` from the CHECK_NONFATAL linter, but given that this is an RPC method I think it would be preferable here?

Thanks, replaced JSONRPCError exception with nonfatal exception

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983586)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377746899

> nit: missing `#include <util/check.h>`

Thanks, fixed

💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387982522)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377777492

> nit: long line

Thanks, added line break

✅ fanquake closed an issue: "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test"
(https://github.com/bitcoin/bitcoin/issues/27582)

💬 fanquake commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1803871925)
Closing for now. Don't seem to be able to reproduce.

🤔 glozow reviewed a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1722565783)
ACK fa520848da
Code LGTM, tried a few roundtrips of stop/start/savemempool/importmempool with combinations of the options. I originally thought `-persistmempool=0` should imply something for `-persistmempoolv1` but makes sense that it applies to `savemempool` when we're not persisting.

💬 glozow commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1388024702)
nit: would be nice to name 1 as `MEMPOOL_DUMP_VERSION_V1` or `MEMPOOL_DUMP_VERSION_NO_XOR`

💬 glozow commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1388019621)
Should the mempool_compatibility.py test be updated? (the comment "200100 # Last release with previous mempool format" isn't accurate anymore)

💬 instagibbs commented on pull request "fuzz: Minor improvements to tx_package_eval target":
(https://github.com/bitcoin/bitcoin/pull/28825#discussion_r1388055793)
done