Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ‘ 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?
πŸš€ fanquake merged a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024)
πŸ’¬ luke-jr commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620528011)
Tag for backport?
πŸ’¬ fanquake commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620530627)
Not if this has (as I understand it) only been broken on master?
πŸ’¬ achow101 commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#discussion_r1252228384)
The descriptor isn't unknown. It's a corruption error as the id written in the database doesn't match the id that we calculate.
πŸ’¬ achow101 commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1620534607)
> Tag for backport?

No need, only affects master.
πŸ“ pablomartin4btc converted_to_draft a pull request: "httpserver, rest: improving URI validation"
(https://github.com/bitcoin/bitcoin/pull/27253)
This PR contains an isolated enhancement of the segfault bugfix https://github.com/bitcoin/bitcoin/pull/27468 (already merged), improving the way we handle the URI validation, which will be performed only once instead of on each query parameter of a rest service endpoint.
πŸ’¬ pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1620535396)
After a chat with @stickies-v where we were discussing different approaches on this enhancement and other details regarding fuzz testing, libevent functionality and each commit intention, I've decided to put this onto draft. Firstly we would need to define the purpose of the `httprequest` object, do we want/ need the `httprequest` obj to exist even with an invalid request?, as @stickies-v [raised his concerns before](https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1481163868), it seem
...