✅ fanquake closed an issue: "把虚拟的“挖矿”货币应用到真实的实体“挖有机矿”,采用IoT设备记录真实的生产数据构成NFT,实现可追溯,不可簒改,去中心化的区块链Pi公链分布式"
(https://github.com/bitcoin/bitcoin/issues/33357)
(https://github.com/bitcoin/bitcoin/issues/33357)
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337076285)
This should be placed earlier, i.e. before `CheckEphemeralSpends`, to bound the number of mempool anestors before we start looking up whether they have dust to sweep.
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337076285)
This should be placed earlier, i.e. before `CheckEphemeralSpends`, to bound the number of mempool anestors before we start looking up whether they have dust to sweep.
💬 mzumsande commented on pull request "help: enrich help text for `-loadblock`":
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3275391542)
> I wrote that before I found the `contrib/linearize` tool, which seems to be designed for unobfuscated block sharing anyway
It was originally designed to to something else (create a linear, in order chain), so using the tool has side effects (maybe the users doesn't want to lose historical forks for some reason). Could mention it as an example though.
I'm not convinced that changing the interface to support adding a xor-key per `-loadblock` arg (or one for all?) is really worth the effort
...
(https://github.com/bitcoin/bitcoin/pull/33343#issuecomment-3275391542)
> I wrote that before I found the `contrib/linearize` tool, which seems to be designed for unobfuscated block sharing anyway
It was originally designed to to something else (create a linear, in order chain), so using the tool has side effects (maybe the users doesn't want to lose historical forks for some reason). Could mention it as an example though.
I'm not convinced that changing the interface to support adding a xor-key per `-loadblock` arg (or one for all?) is really worth the effort
...
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3275442471)
In bff8e053f4e6de3ce39e87a4faea161b1b84d850 "pubkey: Return tweaks from BIP32 derivation"
It would be helpful to mention why this is done in the commit message - specifically to mention around its usage in the "[sign: Create MuSig2 signatures for known MuSig2 aggregate keys](https://github.com/bitcoin/bitcoin/pull/29675/commits/7c085554dce336eb1597ab2fc482163876a49270)" commit pertaining to BIP 328/327.
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3275442471)
In bff8e053f4e6de3ce39e87a4faea161b1b84d850 "pubkey: Return tweaks from BIP32 derivation"
It would be helpful to mention why this is done in the commit message - specifically to mention around its usage in the "[sign: Create MuSig2 signatures for known MuSig2 aggregate keys](https://github.com/bitcoin/bitcoin/pull/29675/commits/7c085554dce336eb1597ab2fc482163876a49270)" commit pertaining to BIP 328/327.
🤔 mzumsande reviewed a pull request: "test: Fix CLI_MAX_ARG_SIZE issues"
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3206763025)
Code Review ACK fa96a4afea2a9bf90c843198e75a00acef02c32d
(https://github.com/bitcoin/bitcoin/pull/33243#pullrequestreview-3206763025)
Code Review ACK fa96a4afea2a9bf90c843198e75a00acef02c32d
💬 glozow commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337280911)
Is it problematic that there is no limit?
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337280911)
Is it problematic that there is no limit?
📝 amishhaa opened a pull request: "contrib: fix for macOS deployment build failing on Qt translations even though it is optional."
(https://github.com/bitcoin/bitcoin/pull/33358)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Q
...
(https://github.com/bitcoin/bitcoin/pull/33358)
From what I deciphered reading the line https://github.com/bitcoin/bitcoin/blob/master/contrib/macdeploy/macdeployqtplus#L390 is that qt translations are optional to have hence we should be able to build without it but the case where the flag translations_dir falls back to its default Null value it raises this error.
I have moved the code which adds language files under the if statement that first checks if the value of the flag is not Null before referencing it.
This PR assumes that having Q
...
👍 theStack approved a pull request: "test/refactor: use test deque to avoid quadratic iteration"
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3206983200)
ACK 8c1f10233dc9a843147772c40973031f5bfdbb7c
While I agree with others than very likely there won't be a notable difference in run-time any time soon (if ever), it seems conceptually the right approach.
(https://github.com/bitcoin/bitcoin/pull/33313#pullrequestreview-3206983200)
ACK 8c1f10233dc9a843147772c40973031f5bfdbb7c
While I agree with others than very likely there won't be a notable difference in run-time any time soon (if ever), it seems conceptually the right approach.
💬 l0rinc commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3276022733)
Not pruned, but did many reindex-chainstates (it's a rpi4b benchmarking server), maybe a few `reindex`es as well. I don't know how to reliably reproduce it, but it always happen on that server with the given state. I will try to copy the data over when the other servers free up.
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3276022733)
Not pruned, but did many reindex-chainstates (it's a rpi4b benchmarking server), maybe a few `reindex`es as well. I don't know how to reliably reproduce it, but it always happen on that server with the given state. I will try to copy the data over when the other servers free up.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337572396)
Excellent, added you as a coathor, thanks.
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337572396)
Excellent, added you as a coathor, thanks.
💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337582862)
not just the number, but also size as a proxy for number of parent outputs which are possibly scanned
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2337582862)
not just the number, but also size as a proxy for number of parent outputs which are possibly scanned
👍 hodlinator approved a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207641440)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
While it is unsettling that we don't fully understand what causes script checking to occur prior to the assumevalid height (https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2329269037) - having an initial "`Enabling signature validations at block #1 ...`" in `-assumevalid=0` cases as per this PR seems fine regardless.
(Empty `std::optional` does not match `bool fScriptChecks` until set).
https://github.com/bitcoin/bitcoin/blob/8cb75e
...
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207641440)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
While it is unsettling that we don't fully understand what causes script checking to occur prior to the assumevalid height (https://github.com/bitcoin/bitcoin/pull/32975#discussion_r2329269037) - having an initial "`Enabling signature validations at block #1 ...`" in `-assumevalid=0` cases as per this PR seems fine regardless.
(Empty `std::optional` does not match `bool fScriptChecks` until set).
https://github.com/bitcoin/bitcoin/blob/8cb75e
...
💬 hodlinator commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337774261)
nit: In case we are ever able to send away all blocks before detecting the expected log message, we could also wait for the block height before stopping the search of the log:
```suggestion
self.wait_until(lambda: self.nodes[0].getblockcount() >= COINBASE_MATURITY + 1)
```
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337774261)
nit: In case we are ever able to send away all blocks before detecting the expected log message, we could also wait for the block height before stopping the search of the log:
```suggestion
self.wait_until(lambda: self.nodes[0].getblockcount() >= COINBASE_MATURITY + 1)
```
⚠️ l0rinc opened an issue: "Corecheck isn't run for latest PRs"
(https://github.com/bitcoin/bitcoin/issues/33359)
See: https://corecheck.dev/bitcoin/bitcoin/pulls/33353
I have pushed a few changes that could fix it, reviews are welcome: https://github.com/corecheck/corecheck/pulls
(https://github.com/bitcoin/bitcoin/issues/33359)
See: https://corecheck.dev/bitcoin/bitcoin/pulls/33353
I have pushed a few changes that could fix it, reviews are welcome: https://github.com/corecheck/corecheck/pulls
👍 TheCharlatan approved a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207912876)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
The log message is a bit clunky though. Since it now appears on startup I think it would be nice if it also mentioned that this just reflects assumevalid.
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3207912876)
ACK 8cb75ee0a63880b55ac695fe884748aa7d604c38
The log message is a bit clunky though. Since it now appears on startup I think it would be nice if it also mentioned that this just reflects assumevalid.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3276630793)
> that this just reflects assumevalid
Unfortunately it's more complicated than that, see https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3273277122
If you have a concrete suggestion for how to rephrase it, I don't mind applying it.
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3276630793)
> that this just reflects assumevalid
Unfortunately it's more complicated than that, see https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3273277122
If you have a concrete suggestion for how to rephrase it, I don't mind applying it.
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337975289)
This seems less racy, indeed, updated, added you as coauthor, thanks
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2337975289)
This seems less racy, indeed, updated, added you as coauthor, thanks
🤔 nervana21 reviewed a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3208228592)
got it, thanks!
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3208228592)
got it, thanks!
📝 jalateras opened a pull request: "rpc: Add validation for invalid taproot signatures in analyzepsbt"
(https://github.com/bitcoin/bitcoin/pull/33360)
## Summary
This PR fixes issue #33320 by adding explicit validation for invalid taproot key path signatures in the `analyzepsbt` RPC command.
## Problem
Currently, when a PSBT contains an invalid `taproot_key_path_sig`, `analyzepsbt` returns a confusing status of `"next": "updater"` instead of properly reporting the validation error. This can happen when external applications incorrectly construct PSBTs.
## Solution
- Added validation in `AnalyzePSBT` to check for invalid taproot key path sign
...
(https://github.com/bitcoin/bitcoin/pull/33360)
## Summary
This PR fixes issue #33320 by adding explicit validation for invalid taproot key path signatures in the `analyzepsbt` RPC command.
## Problem
Currently, when a PSBT contains an invalid `taproot_key_path_sig`, `analyzepsbt` returns a confusing status of `"next": "updater"` instead of properly reporting the validation error. This can happen when external applications incorrectly construct PSBTs.
## Solution
- Added validation in `AnalyzePSBT` to check for invalid taproot key path sign
...
💬 ajtowns commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3277131901)
> My hesitation was mostly about re-using the coins cache as is. In my opinion it is the wrong data structure to use in this case. Potential clients of the kernel are often capable of supplying the coins in order as consumed by a transaction's inputs.
I think that's a false optimisation for two reasons: first, is that the primary client of the bitcoin core kernel is the bitcoin core node software, we should be designing for that case, not for other hypothetical clients; second, looking up a m
...
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-3277131901)
> My hesitation was mostly about re-using the coins cache as is. In my opinion it is the wrong data structure to use in this case. Potential clients of the kernel are often capable of supplying the coins in order as consumed by a transaction's inputs.
I think that's a false optimisation for two reasons: first, is that the primary client of the bitcoin core kernel is the bitcoin core node software, we should be designing for that case, not for other hypothetical clients; second, looking up a m
...