💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486279084)
Yes 👍

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#issuecomment-1938811986)
Thank you @josibake will address comments shortly!

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

I'll let people decide for themselves our professionalism or lack thereof.

I asked hundreds of comments ago to stop
using this PR as a venue for debating the finer points of the LN spec, obviously unheeded.
There's a BOLT repo, which is an appropriate place on github to discuss these things.

You are also free to lobby different projects to not use these primitives moving forward and push
whatever solutions you think are necessary, even consensus changes. Meanwhile people
wh
...

💬 theuni commented on pull request "build: Add missed definition for `AM_OBJCXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29362#issuecomment-1938901924)
@epiccurious What specifically are you ACKing here? Have you read the conversation above?

💬 instagibbs commented on pull request "v3 followups":
(https://github.com/bitcoin/bitcoin/pull/29424#issuecomment-1938910571)
ACK 6b161cb82a9766ef814f05e5b8f019f15d5ee14d

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1486367765)
> Would that execute `rpc_net.py` two times and both in v2 mode?

Yes, although it's not really related to this PR it's the same on master (just that `P2PConnection`s use v1 both times there).
I had thought about this before and was thinking of addressing it in a follow-up PR, which would also add a CI job for `--v2transport`.
The downside of dropping the `--v2transport` specific jobs from `test_runner.py` is that these would be executed a lot less by individuals running the test suite (even
...

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1486374392)
yes. Fixed both.

💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1938941846)
[b9912e2 ](https://github.com/bitcoin/bitcoin/commit/b9912e2df9767540ab7f53cb3ba37fd2e2491806)to [bf5662c](https://github.com/bitcoin/bitcoin/commit/bf5662c678455fb47c402f8520215726ddfe7a94): just small fixes to the last commit message.

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1486385523)
> If not I think its okay like this.

removed comments, how it's precisely used is an implementation detail outside of this code

💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#issuecomment-1938973409)
rebased

🤔 sr-gi reviewed a pull request: "test: adds outbound eviction functional tests, updates comment in ConsiderEviction"
(https://github.com/bitcoin/bitcoin/pull/29122#pullrequestreview-1875503633)
Thanks for the review @tdb3

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1486346242)
That's covered in the last part of `test_outbound_eviction_unprotected`

https://github.com/bitcoin/bitcoin/blob/edc14bc9da332cae39f8803db559b532b3c74e16/test/functional/p2p_eviction.py#L202-L218

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1486369882)
Protected peer actually implies `outbound-full-relay`. The comment you mentioned may be outdated. I may add an update to it to the PR.

https://github.com/bitcoin/bitcoin/blob/edc14bc9da332cae39f8803db559b532b3c74e16/src/net_processing.cpp#L2875-L2885

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1486391103)
The limit is tested in this test. We create 8 connections, from which only 4 are protected. If you mean that there are no unprotected peers that behave properly, that could be added. e.g.

4 protected, send initial headers, do nothing, don't get evicted
2 unprotected, keep in sync, don't get evicted
2 unprotected, do not keep up, get evicted

👍 vasild approved a pull request: "test: use v2 everywhere for P2PConnection if --v2transport is enabled"
(https://github.com/bitcoin/bitcoin/pull/29358#pullrequestreview-1875604974)
ACK bf5662c678455fb47c402f8520215726ddfe7a94

See also https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1485626063

💬 tdb3 commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1486417918)
Correct. >= rather than >.

💬 vasild commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#discussion_r1486432039)
Alright, "it's the same on master" that is a good point!

So, if we have `--v1transport` (for each individual test and for `test_runner.py`?) then what would happen if ran without either one of `--v1transport` or `--v2transport`?

💬 LarryRuane commented on pull request "bitcoin-cli help detail to show full help for all RPCs":
(https://github.com/bitcoin/bitcoin/pull/29163#issuecomment-1939061997)
> Is there a better UX option here? What about `bitcoin-cli helpdetail`?

I agree your suggestion would be better, or maybe even `bitcoin-cli -helpdetail` (with a dash) because as it is now, "detail" seems like the name of an RPC. I'll look into what it would take to do either of those, thanks for the suggestion.

👍 vasild approved a pull request: "Release `LockData::dd_mutex` before calling `*_detected` functions"
(https://github.com/bitcoin/bitcoin/pull/26781#pullrequestreview-1875650020)
ACK 38f93fe0cb680425f5fec7c794b39c0e1795f9dc

💬 sr-gi commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-1939071798)
I redefined how peers are split in `p2p_eviction:test_outbound_eviction_protected_mixed` to address https://github.com/bitcoin/bitcoin/pull/29122#discussion_r1485375517. Both commits can be squashed (leaving them as they are for now in case someone does not agree with the approach)