π¬ maflcko commented on pull request "ci: Work around GitHub Recv failure on raw curl usage":
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396208981)
The issue doesn't happen right now, so I am not sure how to test this pull here. Maybe re-open the next time the issue happens?
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396208981)
The issue doesn't happen right now, so I am not sure how to test this pull here. Maybe re-open the next time the issue happens?
π€ janb84 reviewed a pull request: "doc: archive release notes for v30.0"
(https://github.com/bitcoin/bitcoin/pull/33601#pullrequestreview-3330287799)
ACK 8d6e49158e3a1b3215484b5d139c5d8a3fffc4c9
Normal post-release cleanup pr, move the release-notes to the release-notes archive
(https://github.com/bitcoin/bitcoin/pull/33601#pullrequestreview-3330287799)
ACK 8d6e49158e3a1b3215484b5d139c5d8a3fffc4c9
Normal post-release cleanup pr, move the release-notes to the release-notes archive
π dergoegge approved a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3330359960)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3330359960)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765
π¬ BenWestgate commented on pull request "[POC] wallet: Enable non-electronic (paper-based) wallet backup with codex32":
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3396349582)
> Because Bitcoin Core wallets currently do not store the seed material by default, this PR adds support for doing so, along with a new wallet flag to explicitly indicate when this feature is enabled.
Wallets would need to begin storing the seed by default for `exposesecret` to function.
This feature also should allow specifying the identifier for disambiguation and the threshold if secret sharing, possibly the number of shares as well, but that requires new entropy.
It may be simpler to init
...
(https://github.com/bitcoin/bitcoin/pull/33043#issuecomment-3396349582)
> Because Bitcoin Core wallets currently do not store the seed material by default, this PR adds support for doing so, along with a new wallet flag to explicitly indicate when this feature is enabled.
Wallets would need to begin storing the seed by default for `exposesecret` to function.
This feature also should allow specifying the identifier for disambiguation and the threshold if secret sharing, possibly the number of shares as well, but that requires new entropy.
It may be simpler to init
...
π¬ janb84 commented on pull request "ci: Work around GitHub Recv failure on raw curl usage":
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396436570)
> The issue doesn't happen right now, so I am not sure how to test this pull here. Maybe re-open the next time the issue happens?
Think this is the best action for now.
If we are going to park this PR, maybe it's good to add a bit more context to the PR description (suggestion):
"
Attempt to work around https://github.com/bitcoin/bitcoin/issues/33599 by using a filtered git clone instead of a simple raw curl.
Rationale:
The PR is based on a working theory that this workaround is ne
...
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396436570)
> The issue doesn't happen right now, so I am not sure how to test this pull here. Maybe re-open the next time the issue happens?
Think this is the best action for now.
If we are going to park this PR, maybe it's good to add a bit more context to the PR description (suggestion):
"
Attempt to work around https://github.com/bitcoin/bitcoin/issues/33599 by using a filtered git clone instead of a simple raw curl.
Rationale:
The PR is based on a working theory that this workaround is ne
...
π¬ stratospher commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2425603366)
d1a2e27: it would be nice to have a more general interface where we can pass the parent + child amount and fees.
cc @vasild - you might be interested in this too. I hadn't considered passing both wallet and node as arguments to the function in #32385. there are other functions in util.py which do this too - ex: `create_lots_of_big_transactions`
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2425603366)
d1a2e27: it would be nice to have a more general interface where we can pass the parent + child amount and fees.
cc @vasild - you might be interested in this too. I hadn't considered passing both wallet and node as arguments to the function in #32385. there are other functions in util.py which do this too - ex: `create_lots_of_big_transactions`
π¬ stratospher commented on pull request "test: refactor mempool_accept_wtxid":
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2425605999)
d1a2e27: any reason for removing this line? (comment mentions submitting parent to mempool)
(https://github.com/bitcoin/bitcoin/pull/33067#discussion_r2425605999)
d1a2e27: any reason for removing this line? (comment mentions submitting parent to mempool)
π¬ Sjors commented on pull request "multiprocess: Fix high overhead from message logging":
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3396502813)
CI doesn't look happy.
(https://github.com/bitcoin/bitcoin/pull/33517#issuecomment-3396502813)
CI doesn't look happy.
π¬ alexanderwiederin commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3396550023)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/81cec737e68b91f5edf90179b81aa620a5a68677
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3396550023)
re-ACK https://github.com/bitcoin/bitcoin/pull/30595/commits/81cec737e68b91f5edf90179b81aa620a5a68677
π¬ TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3396649526)
Rebased b27be6cb18dde53863d83d2f2f61d4c92916257b -> 9ce01e051bae5fb1d48d6d297eb011b13d20ec76 ([spendblock_12](https://github.com/TheCharlatan/bitcoin/tree/spendblock_12) -> [spendblock_13](https://github.com/TheCharlatan/bitcoin/tree/spendblock_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_12..spendblock_13))
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3396649526)
Rebased b27be6cb18dde53863d83d2f2f61d4c92916257b -> 9ce01e051bae5fb1d48d6d297eb011b13d20ec76 ([spendblock_12](https://github.com/TheCharlatan/bitcoin/tree/spendblock_12) -> [spendblock_13](https://github.com/TheCharlatan/bitcoin/tree/spendblock_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_12..spendblock_13))
π€ ismaelsadeeq reviewed a pull request: "kernel: Introduce initial C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3330798092)
Code review ACK 81cec737e68b91f5edf90179b81aa620a5a68677 π₯
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3330798092)
Code review ACK 81cec737e68b91f5edf90179b81aa620a5a68677 π₯
β
fanquake closed a pull request: "ci: Work around GitHub Recv failure on raw curl usage"
(https://github.com/bitcoin/bitcoin/pull/33606)
(https://github.com/bitcoin/bitcoin/pull/33606)
π¬ fanquake commented on pull request "ci: Work around GitHub Recv failure on raw curl usage":
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396767734)
> Maybe re-open the next time the issue happens?
I think so. Hopefully some followup from Cirrus in #33599.
(https://github.com/bitcoin/bitcoin/pull/33606#issuecomment-3396767734)
> Maybe re-open the next time the issue happens?
I think so. Hopefully some followup from Cirrus in #33599.
π¬ fanquake commented on pull request "doc: archive release notes for v30.0":
(https://github.com/bitcoin/bitcoin/pull/33601#discussion_r2425804500)
Thanks, however I wont be modifying the release notes further here.
(https://github.com/bitcoin/bitcoin/pull/33601#discussion_r2425804500)
Thanks, however I wont be modifying the release notes further here.
π marcofleon approved a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3330818813)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3330818813)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765
π¬ fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2425811942)
> CMake does not provide options for all NSIS attributes. For attributes that are strictly required,
If migrating to do this the "CMake way" means less flexibility or giving up functionality because CMake doesn't support something, that tradeoff that should be mentioned in the PR description. This also means that any future option we might want to use, may also require workarounds, assuming that CMake support for new options will lag NSIS releases by some amount (or not be added at all), and
...
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2425811942)
> CMake does not provide options for all NSIS attributes. For attributes that are strictly required,
If migrating to do this the "CMake way" means less flexibility or giving up functionality because CMake doesn't support something, that tradeoff that should be mentioned in the PR description. This also means that any future option we might want to use, may also require workarounds, assuming that CMake support for new options will lag NSIS releases by some amount (or not be added at all), and
...
π¬ fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#issuecomment-3396786226)
> Adjust the Guix build script as well?
Yea, this wont Guix build:
```bash
HOSTS="x86_64-w64-mingw32" ./contrib/guix/guix-build
<snip>
[100%] Built target bitcoin-qt
Running symbol and dynamic library checks...
[100%] Built target check-symbols
make: *** No rule to make target 'deploy'. Stop.
```
(https://github.com/bitcoin/bitcoin/pull/33455#issuecomment-3396786226)
> Adjust the Guix build script as well?
Yea, this wont Guix build:
```bash
HOSTS="x86_64-w64-mingw32" ./contrib/guix/guix-build
<snip>
[100%] Built target bitcoin-qt
Running symbol and dynamic library checks...
[100%] Built target check-symbols
make: *** No rule to make target 'deploy'. Stop.
```
π¬ hebasto commented on issue "Remove remaining mention of datacarriersize being deprecated":
(https://github.com/bitcoin/bitcoin/issues/33605#issuecomment-3396835542)
> Best to remove that too?
https://github.com/bitcoin/bitcoin/blob/d0f6d9953a15d7c7111d46dcb76ab2bb18e5dee3/src/qt/bitcoinstrings.cpp#L1
In the case of the source string removed in https://github.com/bitcoin/bitcoin/pull/33453, this line wonβt affect the translation at runtime.
(https://github.com/bitcoin/bitcoin/issues/33605#issuecomment-3396835542)
> Best to remove that too?
https://github.com/bitcoin/bitcoin/blob/d0f6d9953a15d7c7111d46dcb76ab2bb18e5dee3/src/qt/bitcoinstrings.cpp#L1
In the case of the source string removed in https://github.com/bitcoin/bitcoin/pull/33453, this line wonβt affect the translation at runtime.
β
hebasto closed an issue: "Remove remaining mention of datacarriersize being deprecated"
(https://github.com/bitcoin/bitcoin/issues/33605)
(https://github.com/bitcoin/bitcoin/issues/33605)
π¬ maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3396867870)
> > (You can see an outline here if you're interested [master...willcl-ark:bitcoin:docker-ci-containers](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:docker-ci-containers) from march. This attempt tried to preserve the ability to run on bare metal too, but if we didn't want that then most of the scripts could be moved into dockerfiles too, further simplifying/locking us in to docker π .)
>
> bare metal is required for the macOS CI, so I don't think we can go pure do
...
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3396867870)
> > (You can see an outline here if you're interested [master...willcl-ark:bitcoin:docker-ci-containers](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:docker-ci-containers) from march. This attempt tried to preserve the ability to run on bare metal too, but if we didn't want that then most of the scripts could be moved into dockerfiles too, further simplifying/locking us in to docker π .)
>
> bare metal is required for the macOS CI, so I don't think we can go pure do
...