Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ’¬ glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620375694)
> I don't think you need to special case individual transactions even, actually. Instead, use this, instead of chunking:

I'm not sure about using a group's aggregate feerate without checking their spending relationships, as it may allow unrelated transactions to pay for each other. For example:
```
A(1) B(3)
^ ^
C(100)
```
Where minfeerate is 2sat/vB. Imagine C is invalid (e.g. a fake child created to connect A and B).
The ancestor score-based linearization I was imagining (i
...
πŸ’¬ instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1252120078)
> download should be dedup when we receive the ancpkginfos ?

If it's in your orphanage it won't be fetched again. See `ReceivedAncPkgInfo`
πŸ’¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1620395050)
Nothing is perfect, but a linearizer would ideally pick `B` before `A`, yes. So you might get:

`B, A, C`
or
`B, C, A`

depending on if the strategy is greedy. "topo" sort may miss this, which is why we should probably be smarter than that?
πŸ‘ hebasto approved a pull request: "Remove confusing "Dust" label from coincontrol / sendcoins dialog"
(https://github.com/bitcoin-core/gui/pull/719#pullrequestreview-1513056670)
Looks good. ACK a582b4141f0756faa3793fb1c772898a984c83e4.
πŸ’¬ 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
...