💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2663218525)
> > [96d3b2a](https://github.com/bitcoin/bitcoin/commit/96d3b2a0bb0c267569162b055ca306f709ec0b4e) "cmake: Fix fuzzer "multiple definition of `main'" errors"
>
> It may be good to explain this commit, or add steps to reproduce.

@maflcko guessing you maybe did not notice the comments or commit message? All the points you raised should be covered there, but let me know if anything should be updated or clarified.

Steps to reproduce are to check out the branch, revert the commit, and try to
...

✅ maflcko closed an issue: "ci: Intermittent failure "Could not resolve host: github.com""
(https://github.com/bitcoin/bitcoin/issues/31889)

💬 maflcko commented on issue "ci: Intermittent failure "Could not resolve host: github.com"":
(https://github.com/bitcoin/bitcoin/issues/31889#issuecomment-2663219347)
Closing for now, because it seems out-of-scope for this repo. (Not sure where to file otherwise, so feel free to continue discussion here for now)

🤔 sipa reviewed a pull request: "random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop"
(https://github.com/bitcoin/bitcoin/pull/31826#pullrequestreview-2621167189)
utACK 09b150bb8adae00854f02ece69fc6ef222fb07d9

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#discussion_r1958297299)
thanks, will do in the follow-up

💬 hodlinator commented on pull request "qa wallet: Activate dormant checks in wallet_multisig_descriptor_psbt.py":
(https://github.com/bitcoin/bitcoin/pull/31874#discussion_r1958297307)
I'd say #29500 is more related to #23119 than this PR.
> Btw, [type hints](https://github.com/bitcoin/bitcoin/compare/84485368c4234596519aace2b851402e3b53e624..790c509a4796ec47be2275d86b35370ff25a599a#diff-e20e1f68486e5c096fdc11bca1cda063aacef524411f011d9c3497eb650c5eafR73) in Python are just hints, adding assert_true(1) will still pass the test.

Yes, that's why I commented about the linter situation above.

> @hodlinator, could we separate fixing dead code from introducing another asserti
...

💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2663237229)
Thanks for the two nits. I'll address them in one of the related follow-ups that will touch this area anyway.

💬 maflcko commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2663270815)
Thanks for explaining. I must have skipped over the last paragraph in the commit message, which mentions the mpgen executable. However, given the lack of any fuzz target that could possibly call into mp code, I wonder if it may be easier to just disable the MP configure option when compiling for fuzzing. Otherwise, it seems like changing the build code to support compiling code that will never be executed anyway.

✅ l0rinc closed a pull request: "WIP: speed up `BatchWrite` by sorting the batches in descending order"
(https://github.com/bitcoin/bitcoin/pull/31875)

💬 l0rinc commented on pull request "WIP: speed up `BatchWrite` by sorting the batches in descending order":
(https://github.com/bitcoin/bitcoin/pull/31875#issuecomment-2663275569)
Closing for now to avoid needless reviews, since further benchmarks cast doubt on the validity of this finding, will reopen after I clarify the source of this ambiguity.

💬 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..

💬 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

⚠️ 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/
...

💬 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!

👍 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
...

💬 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)
```

💬 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).

💬 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"]}')
```

💬 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).

💬 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()`.