💬 Ajeetprajapati315 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/e49d858aab9bfae2455d87226d9ebebbdf2c40c8#commitcomment-146902756)
BTC

💬 achow101 commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765908400)
Good point, removed.

💬 achow101 commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765908620)
It was unsigned when this PR was written 3 years ago lol.

Updated to be unsigned.

💬 achow101 commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765908738)
Fixed the whitespace to be consistent with our style guidelines

💬 achow101 commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765909311)
I've added a basic test for this and a release note fragment. The bulk of the test changes in previous commits is to setup for this though.

💬 tcharding commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765924789)
Cheers

💬 tcharding commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1765925066)
Nice!

💬 andrewtoth commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#issuecomment-2359737551)
re-ACK 4feaa28728442b0fd29a677d2b170a05fdf967c0

💬 andrewtoth commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1765988081)
I meant that the `HaveCoin` signature is not relevant to this PR. So, just modifying it by moving the brace to the next line and moving the `&` to be next to the type is making a style change to a line that doesn't need to be touched in this PR.

If a line must be changed for the PR anyways, then yes I agree the style of the entire line should be updated.

💬 furszy commented on pull request "Fix linking when configured with `-DENABLE_WALLET=OFF` (Qt 6)":
(https://github.com/bitcoin-core/gui/pull/837#discussion_r1766029532)
How the `RPCExecutor::request` method compiles when this line doesn't exist?

👍 tdb3 approved a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2314225218)
ACK 1365ee8e9c7a20aa63bcddb1a6d5843c05ff9330

This is a value-add that I hope gets more traction/review.
Left a few more minor nits.
As time allows, I may exercise additional descriptors.

💬 tdb3 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1766056564)
If this gets touched again, could add asserts to check that the unconfirmed tx has blank blockhash and -1 height. Could be left for a follow-up PR.

💬 tdb3 commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1766035266)
nit: if another update occurs, may want to use `STR_HEX` for `scriptpubkey_hex`

✅ LuizWT closed a pull request: "Optimize: convert trusted keys list to a set for better performance"
(https://github.com/bitcoin/bitcoin/pull/30925)

💬 LuizWT commented on pull request "Optimize: convert trusted keys list to a set for better performance":
(https://github.com/bitcoin/bitcoin/pull/30925#discussion_r1766112838)
> This is not even an equivalent expression. When you convert a list to a set, the order of the lines is lost. Therefore, accessing the first element with [0] could give you any of the lines randomly, not necessarily the first line from the original file. In this case you needed index 0 ( root ).

You're right. It makes no sense to use a set in this context. Thanks for pointing this out

💬 maflcko commented on pull request "test: generalize HasReason and use it in FailFmtWithError":
(https://github.com/bitcoin/bitcoin/pull/30921#issuecomment-2360050658)
> Is there a way to see complete force-push history?

GitHub sometimes(?) sends commit hashes by email, it was 8ffb79805d3589a86ca97e976b9d271cd8062ae8

💬 maflcko commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2360075066)
> Well, the approach seems to be similar with pruning:

Sure, if that is the case, the pull request description should mention it. Also, the error message could probably be updated, because it only mentions pruning, no AU. (Note to self: Reminds me of https://github.com/bitcoin/bitcoin/pull/26282)

💬 maflcko commented on issue "Race condition between ZMQ UpdateTip and getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/30862#issuecomment-2360121854)
> ### Expected behaviour
>
> `getblocktemplate` should return a result consistent with the information announced via ZMQ and correctly reflect the current chain tip.

So to clarify my previous reply. What you describe as "expected behavior" is impossible to achieve. A `C` message just contains the block hash of a previous `BlockConnected` event. While often this will be equal to the result of an RPC `getbestblockhash` or `getblocktemplate` done after receiving the `C` message, there isn't (
...

💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1766285389)
`FRESH` is definitely a [very confusing concept](https://github.com/bitcoin/bitcoin/blob/master/src/coins.h#L143-L151)
> the parent cache does not have this coin or that it is a spent coin in the parent cache

Maybe after https://github.com/bitcoin/bitcoin/pull/30673 we can find a better name (and description) for it

🤔 vasild reviewed a pull request: "Fix display issues for IPv6 proxy setup in Options Dialog (UI only, no functionality impact)"
(https://github.com/bitcoin-core/gui/pull/836#pullrequestreview-2314606389)
Approach ACK a7d8de94adc3e94ea33d698c307a87ab775c3d20