π¬ 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
```
(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
...
(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
(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.
(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:
...
(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).
(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

(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)
(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)
(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)) {
```
(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.
(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)
(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.
(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
...
(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
...
(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`.
(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!
(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__)
```
(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__)
```
π¬ dergoegge commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1141365401)
Done!
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1141365401)
Done!
π¬ dergoegge commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#issuecomment-1475261198)
Rebased
(https://github.com/bitcoin/bitcoin/pull/27257#issuecomment-1475261198)
Rebased