💬 0xB10C commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663292239)
Likely unrelated, but I've seen `Could not resolve host: github.com` as well a couples of times on my runners when attempting to upgrade to newer Nix packages. I've tried to bisect which changed package causes it, but no luck so far. I might give it another try tough now that you mention it here too. However, I'm using docker and not podman..
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663292239)
Likely unrelated, but I've seen `Could not resolve host: github.com` as well a couples of times on my runners when attempting to upgrade to newer Nix packages. I've tried to bisect which changed package causes it, but no luck so far. I might give it another try tough now that you mention it here too. However, I'm using docker and not podman..
💬 maflcko commented on pull request "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/31874#issuecomment-2663316239)
Tend towards approach NACK for now, concerning `PYTHONOPTIMIZE`, due to the outstanding issues mentioned in https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1957179722
(https://github.com/bitcoin/bitcoin/pull/31874#issuecomment-2663316239)
Tend towards approach NACK for now, concerning `PYTHONOPTIMIZE`, due to the outstanding issues mentioned in https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1957179722
⚠️ hebasto opened an issue: "Avoid plural forms in non-GUI translatable strings (lacks `%n` support)"
(https://github.com/bitcoin/bitcoin/issues/31890)
In non-GUI code, translatable strings with format specifiers rely on the `tinyformat::format` implementation, which does not support the `%n` specifier that is [crucial](https://doc.qt.io/qt-6/i18n-plural-rules.html) for pluralising with Qt translation tools.
Therefore, it may be worth considering rephrasing translatable strings to avoid potential plural forms.
This issue was initially reported on Transifex: https://app.transifex.com/bitcoin/bitcoin/translate/#fr/qt-translation-029x/570550303/
...
(https://github.com/bitcoin/bitcoin/issues/31890)
In non-GUI code, translatable strings with format specifiers rely on the `tinyformat::format` implementation, which does not support the `%n` specifier that is [crucial](https://doc.qt.io/qt-6/i18n-plural-rules.html) for pluralising with Qt translation tools.
Therefore, it may be worth considering rephrasing translatable strings to avoid potential plural forms.
This issue was initially reported on Transifex: https://app.transifex.com/bitcoin/bitcoin/translate/#fr/qt-translation-029x/570550303/
...
💬 philmb3487 commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1958379625)
Is that really judicious? Plenty of people run Bitcoind on Raspberry Pi's!
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1958379625)
Is that really judicious? Plenty of people run Bitcoind on Raspberry Pi's!
👍 hodlinator approved a pull request: "tests: add functional test for miniscript decaying multisig"
(https://github.com/bitcoin/bitcoin/pull/29156#pullrequestreview-2619489252)
ACK bb633c9407c46eeadf6fe85e859cea1fed24473f
Pretty neat to see thresholds being used in that way - as block heights are reached, each fulfilled locktime contributes to the count for the `thresh`. Was imagining some gigantic repeated `sortedmulti` + `or` + `after` combo (which would not have the key order problem inside of each `sortedmulti`, but would still require correct order of everything else and be prohibitively big).
PR helped me realize how different [Miniscript](https://github.co
...
(https://github.com/bitcoin/bitcoin/pull/29156#pullrequestreview-2619489252)
ACK bb633c9407c46eeadf6fe85e859cea1fed24473f
Pretty neat to see thresholds being used in that way - as block heights are reached, each fulfilled locktime contributes to the count for the `thresh`. Was imagining some gigantic repeated `sortedmulti` + `or` + `after` combo (which would not have the key order problem inside of each `sortedmulti`, but would still require correct order of everything else and be prohibitively big).
PR helped me realize how different [Miniscript](https://github.co
...
💬 hodlinator commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957188195)
Seems like a good time to use:
```suggestion
assert_greater_than(locktime, current_height)
```
If you prefer a visible comparison operator, this is simpler (states what we are testing against first and avoids negation):
```suggestion
assert_equal(True, current_height < locktime)
```
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957188195)
Seems like a good time to use:
```suggestion
assert_greater_than(locktime, current_height)
```
If you prefer a visible comparison operator, this is simpler (states what we are testing against first and avoids negation):
```suggestion
assert_equal(True, current_height < locktime)
```
💬 hodlinator commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957185305)
The "original" test `WalletMultisigDescriptorPSBTTest` doesn't mention Miniscript in the name, so I would drop that, and possibly more ("decaying multisig" hopefully implies descriptors and PSBTs being used).
```suggestion
class WalletDecayingMultisigTest(BitcoinTestFramework):
```
(Filename should be kept in sync if you rename).
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957185305)
The "original" test `WalletMultisigDescriptorPSBTTest` doesn't mention Miniscript in the name, so I would drop that, and possibly more ("decaying multisig" hopefully implies descriptors and PSBTs being used).
```suggestion
class WalletDecayingMultisigTest(BitcoinTestFramework):
```
(Filename should be kept in sync if you rename).
💬 hodlinator commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957191215)
Might be nice to add this just after to aid understanding of the complex expressions above:
```python
self.log.debug(f'Miniscript: {external["descriptor"]}')
```
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957191215)
Might be nice to add this just after to aid understanding of the complex expressions above:
```python
self.log.debug(f'Miniscript: {external["descriptor"]}')
```
💬 hodlinator commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957199989)
(I'm happy *wallet_multisig_descriptor_psbt.py* tests using isolated nodes, but that also means there's less need for that in this test).
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957199989)
(I'm happy *wallet_multisig_descriptor_psbt.py* tests using isolated nodes, but that also means there's less need for that in this test).
💬 hodlinator commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957205213)
`M` should probably be lowercase to signal later mutation? Also is only used in this function.
```suggestion
m = 4 # starts as 4-of-4
```
(Could optionally make `self.N` local too if you passed it in as an argument to `create_multisig()`.
(https://github.com/bitcoin/bitcoin/pull/29156#discussion_r1957205213)
`M` should probably be lowercase to signal later mutation? Also is only used in this function.
```suggestion
m = 4 # starts as 4-of-4
```
(Could optionally make `self.N` local too if you passed it in as an argument to `create_multisig()`.
💬 maflcko commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663408696)
As a temporary workaround, I started a terminal in the CI task https://cirrus-ci.com/task/5557162807656448 and typed `podman system reset`:
```
ci_worker_1738693519_026122670@core-ci-2:/tmp/cirrus-build-2385794995$ podman system reset
WARNING! This will remove:
- all containers
- all pods
- all images
- all networks
- all build cache
- all machines
- all volumes
- the graphRoot directory: "/home/ci_worker_1738693519_026122670/.loc
...
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663408696)
As a temporary workaround, I started a terminal in the CI task https://cirrus-ci.com/task/5557162807656448 and typed `podman system reset`:
```
ci_worker_1738693519_026122670@core-ci-2:/tmp/cirrus-build-2385794995$ podman system reset
WARNING! This will remove:
- all containers
- all pods
- all images
- all networks
- all build cache
- all machines
- all volumes
- the graphRoot directory: "/home/ci_worker_1738693519_026122670/.loc
...
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1958410398)
Indeed, I think I mixed this up at some point during a rebase. Thank!
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1958410398)
Indeed, I think I mixed this up at some point during a rebase. Thank!
🚀 fanquake merged a pull request: "cmake: Add `libbitcoinkernel` target"
(https://github.com/bitcoin/bitcoin/pull/31869)
(https://github.com/bitcoin/bitcoin/pull/31869)
💬 l0rinc commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1958426704)
Yes, but mostly with SSDs, not SD cards.
(https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1958426704)
Yes, but mostly with SSDs, not SD cards.
💬 darosior commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663468406)
> [bitcoin-096525e92cc2-arm64-apple-darwin.zip](https://github.com/user-attachments/files/18781660/bitcoin-096525e92cc2-arm64-apple-darwin.zip)
Tested this on a Mac M1. I could download it flawlessly through Firefox and perform most of IBD.
I tried downloading it through Safari and likewise other reviewers i encountered an error:
https://github.com/user-attachments/assets/4f244f13-93ba-4071-bca1-4232cb69b69e
The system language was set to French but i guess what matters here is the
...
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663468406)
> [bitcoin-096525e92cc2-arm64-apple-darwin.zip](https://github.com/user-attachments/files/18781660/bitcoin-096525e92cc2-arm64-apple-darwin.zip)
Tested this on a Mac M1. I could download it flawlessly through Firefox and perform most of IBD.
I tried downloading it through Safari and likewise other reviewers i encountered an error:
https://github.com/user-attachments/assets/4f244f13-93ba-4071-bca1-4232cb69b69e
The system language was set to French but i guess what matters here is the
...
💬 maflcko commented on pull request "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports":
(https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2663471948)
I've removed the milestone here for now, because this looks like it is adding a new feature that never existed before, so shouldn't be a blocker.
(https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2663471948)
I've removed the milestone here for now, because this looks like it is adding a new feature that never existed before, so shouldn't be a blocker.
💬 darosior commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663476018)
Another thing worth mentioning is that the Liana GUI would download and start a `bitcoind`. This means a notarized application can run non-notarized binaries just fine. This may be helpful to know in considering how we approach notarization here (and in a possible multiprocess future).
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2663476018)
Another thing worth mentioning is that the Liana GUI would download and start a `bitcoind`. This means a notarized application can run non-notarized binaries just fine. This may be helpful to know in considering how we approach notarization here (and in a possible multiprocess future).
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2663481343)
> I wonder if it may be easier to just disable the MP configure option when compiling for fuzzing
Not sure I understand the suggestion. Is it to not set ENABLE_IPC in the CI configuration? Show an error in the build when ipc and fuzzing are enabled together? Ignore the ipc option when fuzzing is enabled?
The bugfix here is simple, even though the bug is complicated to explain. It builds libraries with `-fsanitize=fuzzer-no-link` which is how libraries should be built, and builds the fuzz
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2663481343)
> I wonder if it may be easier to just disable the MP configure option when compiling for fuzzing
Not sure I understand the suggestion. Is it to not set ENABLE_IPC in the CI configuration? Show an error in the build when ipc and fuzzing are enabled together? Ignore the ipc option when fuzzing is enabled?
The bugfix here is simple, even though the bug is complicated to explain. It builds libraries with `-fsanitize=fuzzer-no-link` which is how libraries should be built, and builds the fuzz
...
💬 hebasto commented on issue "qa: `wallet_importdescriptors` failure "TypeError: 'bool' object is not subscriptable"":
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2663497225)
Another one: https://github.com/bitcoin/bitcoin/actions/runs/13373718736/job/37348002275
(https://github.com/bitcoin/bitcoin/issues/31881#issuecomment-2663497225)
Another one: https://github.com/bitcoin/bitcoin/actions/runs/13373718736/job/37348002275
💬 hebasto commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-2663500472)
> Fixes #31482.
The issue been fixed is assigned to the "29.0" milestone.
Should the milestones for an issue and a PR with its fix be aligned?
(https://github.com/bitcoin/bitcoin/pull/31724#issuecomment-2663500472)
> Fixes #31482.
The issue been fixed is assigned to the "29.0" milestone.
Should the milestones for an issue and a PR with its fix be aligned?