💬 maflcko commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#issuecomment-1937657442)
> Any conceivable downsides to removing it?

I can't see one, apart from the change consuming time to review it.

✅ sdaftuar closed a pull request: "Enforce incentive compatibility for all RBF replacements"
(https://github.com/bitcoin/bitcoin/pull/26451)

💬 sdaftuar commented on pull request "Enforce incentive compatibility for all RBF replacements":
(https://github.com/bitcoin/bitcoin/pull/26451#issuecomment-1937672476)
This work has obviously been superseded by the cluster mempool proposal, which solves these problems in a much better way than anything contemplated here.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-1937766372)
@epiccurious, p2p encryption "solves" the spying from intermediate routers on clearnet (aka man-in-the-middle). Tor, I2P and CJDNS solve that too. While this PR uses only Tor and I2P it would solve that problem. But there is more - it will as well solve issues with spying bitcoin nodes.

📝 vasild opened a pull request: "log: deduplicate category names and improve logging.cpp"
(https://github.com/bitcoin/bitcoin/pull/29419)
The code in `logging.cpp` needs to:
* Get the category name given the flag (e.g. `BCLog::PRUNE` -> `"prune"`)
* Get the flag given the category name (e.g. `"prune"` -> `BCLog::PRUNE`)
* Get the list of category names sorted in alphabetical order

Achieve this by using the proper std containers. The result is
* less code (the diff of the first commit is +62 / -129)
* faster code (to linear search and no copy+sort)
* more maintainable code (the categories are no longer duplicated in `LogC
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r1485605463)
Done in https://github.com/bitcoin/bitcoin/pull/29419, thanks for the suggestion!

📝 vasild opened a pull request: "test: extend the SOCKS5 Python proxy to actually connect to a destination"
(https://github.com/bitcoin/bitcoin/pull/29420)
If requested, make the SOCKS5 Python proxy redirect connections to a set of given destinations. Actually act as a real proxy, connecting the client to a destination, except that the destination is not what the client asked for.

This would enable us to "connect" to Tor addresses from the functional tests.

Plus a few other minor improvements in the test framework as individual commits.

---

These changes are part of https://github.com/bitcoin/bitcoin/pull/29415 but they make sense on th
...

📝 vasild opened a pull request: "net: make the list of known message types a compile time constant"
(https://github.com/bitcoin/bitcoin/pull/29421)
Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `g_all_net_message_types.size()` which can be used in future code to build other `std::array`s with that size.

---

This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would red
...

👍 epiccurious approved a pull request: "log: Nuke error(...)"
(https://github.com/bitcoin/bitcoin/pull/29236#pullrequestreview-1874393601)
utACK fa1d4f07c36dda3c72fb328918bddd7de062ef96.

💬 epiccurious commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1485621343)
nit - Noticed this when reviewing the changes. Since we're already changing the file - should Line 357/358 be a `LogError` not a `LogPrintf`?

💬 epiccurious commented on pull request "log: Nuke error(...)":
(https://github.com/bitcoin/bitcoin/pull/29236#discussion_r1485623720)
nit - Also noticed this when reviewing the changes. Since we're already changing the file - should Line 413/422 be a `LogError` not a `LogPrintf`?

💬 vasild commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1485626063)
`test/functional/test_runner.py` contains:

```py
'rpc_net.py',
'rpc_net.py --v2transport',
```

So, on `master`, when I execute `test_runner.py` it will run this test two times, once in v1 mode and once in v2 mode. What about if I run, with this PR, `test_runner.py --v2transport`? Would that execute `rpc_net.py` two times and both in v2 mode? If yes, then what about adding `--v1transport` option as well? Or maybe just change the above to just:

```py
'rpc_net.py',
```


...

🤔 vasild reviewed a pull request: "test: use v2 everywhere for P2PConnection if --v2transport is enabled"
(https://github.com/bitcoin/bitcoin/pull/29358#pullrequestreview-1874395839)
Approach ACK b9912e2df9767540ab7f53cb3ba37fd2e2491806

The code changes look good. Just the question below.

💬 vasild commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1485622195)
nit: in commit message of b9912e2df9767540ab7f53cb3ba37fd2e2491806 `test: enable v2 for python p2p depending on global --v2transport flag`:

* s/all connection/all connections/
* "before this test" - should this be "before this change"?

💬 vasild commented on pull request "addrman: delete addresses that don't belong to the supported networks":
(https://github.com/bitcoin/bitcoin/pull/29330#issuecomment-1937824083)
What about setting `preferred_net`

https://github.com/bitcoin/bitcoin/blob/7143d4388407ab3d12005e55a02d5e8f334e4dc9/src/net.cpp#L2682

if `-onlynet` is used? Surely it is a waste to select an unreachable address:

https://github.com/bitcoin/bitcoin/blob/7143d4388407ab3d12005e55a02d5e8f334e4dc9/src/net.cpp#L2695-L2697

That would skew the outcome though. If addrman has 60% IPv4, 25% Tor and 15% I2P addresses right now it is more likely to select Tor over I2P. If we run with `-onlynet=tor
...

⚠️ vostrnad opened an issue: "Assertion chainman().ActiveChain()[height] fails on startup on memory-constrained system"
(https://github.com/bitcoin/bitcoin/issues/29422)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

bitcoind crashes on startup with this error: (not logged in debug.log)
```
node/interfaces.cpp:525 getBlockHash: Assertion `chainman().ActiveChain()[height]' failed.
Aborted (core dumped)
```
This is after trying to bring the node up after a previous crash from which I don't have the error as I was running in `-daemon` mode. The previous crash happened during IBD at height 140920.

##
...

💬 satsie commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1937844734)
RE:

>> ... this requires the "inverse" ...
>
> Ha, I did not realize this before! I am fine either way.

I voted for `bitcoin-cli getnetmsgstats '["message_type"]'` because as a node operator it's simpler to remember the dimensions I want (i.e. message) instead of knowing the RPC well enough to list every dimension I don't want (i.e. direction, network, connection type). Of course anyone using an RPC should read the docs :smile:

⚠️ DoctorBuzz1 opened an issue: "Move from a Static Dust Limit [of 330 / 546 sats] to a Variable Dust Limit [= to TxFee]..."
(https://github.com/bitcoin/bitcoin/issues/29423)
### Please describe the feature you'd like to see added.

This is the basis for my proposal....





### Is your feature related to a problem, if so please describe it.

Based on the number of Txs that have been unmovable, unspendable for nearly a solid year, it appears that the static dust limit is still permitting dust attacks, based on how the UTXO system works. Any entity with the ability
...

💬 ariard commented on issue "Update security.md with all PGP fingerprints":
(https://github.com/bitcoin/bitcoin/issues/29366#issuecomment-1937865544)
For everyone knowledge, Linux kernel has a separation between software security bugs and hardware ones for good historical reasons. Few years ago, in the handling of Meltdown / Spectre (the 1st gen of transient execution CPU vulnerabilities), early leaks of the issues did happen. Since then, the kernel and OS folks have put in place clearer security handling policy to coordinate vulnerability response, including expelling members from future security incidents who are not respecting embargo dead
...

💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1937866820)
@sdaftuar @achow101 @instagibbs

Congrats guys to ACK and merge a +1100 CoC PR a Friday evening with still standing objections on the very heart of the PR.
What a lack of professionalism…Still my pleasure to break v3 anti-pinning mitigations in the future, with a pinning toolkit.
If script kiddies don’t make it before me and go to savage the LN ecosystem, who would have naively thought they’re safe.

@glozow

I think there is medium difficulty follow-up to this PR to make pinning far ha
...