💬 marcofleon commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989708089)
Makes sense, thanks
(https://github.com/bitcoin/bitcoin/pull/32025#discussion_r1989708089)
Makes sense, thanks
💬 instagibbs commented on pull request "validation, fix: Use wtxid instead of txid in `CheckEphemeralSpends`":
(https://github.com/bitcoin/bitcoin/pull/32025#issuecomment-2715039398)
ACK https://github.com/bitcoin/bitcoin/pull/32025/commits/e637dc2c01c3b566e6c51c911c5881a8d206c924
Places the result in the map under txid, improving the reported error in certain cases, and typing future proofs against regressions automagically.
(https://github.com/bitcoin/bitcoin/pull/32025#issuecomment-2715039398)
ACK https://github.com/bitcoin/bitcoin/pull/32025/commits/e637dc2c01c3b566e6c51c911c5881a8d206c924
Places the result in the map under txid, improving the reported error in certain cases, and typing future proofs against regressions automagically.
💬 hebasto commented on issue "build: ccache doesn't hit across build dirs":
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2715048334)
> > This interpretation of the Ccache documentation does not describe the actual behaviour on my different systems. I read "base_dir matches the CWD" as "base_dir equals the CWD". However, I might be wrong.
>
> You are probably right but this behavior does not seem to make sense or correspond to documentation of base_dir. It is probably ok for us to force CCACHE_NOHASHDIR, but it seems like it would be safer if ccache just detected conditions it should and shouldn't hash CWD correctly itself. C
...
(https://github.com/bitcoin/bitcoin/issues/31994#issuecomment-2715048334)
> > This interpretation of the Ccache documentation does not describe the actual behaviour on my different systems. I read "base_dir matches the CWD" as "base_dir equals the CWD". However, I might be wrong.
>
> You are probably right but this behavior does not seem to make sense or correspond to documentation of base_dir. It is probably ok for us to force CCACHE_NOHASHDIR, but it seems like it would be safer if ccache just detected conditions it should and shouldn't hash CWD correctly itself. C
...
💬 furszy commented on pull request "Wallet: "listreceivedby*" fix":
(https://github.com/bitcoin/bitcoin/pull/30972#discussion_r1989789588)
Since the `mapTally` loading procedure already performs the `IsMine` check, why can't we just use that instead of executing it again?
It seems to me that if an element exists in `mapTally`, we can be certain that everything inside it belongs to the wallet. Additionally, this element also includes an `nAmount` field. So, ideally, we could decouple the `mapTally` existence check from the "include empty" check, which would eliminate the second `IsMine()` call.
(https://github.com/bitcoin/bitcoin/pull/30972#discussion_r1989789588)
Since the `mapTally` loading procedure already performs the `IsMine` check, why can't we just use that instead of executing it again?
It seems to me that if an element exists in `mapTally`, we can be certain that everything inside it belongs to the wallet. Additionally, this element also includes an `nAmount` field. So, ideally, we could decouple the `mapTally` existence check from the "include empty" check, which would eliminate the second `IsMine()` call.
💬 achow101 commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1989790658)
This file has a bunch of these parentheticals that I think are also the pronunciations.
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1989790658)
This file has a bunch of these parentheticals that I think are also the pronunciations.
⚠️ BullishNode opened an issue: "Allow sending untrusted utxos in the sendtoaddress api"
(https://github.com/bitcoin/bitcoin/issues/32034)
### Please describe the feature you'd like to see added.
We would like to be able to send untrusted utxos via the sendtoaddress API.
Currently, we have to go through the createrawtransaction flow to achieved this.
While I understand why it may be a sane default to have not to allow that, there is also very good reasons. Bitcoin Core may assume that a coin is "untrusted" because it is unconfirmed and is not change, but it doesn't know that this coins is actually coming from a separate wallet t
...
(https://github.com/bitcoin/bitcoin/issues/32034)
### Please describe the feature you'd like to see added.
We would like to be able to send untrusted utxos via the sendtoaddress API.
Currently, we have to go through the createrawtransaction flow to achieved this.
While I understand why it may be a sane default to have not to allow that, there is also very good reasons. Bitcoin Core may assume that a coin is "untrusted" because it is unconfirmed and is not change, but it doesn't know that this coins is actually coming from a separate wallet t
...
💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715136288)
Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715136288)
Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
💬 achow101 commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715139938)
> Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
Makes sense if you don't want the wallet?
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715139938)
> Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
Makes sense if you don't want the wallet?
💬 volkanural commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715146758)
> > @kallewoofBIP-322'yi uygulayan başka bir cüzdan olup olmadığını biliyor musunuz, böylece uygulamayı karşılaştırabiliriz?
> > Bu, BIP'e (ve bu PR'ye) test vektörleri eklemek için gerçekten iyi bir zamandır.
>
> Burada bir uygulamam var: [bitcoin-s/bitcoin-s#3823](https://github.com/bitcoin-s/bitcoin-s/pull/3823)
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715146758)
> > @kallewoofBIP-322'yi uygulayan başka bir cüzdan olup olmadığını biliyor musunuz, böylece uygulamayı karşılaştırabiliriz?
> > Bu, BIP'e (ve bu PR'ye) test vektörleri eklemek için gerçekten iyi bir zamandır.
>
> Burada bir uygulamam var: [bitcoin-s/bitcoin-s#3823](https://github.com/bitcoin-s/bitcoin-s/pull/3823)
💬 volkanural commented on pull request "BIP-322 basic support":
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715147604)
open
(https://github.com/bitcoin/bitcoin/pull/24058#issuecomment-2715147604)
open
💬 GarmashAlex commented on pull request "tests: Improve stderr validation in test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2715155554)
@yancyribbens Sorry, my bad. I removed TODO
(https://github.com/bitcoin/bitcoin/pull/31966#issuecomment-2715155554)
@yancyribbens Sorry, my bad. I removed TODO
💬 hebasto commented on pull request "qt: 29.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1989825367)
There are too many of them. The entire Thai (th) translation update has been discarded.
(https://github.com/bitcoin/bitcoin/pull/32004#discussion_r1989825367)
There are too many of them. The entire Thai (th) translation update has been discarded.
📝 VolodymyrBg opened a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin/bitcoin/pull/32035)
This pull request adds a new field `addressList` to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the `address` field was being abused for this purpose, as noted
in a TODO comment.
The changes include:
- Adding a new `addressList` field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updat
...
(https://github.com/bitcoin/bitcoin/pull/32035)
This pull request adds a new field `addressList` to the SendCoinsRecipient class
to properly handle multiple addresses from unauthenticated payment requests.
Previously, the `address` field was being abused for this purpose, as noted
in a TODO comment.
The changes include:
- Adding a new `addressList` field to SendCoinsRecipient
- Updating the serialization methods to handle the new field
- Modifying PaymentServer to populate the new field when multiple addresses
are present
- Updat
...
💬 hebasto commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814)
> > Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
>
> Makes sense if you don't want the wallet?
```
$ make -C depends NO_SQLITE=1
$ cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
-- The CXX compiler identification is GNU 14.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-
...
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715187814)
> > Building depends with `NO_SQLITE=1` makes no sense now, doesn't it?
>
> Makes sense if you don't want the wallet?
```
$ make -C depends NO_SQLITE=1
$ cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
-- The CXX compiler identification is GNU 14.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-
...
💬 hebasto commented on pull request "qt: Add addressList field to SendCoinsRecipient for multiple addresses":
(https://github.com/bitcoin/bitcoin/pull/32035#issuecomment-2715197889)
Please re-open this PR in https://github.com/bitcoin-core/gui.
(https://github.com/bitcoin/bitcoin/pull/32035#issuecomment-2715197889)
Please re-open this PR in https://github.com/bitcoin-core/gui.
✅ hebasto closed a pull request: "qt: Add addressList field to SendCoinsRecipient for multiple addresses"
(https://github.com/bitcoin/bitcoin/pull/32035)
(https://github.com/bitcoin/bitcoin/pull/32035)
💬 mzumsande commented on issue "Fully validated AssumeUTXO starts revalidating after restart":
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715268576)
> _23.5 more minutes_
That's way longer than I thought it would take but I guess the UTXO set has grown a lot over the last years.
My idea of a fix would be to set some kind of DB flag such as (`DB_ASSUMEUTXO_VALIDATED`) in the background chainstate after the assumeutxo hash was successfully validated the first time, and then skip the check at next startup if that flag was set. I think I'll try that out / open a PR if it works.
(https://github.com/bitcoin/bitcoin/issues/32029#issuecomment-2715268576)
> _23.5 more minutes_
That's way longer than I thought it would take but I guess the UTXO set has grown a lot over the last years.
My idea of a fix would be to set some kind of DB flag such as (`DB_ASSUMEUTXO_VALIDATED`) in the background chainstate after the assumeutxo hash was successfully validated the first time, and then skip the check at next startup if that flag was set. I think I'll try that out / open a PR if it works.
💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2715298456)
> > Perhaps @purpleKarrot could suggest a better approach?
>
> CPack?
https://github.com/bitcoin/bitcoin/blob/dbc89b604c4dae9702f1ff08abd4ed144a5fcb76/cmake/module/Maintenance.cmake#L47-L48 :)
(https://github.com/bitcoin/bitcoin/pull/32019#issuecomment-2715298456)
> > Perhaps @purpleKarrot could suggest a better approach?
>
> CPack?
https://github.com/bitcoin/bitcoin/blob/dbc89b604c4dae9702f1ff08abd4ed144a5fcb76/cmake/module/Maintenance.cmake#L47-L48 :)
💬 Sjors commented on pull request "Require sqlite when building the wallet":
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715317205)
I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?
(https://github.com/bitcoin/bitcoin/pull/31961#issuecomment-2715317205)
I could change depends here, but maybe it's easier to do that in the final PR that drops BDB itself?
💬 hebasto commented on pull request "cmake: Check for `makensis` and `zip` tools before using them for optional `deploy` targets":
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1989903555)
> This seems far too verbose...
Having an optional dependency, the absence of which is revealed during the configuration stage, is quite unusual and requires better communication to the user.
> This seems ... exposing implementation details
`MAKENSIS_EXECUTABLE` is a cache variable that the user is expected to set when necessary, like any other cache variable. It cannot be considered as "implementation details".
> we don't do this for any other dep
Other deps are handled during th
...
(https://github.com/bitcoin/bitcoin/pull/32019#discussion_r1989903555)
> This seems far too verbose...
Having an optional dependency, the absence of which is revealed during the configuration stage, is quite unusual and requires better communication to the user.
> This seems ... exposing implementation details
`MAKENSIS_EXECUTABLE` is a cache variable that the user is expected to set when necessary, like any other cache variable. It cannot be considered as "implementation details".
> we don't do this for any other dep
Other deps are handled during th
...