💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138744578)
Add `AssertLockNotHeld(m_msg_process_queue_mutex)` as per `doc/developer-notes.md`:
> - Combine annotations in function declarations with run-time asserts in
function definitions:

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138747571)
The nested scope is unnecessary now, you can remove `{` and `}`.

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1138756727)
`s/Send/Sent/`
`s/received_bytes/sent_bytes/`

```suggestion
void AccountForSentBytes(const std::string& msg_type, size_t sent_bytes)
```

💬 SkybuckFlying commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472092677)
What is the difference between the two ? confused...

💬 hebasto commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472095887)
> What is the difference between the two ? confused...

Is your question about screenshots or about websites?

💬 SkybuckFlying commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472099581)
I read this:

https://bitcoin.stackexchange.com/questions/101495/is-bitcoin-org-or-bitcoincore-org-the-one-to-trust

So it seems bitcoin core developers moved to bitcoincore.org.

I see bitcoin.org is linking to an older version 22 ? Why is this ? any idea ?

💬 hebasto commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472103748)
> I see bitcoin.org is linking to an older version 22 ? Why is this ? any idea ?

Perhaps, bitcoin.org maintainers have their own view on how Bitcoin clients should be updated :)

💬 SkybuckFlying commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472105395)
I notice 22 freezes a lot, any idea if 23 is better ? :)

💬 hebasto commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472107459)
> I notice 22 freezes a lot, any idea if 23 is better ? :)

The most recent version, which is v24.0.1 for now, is better.

Can your initial issue be considered as resolved?

💬 SkybuckFlying commented on issue "Blocks remaining falls offscreen with dutch language setting.":
(https://github.com/bitcoin-core/gui/issues/721#issuecomment-1472112609)
I think not, it should be made such that it can display it properly... either re-size the screen such that it is default a little bit wider... or modify it somehow.

It wasn't clear I could resize it, perhaps I tried it and it didn't work.

I did try and re-size it, but I tried to resize the inner screen, not the outer screen.

So it's weird to have to re-size the outer screen to re-size the inner screen that makes 0% sense...

Perhaps a "normal" window/form should be used instead of thi
...

💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1138892649)
Alright, finally applied the change to the many occurrences in each commit. It should also be more readable now.

💬 mzumsande commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1137786018)
I think it might be possible that in between the spot where `txinfo` is queried, and the time we call `m_mempool.GetIter()` some other thread could remove the tx from the mempool (because no lock is held), so that this assert would be hit.

💬 mzumsande commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1138793203)
Having a mix of float (`limit`) and double (`fractional_peer`) is probably not ideal.

💬 mzumsande commented on pull request "p2p: Fill reconciliation sets and request reconciliation (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/26283#discussion_r1138809864)
`wtxid` is unused

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1472282805)
Updated 142718890e718397e0026c315c8940102b9657ce -> 4b31ef43fa198a0618fd61da121bd6f59e8be7f6 ([pr26749.08](https://github.com/hebasto/bitcoin/commits/pr26749.08) -> [pr26749.09](https://github.com/hebasto/bitcoin/commits/pr26749.09), [diff](https://github.com/hebasto/bitcoin/compare/pr26749.08..pr26749.09)).

Addressed @ryanofsky's comments:
- https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1123317784
- https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1123429117

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1138986823)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1472282805).

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1138987783)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1472282805).

💬 RCasatta commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1472285629)
Yeah as I specified in the issue and as @TheCharlatan wrote more ram shouldn't be needed because txid are iterated sorted from leveldb.

I was also wondering why the `bitcoin-cli savetxoutset` API (same goes for `savemempool`) requires to specify a filename instead of writing to stdout that would offer better composition while maintaining the possibility to write to file with `>utxo.bin`

📝 hebasto converted_to_draft a pull request: "refactor: Use move semantics instead of custom swap functions"
(https://github.com/bitcoin/bitcoin/pull/26749)
This PR makes code more succinct and readable by using move semantics.

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1139007798)
Well, the [reason](https://api.cirrus-ci.com/v1/task/5641079790239744/logs/ci.log) of that change is to avoid using of `vChecks` after it was moved:
```
test/checkqueue_tests.cpp:166:13: error: 'vChecks' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
vChecks.resize(std::min(total, (size_t) InsecureRandRange(10)));
^
test/checkqueue_tests.cpp:168:21: note: move occurred here
control.Add(std::move(vChecks));
^
`
...

👋 hebasto's pull request is ready for review: "refactor: Use move semantics instead of custom swap functions"
(https://github.com/bitcoin/bitcoin/pull/26749)