💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1578537557)
have you considered using `SoftSetBoolArg` to set `-walletbroadcast` to 0 if `-privatebroadcast` was chosen?
It's not the most user-friendly thing to introduce a new command arg that would only work if another arg was also changed from its default.

💬 mzumsande commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1580115581)
It looks like we make sure in `net_processing` to not send any of theses messages, and here we check again. I haven't thought about this long, but can this log be triggered, or could this be an `Assume(false)` instead?

💬 davidgumberg commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1580221364)
On second thought it seems a bad idea to depend on the order in which tests are called for deciding whether or not to perform cleanup. I think that might be kind of a footgun for anyone modifying the test in the future

💬 davidgumberg commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2078316239)
ACK https://github.com/bitcoin/bitcoin/pull/29122/commits/be2c5510521081119f62ef3b29f76bb9097d0f34
Re-reviewed code, and attempted to break tests by making `ConsiderEviction` misbehave.

It is worth noting that these tests will not catch if outbound eviction logic gets triggered too early. e.g.:

```cpp
/** How long to wait for a peer to respond to a getheaders request */
static constexpr auto HEADERS_RESPONSE_TIME{0s};
// [...]
/** Timeout for (unprotected) outbound peers to sync to ou
...

:lock: fanquake locked an issue: "Bitcoin with horcrux and powmem?"
(https://github.com/bitcoin/bitcoin/issues/29964)

📝 davidgumberg opened a pull request: "Lint: support running individual rust linters and improve subtree exclusion"
(https://github.com/bitcoin/bitcoin/pull/29965)
This PR improves subtree exclusion by rewriting lint checks to reuse common exclusion logic. Most lint checks previously did not exclude `crypto/ctaes`, the locale linter did not exclude `src/crc32`

Makes the include guards lint check print all missing include guards before exiting.

Adds support for running individual linters by passing `--lint=LINT_CHECK_TO_RUN` to the lint runner. If using cargo run arguments can be passed e.g.:

```bash
cd test/lint/test_runner && cargo run -- --lint
...

💬 davidgumberg commented on pull request "lint: Refactor lint checks to reuse exclusion logic; Support running individual lint checks in lint runner":
(https://github.com/bitcoin/bitcoin/pull/29744#issuecomment-2078380040)
@maflcko @fjahr Thank you for the feedback, sorry for the confusion on this PR. I've opened https://github.com/bitcoin/bitcoin/pull/29965

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2078452007)
Rebased 88aa093356ff142efe60df989323eb1ed1aa9e18 -> 0cbcb66257db65f0a68d82d17a2d1a83d8654b38 ([`pr/saferesult.5`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.5) -> [`pr/saferesult.6`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.6), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/saferesult.5-rebase..pr/saferesult.6)) dropping Result::Update() method in this PR, and last 2 uses of it.

re: https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2078213
...

💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-2078502542)
> Throw `DISABLED_OPCODE` error for op_ver and its variants as it's known but just disabled opcodes. Currently, interpreter throws `BAD_OPCODE` which contains the message of `"Opcode missing or not understood"`, which isn't accurate and confusing.

Apologize for poor description, I just re-write to make it simple and clear.
I agree that this one is practically doesn't affect any, as only a few will push OP_VER opcodes on stack.
It's such a minor fix, and when I just found out that OP_VER thr
...

💬 fanquake commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2078569791)
Seems like there's at least something which could/should be backported here? Maybe only the other change, given the newly broken code has only existed in master?

✅ maflcko closed a pull request: "script: throw disabled err for op_ver and its variants"
(https://github.com/bitcoin/bitcoin/pull/28169)

💬 maflcko commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-2078663017)
> It's ok to close this PR if that is better.

Ok, I'll close it for now. When there is need, it can be re-opened later or picked up again. Thanks for working on this for such a long time.

📝 maflcko opened a pull request: "test: Add two more urlDecode tests"
(https://github.com/bitcoin/bitcoin/pull/29967)
Trivial follow-up after https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1579216072

💬 maflcko commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2078730201)
The Win64 GHA test failure looks unrelated. If you want, you can split up 54b2115bb848e1aad8a86aae858de63a851235ce into its own pull request, as it is an independent improvement. But up to you.

💬 maflcko commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1580556442)
https://github.com/bitcoin/bitcoin/pull/29967/files

💬 laanwj commented on pull request "test: Add two more urlDecode tests":
(https://github.com/bitcoin/bitcoin/pull/29967#issuecomment-2078798420)
Code review ACK https://github.com/bitcoin/bitcoin/pull/29967/commits/fa55972a758865a6bd0114afe72e51877896d495

💬 fanquake commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#discussion_r1580606777)
FYI https://github.com/bitcoin/bitcoin/runs/24257306970:
```bash
init.cpp: In function ‘bool AppInitMain(node::NodeContext&, interfaces::BlockAndHeaderTipInfo*)’:
init.cpp:1319:20: error: unused variable ‘unix’ [-Werror=unused-variable]
1319 | const bool unix{port_option.second};
| ^~~~
```

👍 stickies-v approved a pull request: "test: Add two more urlDecode tests"
(https://github.com/bitcoin/bitcoin/pull/29967#pullrequestreview-2024343300)
ACK fa55972a758865a6bd0114afe72e51877896d495

💬 fjahr commented on pull request "test: Add two more urlDecode tests":
(https://github.com/bitcoin/bitcoin/pull/29967#issuecomment-2078824038)
ACK fa55972a758865a6bd0114afe72e51877896d495

💬 fanquake commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#issuecomment-2078841172)
> Note that there are some API functions of the Popen class that we don't use, e.g. wait(), pid(), poll(), kill(), but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there.

Deleting as much as possible seems fine. If code turns out to be needed in future (although I wouldn't think we are going to expand this interface), it can be retrieved from the git history.