💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2386939634)
This code was not fully baked between me soliciting conceptual feedback and putting it up for formal review, thanks for catching that!

Switched to:
```C++
if (std::any_of(argv, argv + argc, [] (char* arg) { return strcmp(arg, "-waitfordebugger") == 0; })) {
```

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2388939051)
Might be some lingering bad habit to include windows.h early. CI seems okay with sorting - fixed.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2387353926)
That piece of C++26 wasn't on my radar, nice! Added comment.

💬 hodlinator commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2388927844)
Happy to take contributions for them, but focusing on the main ones for now.

📝 JACOBO10S opened a pull request: "Create leading new hihgs"
(https://github.com/bitcoin/bitcoin/pull/33505)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 JACOBO10S commented on pull request "Create leading new hihgs":
(https://github.com/bitcoin/bitcoin/pull/33505#issuecomment-3349009108)
next one

🤔 fjahr reviewed a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349#pullrequestreview-3281444082)
Hm, I was hoping I could easily test this by uncommenting the change in the first commit and then running this in the CI, which I did by pushing to this branch: https://github.com/fjahr/bitcoin/commits/pr31349/ While not everything has finished, some job including unit tests succeeded and the only failing job [ran out of space](https://github.com/fjahr/bitcoin/actions/runs/18109731912/job/51532955001) which seems unrelated. Am I missing something here?

💬 fjahr commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2389021877)
nit: Would be nice to add a comment here so people don't have to hunt down the commit description.

💬 fjahr commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2389363270)
nit: here I would also like a bit of documention, maybe adding a small section in `ci/readme.md` would be the right place for this?

📝 davidgumberg opened a pull request: "test: sock: Enable socket pair tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/33506)
Some `class Sock` tests were previously disabled because Windows lacks [`socketpair(2)`](https://man7.org/linux/man-pages/man2/socketpair.2.html). This PR adds a `CreateSocketPair()` helper for Windows which imitates [`socketpair()`](https://github.com/torvalds/linux/blob/1896ce8eb6c61824f6c1125d69d8fda1f44a22f8/net/unix/af_unix.c#L1830-L1859), and enables these test cases for Windows.

This change is generally an improvement, but is also broken out of a [branch](github.com/davidgumberg/bitc
...

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389620468)
"support" doesn't help me understand this:
> support
To bear the weight of, especially from below; keep from falling, sinking, or slipping.

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389636212)
I was just referring to the concerns of other reviewers about modifying production code - maybe a test could make sure we don't accidentally include this in the future. We have tests that start up `perf` tool, it shouldn't be too difficult to check if a debugger can attach to us in a test (though I wouldn't guard, I would want that test to fail if run with a debugger, that's how I would trust it) - but if you don't think it's useful, I understand.

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389639009)
> add a functional test that runs bitcoind to confirm that `-waitfordebugger` triggers this error message

Yes, it would be reassuring to me that it's not on by default.

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389640672)
Ah, so it's not the negative that we're guarding, in that case maybe:
```suggestion
attached = pid != 0;
```

📝 polespinasa opened a pull request: "RPC: add sendrawtransactiontopeer"
(https://github.com/bitcoin/bitcoin/pull/33507)
Adds a new RPC `sendrawtransactiontopeer`, which sends a single tx to a specific peer.
After the transaction is sent to the peer it is forgotten and not stored in the local mempool.

This rpc can serve several purposes. For example, it allows you to “spoof” an initial retransmission from a different "trusted" peer, making it appear as if the original participant was never aware of the transaction. It can also be useful in testing and simulation environments.



Solves #28636 and #21876 (P
...

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2389659427)
Even if the tx is already in the mempool `m_result_type` is never `ResultType::MEMPOOL_ENTRY` making it throw a `JSONTransactionError`.

This can be seen in the last test case in 5e4b8120af266026cfbc3ca29fc98ebb4fcf8f55.
This should not be the behavior, but I didn't find a solution yet.

💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2389659844)
This test case should be deleted as this is not the expected behavior, see https://github.com/bitcoin/bitcoin/pull/33507/files#r2389659427

💬 andrewtoth commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#issuecomment-3349669141)
Have you seen https://github.com/bitcoin/bitcoin/pull/29415? Is there a reason you would want this if we had private broadcast?

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2389662377)
Precompiled regexes are optimized to NFAs, but this one doesn't even backtrack so it's likely really fast and very easy to read.

But looking at https://github.com/catchorg/Catch2/blob/devel/src/catch2/internal/catch_debugger.cpp#L95 it seems to me that we don't even need to parse it, just check if it starts with zero or not, maybe something like:
```C++
for (std::string line; !attached && std::getline(status, line);) {
if (line.starts_with("TracerPid:")) {
attached = line.find
...

💬 pablomartin4btc commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2389746621)
nit: if you re-touch, same as `walletdb.cpp`, copyright dates needs to be updated...