💬 paulkania commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#discussion_r1141172272)
not sure what this comment means.
"ever" could be a misspelt "every". If that's the case, best to change it to from 'every' to 'all the'

💬 pinheadmz commented on issue "`getblocktemplate` returns a standard P2PKH with 0 sigops (testnet)":
(https://github.com/bitcoin/bitcoin/issues/24255#issuecomment-1475012160)
It's not unusual for a transaction to have 0 block-limit sigops.

The tx in your example has 9 inputs which all spend from legacy p2pkh UTXOs. In each of those spends, the `OP_CHECKSIG` is literally in the output of the previous transaction, and the sigop cost was "paid for" by the funding transactions. The inputs only contain public keys and signatures (no sigops). The single output of that tx is a P2WPKH. The signature operation required to spend that output will be "paid for" by the *spendi
...

💬 theStack commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1141224091)
In commit 225023d9e3ea5a89037ef8a4f4404a0fdd3f1cf7: This function is not used anywhere in this file and hence can be removed (or rather moved to rfc8439.cpp, see other review comment).
```suggestion
```

💬 theStack commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1141226425)
In commit 7a9d2fb8cf11eef8b6bd87ad1066df374bf3b620: Is there any reason to prefix the define and the function with "rfc8439"? Note that the point here is to let the build system detect if the function `timingsafe_bcmp` is available on the system (if yes, the define `HAVE_TIMINGSAFE_BCMP` is set) and only use the custom implementation if we need to. With this current construct, we would always use the custom one, since `RFC8439_TIMINGSAFE_BCMP` would obviously never be set. This should probably j
...

💬 ishaanam commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#discussion_r1141227595)
Done

💬 ishaanam commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#discussion_r1141227618)
Yes, I've moved it.

💬 ishaanam commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#discussion_r1141229127)
> I see that you’re just moving this code, but could you add a little more explanation here?

Done as per your suggestion above.
> I would suspect that we might also be able to drop witness_utxos on non-segwit inputs when we do have the corresponding non_witness_utxos,

I don't think `witness_utxo`s should ever get added to non-segwit inputs.
> and vice versa, drop the non_witness_utxos on segwit inputs, if we do have the witness_utxos for them.

Currently I'm not sure, though in https:
...

💬 theStack commented on pull request "Remove confusing "Dust" label from coincontrol dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1475063342)
@furszy: Thanks, good catch, completely missed that! Force-pushed to remove that as well. (FWIW, I _do_ think having a dust label in the sendcoins dialog would be useful, but only if it is always displayed, independently of whether coin control is used).

💬 russeree commented on issue "Improve the error message when an address cannot be parsed because it is for a different network ":
(https://github.com/bitcoin/bitcoin/issues/26290#issuecomment-1475164448)
This was quite fun to work on and I have submitted a PR for this. This was quite a difficult task to do and there could be room for improvement but it would require modifications to bech32.cpp for better error decoding.

These are my results.

Base58

Bech32

🚀 fanquake merged a pull request: "p2p: Improve diversification of new connections"
(https://github.com/bitcoin/bitcoin/pull/27264)

🚀 fanquake merged a pull request: "test: check that sigop limit also affects ancestor/descendant size (27171 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/27265)

💬 fanquake commented on pull request "refactor: remove unused param from legacy pubkey interface":
(https://github.com/bitcoin/bitcoin/pull/27274#discussion_r1141346046)
```suggestion
if (!CanGetAddresses(/*internal=*/ false)) {
```

📝 Sjors opened a pull request: "Log successful AcceptBlockHeader()"
(https://github.com/bitcoin/bitcoin/pull/27276)
Knowing when a header was first seen may help distinguish between a regular reorg and a selfish mining or eclipse attack.

🚀 fanquake merged a pull request: "refactor: wallet, do not translate init options names"
(https://github.com/bitcoin/bitcoin/pull/25666)

💬 Bushstar commented on pull request "refactor: remove unused param from legacy pubkey interface":
(https://github.com/bitcoin/bitcoin/pull/27274#discussion_r1141353847)
Updated as per the suggestion.

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1475248494)
Updated 54d8644dce180197fa657e9b68d6de63cf4c8072 -> 9f5600742c65a2421c9fe5a2a2670e86a25ef696 ([removeBlockstorageArgs_9](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_9) -> [removeBlockstorageArgs_10](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_9..removeBlockstorageArgs_10)) to split a commit moving methods and instantiating a block manager in zmq, and replacing the
...

📝 Sjors opened a pull request: "Move tx enqueue messages to mempool log"
(https://github.com/bitcoin/bitcoin/pull/27277)
When running with `-debug=validation` the log is filled with:

```
2023-03-19T12:46:01Z [validation] Enqueuing TransactionAddedToMempool: txid=37c8e1ef87d75a67fbaf44e116018fbc70d9c4ef0e27e2ae56861395101f9b8e wtxid=24e2d76012f9cde451d085ead4feab136feddae6cb55d37221a81aac6c0eaf42
2023-03-19T12:46:01Z [validation] TransactionAddedToMempool: txid=37c8e1ef87d75a67fbaf44e116018fbc70d9c4ef0e27e2ae56861395101f9b8e wtxid=24e2d76012f9cde451d085ead4feab136feddae6cb55d37221a81aac6c0eaf42
```

These l
...

💬 dergoegge commented on pull request "Log successful AcceptBlockHeader()":
(https://github.com/bitcoin/bitcoin/pull/27276#discussion_r1141357586)
```suggestion
LogPrint(BCLog::VALIDATION, "added new block header %s\n", hash.ToString());
```

We have an option for logging source locations `-logsourcelocations`.

💬 Sjors commented on pull request "Log successful AcceptBlockHeader()":
(https://github.com/bitcoin/bitcoin/pull/27276#discussion_r1141360805)
Done!

💬 Sjors commented on pull request "Move tx enqueue messages to mempool log":
(https://github.com/bitcoin/bitcoin/pull/27277#issuecomment-1475261120)
Still investigating the linter issue…

```
src/validationinterface.cpp: Expected 0 argument(s) after format string but found 1 argument(s): LogPrint(category, fmt "\n", __VA_ARGS__)
```