💬 sipa commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620413203)
@glozow You're right; I didn't think this through.

@instagibbs To a limited extent better linearization can help here (though within-chunk optimization isn't something I've been looking at, as it doesn't matter for mining/eviction) but I think you can construct more complex examples where even a "perfect" linearization results in grouping of things that should not pay for each other.

I'm starting to think that something closer to your idea here is right: trying ancestor sets of every transacti
...

✅ hebasto closed an issue: "Confusing/misleading "Dust:" label in coin selection dialog"
(https://github.com/bitcoin-core/gui/issues/699)

🚀 hebasto merged a pull request: "Remove confusing "Dust" label from coincontrol / sendcoins dialog"
(https://github.com/bitcoin-core/gui/pull/719)

💬 luke-jr commented on pull request "build: pass sanitize flags to instrument libsecp256k1 code":
(https://github.com/bitcoin/bitcoin/pull/27991#discussion_r1252148488)
Won't this lose the other `CFLAGS` currently passed by the user?

💬 luke-jr commented on pull request "p2p: gives seednode priority over dnsseed if both are provided":
(https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-1620432636)
If both are provided, I would expect both to be active in parallel, and complete even if the other succeeds...

🤔 luke-jr requested changes to a pull request: "init: adding check for : for -torcontrol flag"
(https://github.com/bitcoin/bitcoin/pull/28018#pullrequestreview-1513100420)
Concept NACK. Port isn't required (the default is 9051). Furthermore, even "1" is a valid value (it would be the same as 0.0.0.1:9051), though I suppose it wouldn't hurt to check for it specifically and reject it.

💬 luke-jr commented on pull request "exclude ipc scheme from port check":
(https://github.com/bitcoin/bitcoin/pull/28020#discussion_r1252163957)
`rfind` is a very weird choice here. Suggest using [`compare`](https://cplusplus.com/reference/string/string/compare/)

💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1620448305)
Building 8587bfbbbc33f2b5e3737c9afe42e6d09464064c fails with:

```
wallet/wallet.cpp: In function ‘std::shared_ptr<wallet::CWallet> wallet::CreateWallet(WalletContext&, const std::string&, std::optional<bool>, DatabaseOptions&, DatabaseStatus&, bilingual_str&, std::vector<bilingual_str>&)’:
wallet/wallet.cpp:342:34: error: ‘WALLET_FLAG_GLOBAL_HD_KEY’ was not declared in this scope
342 | wallet_creation_flags |= WALLET_FLAG_GLOBAL_HD_KEY;
| ^
...

👍 luke-jr approved a pull request: "Switch RPCConsole wallet selection to the one most recently opened/restored/created"
(https://github.com/bitcoin-core/gui/pull/696#pullrequestreview-1513117159)
utACK 99c0eb9701e71f16aa360a420b7e4851d5b92510

💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1620461536)
OK I've pushed a new set of changes which now disconnects nodes synchronously inside of `AttemptToEvictConnection`.

@Crypt-iQ I'd be curious if you still see these new changes as resolving the issue in #27843? I havent' gotten your test patch working to my satisfaction yet (or at least, I don't see positive eviction candidate selection during it so it wouldn't overflow `nMaxInbound` even without these changes).

👍 hebasto approved a pull request: "Switch RPCConsole wallet selection to the one most recently opened/restored/created"
(https://github.com/bitcoin-core/gui/pull/696#pullrequestreview-1513125185)
ACK 99c0eb9701e71f16aa360a420b7e4851d5b92510, tested on Ubuntu 23.04.

🚀 hebasto merged a pull request: "Switch RPCConsole wallet selection to the one most recently opened/restored/created"
(https://github.com/bitcoin-core/gui/pull/696)

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1620477978)
> Looks like a slightly non trivial rebase. For context, which PR's triggered it?

#24914 I think



> Building [8587bfb](https://github.com/bitcoin/bitcoin/commit/8587bfbbbc33f2b5e3737c9afe42e6d09464064c) on Ubuntu 23.04 with gcc 12.2.0 fails with:
>
> ```
> wallet/wallet.cpp: In function ‘std::shared_ptr<wallet::CWallet> wallet::CreateWallet(WalletContext&, const std::string&, std::optional<bool>, DatabaseOptions&, DatabaseStatus&, bilingual_str&, std::vector<bilingual_str>&)’:
> wa
...

💬 MarcoFalke commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1620483829)
Dropped the CCACHE_SIZE stuff and restored the initial change

👍 hebasto approved a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024#pullrequestreview-1513148988)
ACK fac14c4e498f2d38b8b337d4c145129d07403a6d.

Mentioning "timeouts" in the commit message looks unrelated.

🤔 ajtowns reviewed a pull request: "Fix potential network stalling bug"
(https://github.com/bitcoin/bitcoin/pull/27981#pullrequestreview-1513120096)
> The core issue is that whenever our optimistic send fails to fully send a message, we do subsequently not even select() for receiving; if it then turns out that sending is not possible either, no progress is made at all. To address this, the solution used in this PR is to still select() for both sending and receiving when an optimistic send fails, but skip receiving if sending succeeded, and (still) doesn't fully drain the send queue.

AIUI (correct me if I'm wrong!) the backpressure we do i
...

💬 ajtowns commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#discussion_r1252173948)
Would it make sense to introduce a method `bool CNode::WantsToSend() const { return !pnode->vSendMsg.empty(); }` method, and use that here and instead of returning a `pair<X, bool>` above?

🤔 fjahr reviewed a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1512960883)
Code review ACK 03dfe04075668aa3b99a5d6d68adfcecffca0593

(modulo my question about the rev file management)

I will also sync a node with this code and report if I see anything unusual in the coming days.

💬 fjahr commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1252074194)
I must either be missing something about this line or it might be unnecessary. If I understand correctly this is updating `m_undo_height_in_last_blockfile` when the next block is connected after the flush happened. However, why not update it right away when the flush happens? I tried that by removing this line it seems like all tests are still passing without it.

💬 dooglus commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1620521045)
> Do you happen to know what libevent version you were on?

It looks like version 2.1.12.

```
$ dpkg -l | grep libevent
ii libev4:amd64 1:4.33-1 amd64 high-performance event loop library modelled after libevent
ii libevent-2.1-7:amd64 2.1.12-stable-1 amd64 Asynchronous event notification library
ii libevent-core-2.1-7:amd64
...

💬 luke-jr commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#discussion_r1252222085)
Why not `UNKNOWN_DESCRIPTOR` instead?