Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156453195)
fd8cf58d736ed9614f46b4f5b3c92f71ff9c46d4
nit
```suggestion
const int operator() (const OutputGroup& group1, const OutputGroup& group2)
```
💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156500169)
Hmm, what about something like this https://github.com/furszy/bitcoin-core/commit/0ddd8f4e2672f4826b87b1cfda617a3c15caabea
💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156501392)
it's the maximum allowed transaction weight, not only for the inputs.
💬 furszy commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1495056232)
Thanks aureleoules for the feedback.

> Sidenote, It seems this chunk of code is unused.

That chunk of code is not in master, #27227 already cleaned it.
💬 aureleoules commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#issuecomment-1495134375)
> That chunk of code is not in master, https://github.com/bitcoin/bitcoin/pull/27227 already cleaned it.

Ah great, I'm not up-to-date with all the recently merged PRs, it's been a while since I reviewed any!
💬 theStack commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156541825)
in commit 1284223691127e76135a46d251c52416104f0ff1: this doxygen comment about the return-value has to be adapted w.r.t. `std::nullopt`
💬 theStack commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156571087)
> it's the maximum allowed transaction weight, not only for the inputs.

I don't think so? The coin selection algos don't have any knowledge about the additional data of the to-be-created tx (static header size + outputs), so they can only work with a weight limit on the inputs.
📝 meglio opened a pull request: "doc: Add example of how to mix private and public keys in descriptors"
(https://github.com/bitcoin/bitcoin/pull/27414)
It took me quite a while to understand how to use both xpriv and public keys in a single descriptor. The trick was to use xprv key + derivation path. As a newbie to Bitcoin Core, I had to consult stackexchange to understand that.

Adding an example would make it easier to "gotcha" for the reader.
⚠️ kylezs opened an issue: "-fallbackfee should apply to estimatesmartfee"
(https://github.com/bitcoin/bitcoin/issues/27415)
### Please describe the feature you'd like to see added.

`-fallback` fee is used by `sendtoaddress` to provide a fallback fee when sending transactions. However, this does not apply to estimatesmartfee, which still returns the error.

### Is your feature related to a problem, if so please describe it.

The reason for this is because it's difficult to test client applications on regtest, that would use `estimatesmartfee` on mainnet, but because it requires a lot of transactions before `estimates
...
⚠️ Satoshingmx opened an issue: "Bitcoin.org/https://www.bitcoin.org/www.bitcoin.org"
(https://github.com/bitcoin/bitcoin/issues/27416)
Bitcoin or Bitcoin/Bitcoin are not with any way or form part of Bitcoin.org aka Satoshi nakamoto has returned to
Fix many issues that where cause by updates unauthorized and should never been allowed by GitHub. And this is clear by doing something that would not pass but by removing and updating changes not allowed where allowed so Satoshi Nakamoto will show POW and finally take command over all bitcoin.org and exe.
💬 S3RK commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#discussion_r1156788080)
typo: to sign a transaction with the first ~~signature~~ key
💬 S3RK commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#issuecomment-1495422350)
ACK 42107710a35dd4b489757dd9317251f4b27cd3be
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156844031)
I haven't written a test for this, so all the following is not verified and just my guess based on the code understanding.

This is not a regression in this particular PR, but this could lead to misleading errors in a situation when a proper solution exists, but our coin selection failed to find it and only found solution that exceeded weight.
Let's imagine following scenario:
- UTXO pool: two big coins 50BTC, a ton of 0.001BTC.
- Target amount: 90BTC
- Proper solution: just take two big c
...
💬 S3RK commented on pull request "wallet: coin selection, don't return results that exceed the max allowed weight":
(https://github.com/bitcoin/bitcoin/pull/26720#discussion_r1156851716)
I wrote #24580 at some point to capture known to me inefficiencies of current coin selection. The scenario I described above is very close to case no.3 in `test_one_big_and_many_small_coins`
👍 Sjors approved a pull request: "script: add description for the functionality of each opcode"
(https://github.com/bitcoin/bitcoin/pull/27109)
re-ACK 86b43d7fdd086c308f58414f3718a9cbf106bd05
💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1156881801)
I see. In that case, just add what you just wrote as a comment.
💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#discussion_r1156884391)
My main point was to clarify the comment "future, introduce a checkbox to customize this value". It could be done as source code comments or a Github issue.
💬 Sjors commented on pull request "wallet, gui: bugfix, getAvailableBalance skips selected coins":
(https://github.com/bitcoin/bitcoin/pull/26699#issuecomment-1495532740)
ACK 68eed5df8656bed1be6526b014e58d3123102b03

Remaining feedback is only about source code comments.
💬 meglio commented on pull request "doc: Add example of how to mix private and public keys in descriptors":
(https://github.com/bitcoin/bitcoin/pull/27414#discussion_r1156887830)
Thank you, fixed.