💬 hebasto commented on pull request "Dialog for allowing the user to choose the change output when bumping a tx":
(https://github.com/bitcoin-core/gui/pull/700#discussion_r1486212254)
To make translators' life easier and to keep the context as large as possible, I suggest to use `QString::arg`. Also it will fix the `lint-qt-translation.py` warning.

💬 josibake commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#issuecomment-1938735308)
ACK https://github.com/bitcoin/bitcoin/pull/28987/commits/9a3c5c8697659e34d0476103af942a4615818f4e

looks good, also `RemoveTxs` is a much better name. Might be worth updating the description to mention the rename since the description ends up in the commit log.

💬 hebasto commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-1938735622)
@GBKS What do you think from a designer's point of view?

💬 hebasto commented on pull request "Check for private keys disabled before attempting unlock":
(https://github.com/bitcoin-core/gui/pull/773#issuecomment-1938750300)
> > Also, what is the expectation here, merge and backport this PR without backporting #28724? Because, after #28724, this isn't a problem anymore.
>
> 28724 is a much scarier change since it deletes things from the wallet database and I wasn't sure if people were comfortable with that. If we move forward with it, then that can supersede this PR.

I'm going to merge this to get this bugfix into 27.0 as https://github.com/bitcoin/bitcoin/pull/28724 remains untagged.

🤔 josibake reviewed a pull request: "rpc: Drop migratewallet experimental warning"
(https://github.com/bitcoin/bitcoin/pull/28037#pullrequestreview-1875330161)
ACK https://github.com/bitcoin/bitcoin/pull/28037/commits/f1684bb88a878eb3f54e945db39ea69b14256eef

Looks like all the known bugs have been fixed and most of the performance improvements have been merged. The only two outstanding performance improvements are #28987 and #26008, both of which seem close/RFM. Given that we are also including a warning that this process is expected to take longer, I think its safe to drop the warning even without #28987 and #26008 (obviously, better if we can also
...

💬 maflcko commented on pull request "Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog":
(https://github.com/bitcoin-core/gui/pull/762#issuecomment-1938757000)
Not sure. All info displayed on the About page is fixed details about the software itself, not about the current (test)chain. So modulating the icon or title bar seems confusing, but no strong opinion.

💬 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