💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1486244390)
Dropped.

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1486245188)
Dropped

💬 hebasto commented on pull request "Enable HW-accelerated implementations of SHA256 for MSVC builds":
(https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1938773252)
Rebased.

💬 hebasto commented on pull request "Fix SSE4.1-related issues":
(https://github.com/bitcoin/bitcoin/pull/28893#issuecomment-1938781874)
Rebased.

💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1938791206)
Rebased, but not much else changed given the lack of concept-enthusiasm.

Test commit is dropped since #28838 landed.

> I don't consider this a blocker for assumeutxo mainnet params

Agreed. If anywhere, it's a blocker for having _automatic_ and/or very easy UI based snapshot loading.

> Another question for me is: Why don't we have something similar when `assumevalid` is used?

That state is permanent, since we don't have a background check for `assumevalid`.

> Small suggestion,
...

📝 glozow opened a pull request: "v3 followups"
(https://github.com/bitcoin/bitcoin/pull/29424)
Addresses final comments from #28948:
- https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483769698
- https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1483776227
- https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484427635
- https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484467280
- A unit test similar to the one suggested in https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484531064
- https://github.com/bitcoin/bitcoin/pull/28948#discussio
...

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

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1486268465)
#29424replaced these 2 comments with a slightly more helpful one at the top of the block.

💬 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!