Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 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?
📝 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
...
👍 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.
💬 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.
💬 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.
💬 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
👍 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
...
💬 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)
```
⚠️ 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
👍 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.
💬 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.
💬 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
🤔 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!
📝 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
...
💬 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
...
💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2338338113)
Added something like this, though without the "88"
💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2338338321)
Added
💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2338338802)
Round-trip assertions in both directions added to fuzz/script_flags.cpp
💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2338351098)
Instead of the current single-entry LevelDB batching (creating a batch, serializing the `DBHeightKey` and `std::pair<uint256, DBVal>` and writing the batch, containing a single entry), I have extracted the `CDBBatch` outside of the loops and committing and writing *that* regularly, see https://github.com/l0rinc/bitcoin/pull/37/files for a draft (I will need to break into commits and maybe adjust some tests if needed).

I will run a follow-up benchmark to see how well this performs, since the o
...
💬 l0rinc commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#issuecomment-3277968407)
I have instrumented the code, ran some RPC commands and reindexed the whole thing again to see why script verification is always on.
After some discussions with @ajtowns it turns out I needed both `-assumevalid=<in-range header>` and `-minimumchainwork=0` to be able to disable script verification.

---

After `reindex`, my node has ~841k headers, a lot lower than the current tip of ~914k.
The default `assumevalid` block (height 912,683) is in that missing range, so the header lookup fails
...