📝 dergoegge opened a pull request: "net, refactor: Privatise CNode send queue"
(https://github.com/bitcoin/bitcoin/pull/27407)
The send queue members on `CNode` should not be part of the public interface. This PR makes all of them private and creates a clear interface for the send queue.

The interface after this PR consists of:
* `CNode::PushMessage` for appending a message onto the send queue
* `CNode::SocketSendData` for pushing as many messages from the send queue as possible onto the wire
* `CNode::IsSendQueueEmpty` for checking if the send queue is empty
* (`CNode::TestOnlyClearSendQueue` a test-only utility
...

🚀 fanquake merged a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)

💬 fanquake commented on pull request "compat: move (win) S_* defines into bdb":
(https://github.com/bitcoin/bitcoin/pull/26832#issuecomment-1494353437)
Rebased for #27254.

💬 fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-1494357963)
Rebased now that #27381 is resolved.

⚠️ glitteryskye99 opened an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)
null

✅ fanquake closed an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)

:lock: fanquake locked an issue: "Lost"
(https://github.com/bitcoin/bitcoin/issues/27408)

💬 furszy commented on issue "`listtransactions` fails to list self-send transactions (for imported descriptor wallet)":
(https://github.com/bitcoin/bitcoin/issues/26096#issuecomment-1494423948)
> For self-send detection to work we need to distinguish between receiving and change addresses. For descriptor wallets we do that by checking `internal` flag on the descriptor. But `internal` flag could only be set on an `active` descriptor. Both your imported descriptors are not `active`, therefore we can't determine which outputs are change and which are receiving.

@S3RK, #25979 aims to fix that.
note: It's currently up-to-date with master but I should revisit it after that many months.

💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1494455306)
Thanks everyone for the interest, finally reviving this. Rebased and modernized the code (use static casts and named parameters, make use of batched RPC calls after #26656).

@naumenkogs
> If we really want to check, we can measure how would fees differ by applying this change to historic blocks (?)

We'd also need historical snapshots of a mempool to do that. If someone has got some and is willing to test, i'd be very interested in seeing the results.

> Otherwise, it would help if @dar
...

💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1156064559)
Resolving this, i don't think we can get rid of this heuristic. https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1494455306 gives a counter example where it would lead to a false positive. Please let me know if you disagree.

💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1155886202)
8cd77302305a2387368aabe8744b1859caf9956a: what do you mean by this?

💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1155907956)
8cd77302305a2387368aabe8744b1859caf9956a: paging @ryanofsky: do you think we should move all this complexity here to the interface?

💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1156018577)
Here's a commit to handle the error: https://github.com/Sjors/bitcoin/commit/5711f8274b91a278dd9307972e6985dc96ba56cb

💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1155888778)
8cd77302305a2387368aabe8744b1859caf9956a: This seems more consistent with the style above:

```cpp
m_coin_control->m_allow_other_inputs = !m_coin_control->HasSelected();
```

Alternatively you could do `*m_coin_control` at the top of the function.

Maybe also add an `assert(m_coin_control)` at the top.

💬 furszy commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#issuecomment-1494553264)
Hey dergoegge, thanks for the feedback.

> Approach NACK for adding new globals. It is not worth going with a worse design to "rush in" an improvement for a feature that may or may not happen ([#26966](https://github.com/bitcoin/bitcoin/pull/26966)). I would prefer to go with a more sustainable approach from the get go, otherwise this will need to be refactored in the long run.

No rush here.
[#26966](https://github.com/bitcoin/bitcoin/pull/26966) is only one clear use case. It isn’t the ma
...

💬 furszy commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1156147572)
Basically to follow the same procedure as the send process (`SendCoinsDialog::PrepareSendText`), it's is at line 288-290.

Currently, the GUI disables the automatic selection of "other inputs" when the user manually selected coins (the process will only use those). But, in the future, that could also be customizable. The user might want to let the wallet add new inputs, even when the user had selected some, if there aren't enough funds to cover for the tx target amount.

💬 ismaelsadeeq commented on pull request "test: various `converttopsbt` check cleanups in rpc_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/27325#issuecomment-1494594347)
tested ACK afc2dd54848fa70ed408ae259420ff8648f21efc

I ran the test and it passed.

💬 furszy commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1156170345)
The `m_allow_other_inputs` flag is different to the `fAllowWatchOnly` flag.

`fAllowWatchOnly` set is actually redundant here as `privateKeysDisabled()` and `hasExternalSigner()` are wallet constants. It can be set only once, when the wallet is set in the GUI and don't be re-set again.

And, at the contraire, the `m_allow_other_inputs` flag can change as it depends on whether the user manually selected coins or not, which could be modified at any time. Thus why I would try to not set the coi
...

📝 ryanofsky opened a pull request: "Make GUI and CLI tools use the same datadir"
(https://github.com/bitcoin/bitcoin/pull/27409)
**This is based on #27302.** The non-base commits are:

- [`cb2f82421d5` bitcoin-wallet: make bitcoin-wallet tool load config file](https://github.com/bitcoin/bitcoin/pull/27409/commits/cb2f82421d557cfe835b686a6f25965ee190e329)
- [`04a7fca01d3` init: Allow bitcoin default datadir to point at an external datadir](https://github.com/bitcoin/bitcoin/pull/27409/commits/04a7fca01d330bd9d6f33bf01dd380a8f0f36604)
- [`3c2b5edbfbf` Make GUI and CLI tools use the same datadir](https://github.com/bitco
...

💬 ryanofsky commented on pull request "Make GUI and CLI tools use the same datadir":
(https://github.com/bitcoin/bitcoin/pull/27409#issuecomment-1494623027)
This is a draft PR and not fully implemented. Just opening it to share progress since it's taking longer than I expected to implement correctly