💬 jonatack commented on pull request "init: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1297772924)
If you retouch, can remove this added newline.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297791137)
Yeah. Sure for both.
Not sure why I hardcoded the number..

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297796093)
> Was it just outdated already? I'm confused by what NODE_NONE meant here before this change.

It was talking about the input `ServiceFlags`. Not the `GetDesirableServiceFlags()` output.

Basically, we expect to have all the hardcoded seeds supporting `NODE_NETWORK` and `NODE_WITNESS`.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297797152)
yeah sure.

💬 furszy commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297802187)
> The comment ("shortcut for...") was helpful for me, any reason to drop it?

That was because the method was moved to the net interface ([see](https://github.com/bitcoin/bitcoin/blob/1428b7f4e8c648fdc68ba5744d640d120d5d78eb/src/net.h#L673)). And there, this is no longer a shortcut. It is the only available method.

👍 theStack approved a pull request: "crypto: more `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100#pullrequestreview-1583609952)
re-ACK 57cc136282c38825e97bbf85728df4bdf1ccc648

(CI failure is unrelated)

💬 ajtowns commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297864956)
Okay, you nerdsniped me successfully. https://github.com/ajtowns/bitcoin/blob/9f332c93bcf7c2a1a4796dda4d2304014b10037e/src/rpc/util.cpp#L684-L705

💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1297922908)
Unique for any tx and strictly monotonically increasing for child txs would also be violated for accepting packages to the mempool, I think.

💬 ajtowns commented on issue "Auto detect IPv6 connectivity":
(https://github.com/bitcoin/bitcoin/issues/28061#issuecomment-1683257963)
> That is the case currently as well: `ifconfig eth0 up/down` and IPv4 or IPv6 connectivity comes and goes. Tor or I2P or CJDNS router restart - same, at runtime.

Sure: but the way that works is if you configure bitcoind for tor/i2p/etc, and if it goes down for a while, bitcoind will continue working once it comes back up again (and emit log messages about it in the meantime). ipv6 handling shouldn't be worse than that.

📝 russeree opened a pull request: "removed StrFormatInternalBug quote delimitation"
(https://github.com/bitcoin/bitcoin/pull/28291)
This PR rectifies an unnecessary set of quotes delimiting the contents of ```StrFormatInternalBug```. This is a follow up to @MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493. The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format.

The results can be seen below.

Previously
![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-
...

💬 ajtowns commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#discussion_r1297949804)
Done

💬 ajtowns commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1683303626)
First commit from this has been merged via #28244; rebased on top of that. See #28285 or #28289 for CI failure.

📝 russeree converted_to_draft a pull request: "rpc: removed StrFormatInternalBug quote delimitation"
(https://github.com/bitcoin/bitcoin/pull/28291)
This PR rectifies an unnecessary set of quotes delimiting the contents of ```StrFormatInternalBug```. This is a follow up to @MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493. The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format.

The results can be seen below.

Previously
![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-
...

💬 stratospher commented on pull request "crypto: more `Span<std::byte>` modernization & follow-ups":
(https://github.com/bitcoin/bitcoin/pull/28100#issuecomment-1683310137)
ACK 57cc136.

👋 russeree's pull request is ready for review: "rpc: removed StrFormatInternalBug quote delimitation"
(https://github.com/bitcoin/bitcoin/pull/28291)

💬 russeree commented on pull request "Bugfix: RPC: Remove quotes from non-string oneline descriptions":
(https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297992744)
Ref. https://github.com/bitcoin/bitcoin/pull/28291

💬 ajtowns commented on pull request "rpc: remove one more quote from non-string oneline description":
(https://github.com/bitcoin/bitcoin/pull/28289#issuecomment-1683346423)
utACK 239431444216850b63ecf01c3b5c5d6d24230d08

💬 ajtowns commented on pull request "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation":
(https://github.com/bitcoin/bitcoin/pull/28287#issuecomment-1683351664)
Concept ACK. Probably should add some basic functional tests to rpc_net.py with a `P2PInterface` checking that it works -- what happens if `msg_type` is 0 bytes, 13 bytes (`> protocol.h:COMMAND_SIZE`); what if `msg` is empty or more than 4MB (`net.h:MAX_PROTOCOL_MESSAGE_LENGTH`) or 32MB (`serialize.h:MAX_SIZE`)?

💬 kevkevinpal commented on pull request "doc, refactor: changing -torcontrol help to specify that a default port is used":
(https://github.com/bitcoin/bitcoin/pull/28101#discussion_r1298005740)
removed new line and updated title in [9a84200](https://github.com/bitcoin/bitcoin/pull/28101/commits/9a84200cfc994eebf38c46919b20e0c0261799ae)

💬 kevkevinpal commented on pull request "test: fix 'unknown named parameter' test in `wallet_basic`":
(https://github.com/bitcoin/bitcoin/pull/28288#issuecomment-1683375733)
ACK [d9ef627](https://github.com/bitcoin/bitcoin/pull/28288/commits/d9ef627af771987c134e72d8358798e6f376ffdf)

💬 MarcoFalke commented on pull request "rpc, test: add `sendmsgtopeer` rpc and a test for net-level deadlock situation":
(https://github.com/bitcoin/bitcoin/pull/28287#discussion_r1298027612)
```suggestion
thread1 = threading.Thread(target=node1.send_message, args=(0, "unknown", rand_msg))
```

Not sure if it works, but maybe you can drop the wrapper function?