💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486268684)
#29424 added a unit test but slightly different from this one (I didn't like the package size being 1)

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486269325)
Actually, in #29424 I've added unit tests here instead of adding an `Assume` (which would cause those tests to crash). Sorry for the flip flop but should address the fact that these blocks aren't covered by tests.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486269443)
removed in #29424

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486269585)
Yes I think so.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486269770)
Removed in #29424

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486269845)
Fixed in #29424

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486270285)
@ismaelsadeeq that test looks good to me 👍 can I cherry-pick this into #29424?

🤔 furszy reviewed a pull request: "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/26008#pullrequestreview-1875313580)
Concept ACK

I have been considering the idea of using a probabilistic data structure instead of the deterministic one for the membership-test over the past few days. Mainly, because I'm a bit concerned about the unbounded cache size. I was going to share it today, but it requires more work and the local benchmarks show a substantial speed difference in favor of this approach.

So.. as feature freeze is close, will finish review in few hours. I believe this can be implemented simpler, and sh
...

💬 furszy commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#discussion_r1486226771)
This does not seems to be necessary. At this point (`LoadDescriptorScriptPubKeyMan`), the spkm should't contain any script.

💬 hebasto commented on pull request "Release `LockData::dd_mutex` before calling `*_detected` functions":
(https://github.com/bitcoin/bitcoin/pull/26781#issuecomment-1938797790)
Rebased to refresh the CI.

💬 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