π¬ ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255181611)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255181611)
Done
π¬ ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255182702)
Done here and throughout the file
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255182702)
Done here and throughout the file
π¬ ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255183692)
Done
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255183692)
Done
π¬ ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255184775)
I've removed it.
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2255184775)
I've removed it.
π¬ ishaanam commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3156414008)
@achow101 thanks for the review!
> I think sendall is missing the weight check if the transaction is truc.
I've added the weight check in sendall for truc txs (including those spending unconfirmed truc children) and I have added two additional test cases for this.
(https://github.com/bitcoin/bitcoin/pull/32896#issuecomment-3156414008)
@achow101 thanks for the review!
> I think sendall is missing the weight check if the transaction is truc.
I've added the weight check in sendall for truc txs (including those spending unconfirmed truc children) and I have added two additional test cases for this.
π ryanofsky approved a pull request: "test: remove duplicated code in test/functional/wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33122#pullrequestreview-3089541339)
Code review ACK 6a7c0d3f874942a460bf5b13a107a2b21eb2e572. Nice test deduplication, thanks for following on this!
(https://github.com/bitcoin/bitcoin/pull/33122#pullrequestreview-3089541339)
Code review ACK 6a7c0d3f874942a460bf5b13a107a2b21eb2e572. Nice test deduplication, thanks for following on this!
π¬ ryanofsky commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2255218611)
re: https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000995
> addressing this in #33122
Note this change is now dropped from #33122, but could be good to revisit at some point (I still it's still a good idea).
(https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2255218611)
re: https://github.com/bitcoin/bitcoin/pull/32273#discussion_r2249000995
> addressing this in #33122
Note this change is now dropped from #33122, but could be good to revisit at some point (I still it's still a good idea).
π¬ darosior commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3156459304)
> But I'm asking specifically about the script execution cache.
Oh right! I think you are correct and `use_sighash_cache` should not be set for `CheckInputsFromMempoolAndCache`.
> Yeah, imo this would be easier to reason about (and concerns like mine above moot) with unified code paths.
My preference is still to not touch consensus code when unnecessary, and it seems appropriately setting `use_sighash_cache` in `CheckInputsFromMempoolAndCache` would achieve that. But if both Pieter and
...
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3156459304)
> But I'm asking specifically about the script execution cache.
Oh right! I think you are correct and `use_sighash_cache` should not be set for `CheckInputsFromMempoolAndCache`.
> Yeah, imo this would be easier to reason about (and concerns like mine above moot) with unified code paths.
My preference is still to not touch consensus code when unnecessary, and it seems appropriately setting `use_sighash_cache` in `CheckInputsFromMempoolAndCache` would achieve that. But if both Pieter and
...
π KaueTech opened a pull request: "Add Docker support with multi-arch build and user permissions handling"
(https://github.com/bitcoin/bitcoin/pull/33139)
# Add Docker support with multi-arch build and user permissions handling
This pull request introduces a Dockerfile and supporting scripts to enable containerized builds and deployment of Bitcoin Core, targeting multiple architectures (amd64, arm64, riscv64).
## Key changes include:
- Multi-stage Dockerfile to build Bitcoin Core from official releases, GitHub source, or local source with consistent environments.
- User and group creation inside the container to avoid running bitcoind as r
...
(https://github.com/bitcoin/bitcoin/pull/33139)
# Add Docker support with multi-arch build and user permissions handling
This pull request introduces a Dockerfile and supporting scripts to enable containerized builds and deployment of Bitcoin Core, targeting multiple architectures (amd64, arm64, riscv64).
## Key changes include:
- Multi-stage Dockerfile to build Bitcoin Core from official releases, GitHub source, or local source with consistent environments.
- User and group creation inside the container to avoid running bitcoind as r
...
π¬ pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156502301)
This has been proposed before and typically gets rejected:
https://github.com/bitcoin/bitcoin/pull/25681
https://github.com/bitcoin-core/packaging/issues/55
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156502301)
This has been proposed before and typically gets rejected:
https://github.com/bitcoin/bitcoin/pull/25681
https://github.com/bitcoin-core/packaging/issues/55
π¬ KaueTech commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156527625)
> This has been proposed before and typically gets rejected:
>
> #25681
>
> [bitcoin-core/packaging#55](https://github.com/bitcoin-core/packaging/issues/55)
I believe these are very different pull requests.
In my case, the goal is to allow anyone to run Bitcoin Core simply by cloning the repository and running:
```bash
docker compose up --build -d
````
There are three image build modes:
* `local`: for development (builds from local source)
* `github`: clones directly fr
...
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156527625)
> This has been proposed before and typically gets rejected:
>
> #25681
>
> [bitcoin-core/packaging#55](https://github.com/bitcoin-core/packaging/issues/55)
I believe these are very different pull requests.
In my case, the goal is to allow anyone to run Bitcoin Core simply by cloning the repository and running:
```bash
docker compose up --build -d
````
There are three image build modes:
* `local`: for development (builds from local source)
* `github`: clones directly fr
...
π¬ luke-jr commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2255285741)
Moving multiple files to a single file is an error, so it should cause the guix build to abort?
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2255285741)
Moving multiple files to a single file is an error, so it should cause the guix build to abort?
π¬ maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156558206)
> `local`: for development (builds from local source)
Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156558206)
> `local`: for development (builds from local source)
Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
π¬ KaueTech commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156572778)
> Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
> Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
Thanks for the feedback!
I understand that developers might want to customize build options β and this setup doesn't remove that flexibility. T
...
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3156572778)
> Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
> Developer would want to be able to pick different compile options. I don't think your approach of adding a hardcoded command is useful. Has any developer asked for this?
Thanks for the feedback!
I understand that developers might want to customize build options β and this setup doesn't remove that flexibility. T
...
π¬ l0rinc commented on pull request "test: remove duplicated code in test/functional/wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33122#issuecomment-3156583828)
code review ACK 6a7c0d3f874942a460bf5b13a107a2b21eb2e572
(https://github.com/bitcoin/bitcoin/pull/33122#issuecomment-3156583828)
code review ACK 6a7c0d3f874942a460bf5b13a107a2b21eb2e572
π¬ pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255303369)
There's also 3 other network types as well as zmq and tor ports.
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255303369)
There's also 3 other network types as well as zmq and tor ports.
π¬ pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255302543)
EspaΓ±ol here?
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255302543)
EspaΓ±ol here?
π¬ pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255305327)
This is lannwj's key but this isn't how we "sign" releases any more.
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2255305327)
This is lannwj's key but this isn't how we "sign" releases any more.
π¬ achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2255308540)
Done as suggested
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2255308540)
Done as suggested
π¬ w0xlt commented on pull request "wallet: prevent accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2255308480)
nit
```suggestion
height_delta = 65535 + (flag if flag is SEQUENCE_LOCKTIME_TYPE_FLAG else 0)
```
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2255308480)
nit
```suggestion
height_delta = 65535 + (flag if flag is SEQUENCE_LOCKTIME_TYPE_FLAG else 0)
```