Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 glozow reviewed a pull request: "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only"
(https://github.com/bitcoin/bitcoin/pull/26398#pullrequestreview-2725923391)
Thanks for rebasing! Needs release note.
💬 glozow commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018888797)
This looks incorrect, it's still using <=64
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2761728012)
@hodlinator
> > A new style plugin causes minor visual glitches that will be fixed in follow-ups.
>
> Did not notice glitches, what are you referring to?

I've added a screenshot to the PR description.
💬 darosior commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018916429)
This looks correct to me? It preserves existing behaviour, i don't think the wallet needs to be updated to support 61 62 and 63 bytes transactions in this PR?
🤔 glozow reviewed a pull request: "descriptors: Multipath/PR 22838 follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32134#pullrequestreview-2725971903)
utACK 7aa397a7d45ce0ae1b83cc6f3a65db0e41b21ee8, cc @achow101
💬 instagibbs commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2018920194)
I intended to maintain current behavior, which I think I've done?
💬 sipa commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2761742864)
utACK b6fe5a2196d53a3024dfe7f11b316a50acc3cb7

The code changes make sense, I think, given the fact that the block index failed flags are generally kept consistent already, apart from some edge cases around `invalidateblock`. I think the existing design stemmed from a desire not to have full block index iterations wherever possible, but if we've already bit the bullet on that, there is no need to for the complexity laziness brings here.
💬 Sjors commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2018964740)
Note to reviewers: `IsFinal()` has been present since the first commit as a way to ignore the sequence number, hence the need to avoid `SEQUENCE_FINAL`.

The proposed consensus rule doesn't require `MAX_SEQUENCE_NONFINAL`, anything other than `SEQUENCE_FINAL` is allowed. Maybe that's good to say in a comment.
💬 Sjors commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2018965668)
As a sanity check, I changed this to just `nHeight`, which immediately triggers:

```
TestBlockValidity: Consensus::ContextualCheckBlock: bad-txns-nonfinal, non-final transaction
```

(either when calling `getblocktemplate` or in sv2 as soon as the Template Provider tries to build a template)

Dropping the `nSequence` line makes that error go away.

In other words, this guarantees that the coinbase transaction can't have been present in an older block. But as the BIP points out, nLockT
...
💬 pablomartin4btc commented on pull request "wallet, migration: Fix empty wallet crash":
(https://github.com/bitcoin/bitcoin/pull/32149#issuecomment-2761858650)
_<ins>Updates</ins>:_
- Addressed @furszy's [feedback](https://github.com/bitcoin/bitcoin/pull/32149#discussion_r2018614237).
💬 pablomartin4btc commented on pull request "wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled":
(https://github.com/bitcoin/bitcoin/pull/31451#discussion_r2018987451)
@furszy, let me know if this is what you meant... https://github.com/bitcoin/bitcoin/commit/401121530c0f0331ad73938f30bf18f11d759bfc (you can comment there if you like before I open a PR)
💬 polespinasa commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2762061412)
> That's what activating the soft-fork does. But there's a chicken-egg problem, because one reason to delay activating a soft fork is because part of the ecosystem isn't ready

Sorry maybe I didn't explain myself. I meant to introduce the code, but without activating it and without activation date. So if this consensus change doesn't go forward, we make sure that we will not have hard fork because of nodes running a version with the check code implemented. Just put it there to say: "Here it is
...
💬 DaanyaalSobani commented on issue "Allow sending untrusted utxos in the sendtoaddress api":
(https://github.com/bitcoin/bitcoin/issues/32034#issuecomment-2762107181)
Is there currently a difference between untrusted and unconfirmed?
💬 glozow commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#discussion_r2019134032)
Ah I see!
💬 fjahr commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019138847)
It seems to have worked with older macos and/or clang versions in the past and I have heard from someone that they avoid upgrading because they don't want to deal with these issues. So there are un-upgraded users that may not run into this and hopefully in the future this will be fixed again and the instructions are not necessary anymore.

Would be great to remove the section as a whole in the future. The problem is that if we make everyone run the special instructions we will not know if they
...
💬 fjahr commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2019141108)
I am here commenting in this issue because I ran into the problem. Macos users that don't have this problem will probably not even look at this issue. So could be that the people that have the problem are in the minority.
🤔 instagibbs reviewed a pull request: "cluster mempool: add txgraph diagrams/mining/eviction"
(https://github.com/bitcoin/bitcoin/pull/31444#pullrequestreview-2725655591)
Looking good 0630995fee22990402771547be1480b8706c76ce

No real show-stoppers except for the diagram check fuzz test conceptual issue I'm still having.
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018742079)
micro-nit
```Suggestion
std::vector<FeeFrac> chunk_feerates;
```
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018744853)
We don't care or it will be respected within clusters due to CFR?
💬 instagibbs commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#discussion_r2018933019)
nit: const chunk_count