Bitcoin Core Github
44 subscribers
121K links
Download Telegram
βœ… fanquake closed an issue: "Flash bitcoin"
(https://github.com/bitcoin/bitcoin/issues/28182)
:lock: fanquake locked an issue: "Flash bitcoin"
(https://github.com/bitcoin/bitcoin/issues/28182)
πŸ’¬ MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1656730905)
We'll be dropping support for Cirrus CI dockerfiles anyway within the next 30 days, going to full VMs (either on GitHub Actions CI, or on self-hosted machines), so I think `chattr` should work fine, if this pull can wait for a few more days.
πŸ’¬ MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1656733618)
Unrelated to this thread, but I am moving the Linux stuff to self-hosted runners (like the "previous releases" task already is). Starting in https://github.com/bitcoin/bitcoin/pull/28161 with the `ASan` task, which needs a dedicated Ubuntu Lunar VM.
πŸ’¬ pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1656734972)
Cool. Is there an open PR for that yet? Plz add me as reviewer!
πŸ’¬ Sjors commented on pull request "validation: add missing insert to m_dirty_blockindex":
(https://github.com/bitcoin/bitcoin/pull/27905#issuecomment-1656738063)
Should we make `nStatus` private and only allow writing to it through a helper method that makes it dirty?
πŸ’¬ MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1656744733)
Sure, https://github.com/bitcoin/bitcoin/pull/28161 is the fist step. It won't make a difference for `chattr`, because it is just moving one VM to another VM, but it is the first step.
πŸ€” Sjors reviewed a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1553292155)
Comments around 768690b7ce551cd403f8e2a099372915f6022ad4.

It would be nice if the documentation for `setBlockIndexCandidates` and `m_blocks_unlinked` explained their purpose (as opposed to merely describing what's in them).

```cpp
/**
* The set of all CBlockIndex entries that are - or recently
* were - candidates for a new tip. These are as good as our
* current tip or better, and must either have BLOCK_VALID_TRANSACTIONS
* (for itself and all ancestors) *or* BLOCK_ASSUMED_VALID (if
...
πŸ’¬ Sjors commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1278308074)
768690b7ce551cd403f8e2a099372915f6022ad4: Maybe clarify that initially this only applies to the snapshot chainstate, which uses the base block as its starting point. For the background chainstate, as well an active chainstate not created from a snapshot, `GetSnapshotBaseBlock()` is `nullptr` so the special-case won't apply. But the background state will consider it a candidate after download.
πŸ’¬ Sjors commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1278312526)
768690b7ce551cd403f8e2a099372915f6022ad4: this had me confused, but it's because the test considers a scenario where the snapshot base block has been downloaded. It could be nice to include the moment _before_ it's downloaded where this should be `0`.
πŸ’¬ Sjors commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1278308558)
768690b7ce551cd403f8e2a099372915f6022ad4

```cpp
//!
//! - Run LoadBlockIndex() and ensure that setBlockIndexCandidates of the first ...
```
πŸ’¬ petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656763717)
> > So frankly, at this point I don't care to split it up into two pull-reqs for such a minor reason.
>
> Making a significant change to default policy is not a minor reason.

This is not a significant change. Large amounts of data are already published in the bitcoin blockchain in a variety of ways. Heck, `-permitbaremultisig` is still allowed by default, and people still use it to publish data in the form of spendable and even unspendable outputs. And unlike OP_Return outputs, bare multis
...
πŸ’¬ petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#discussion_r1278334958)
I reworked those unit tests with better comments and clearer constants, and expanded the test coverage a bit.
πŸ’¬ petertodd commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656787140)
@willcl-ark

> I’m on mobile and didn’t check the pull, but do you need to `from typing import List` too?

Thanks! I believe that fixed the issue.
πŸ‘ Sjors approved a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1553302075)
Code review ACK a733dd79e29068ad1e0532ac42a45188a040a7b9.

My comments above can all wait for a followup, if any.

I'd like to take this for a spin in a rebased #27596 with `-checkblockindex` to see if we didn't miss anything (without pruning, since that has a clear TODO).
πŸ’¬ Sjors commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1278316042)
d0d40ea9a6478d81d7531b7cfc52a8bdaa0883d6: `// We don't relay background validation blocks.`
πŸ’¬ Sjors commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1278315696)
d0d40ea9a6478d81d7531b7cfc52a8bdaa0883d6 Maybe add: `Background validation always ignores unrequested blocks.`
πŸ’¬ Sjors commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1656811415)
@ChristopherA wrote:

> In my own case, current op_return limits keep me from posting a signed hash plus some metadata (<256 bytes) and thus I must use Taproot tricks instead.

@petertodd wrote:

> Large amounts of data are already published in the bitcoin blockchain in a variety of ways.

This seems like something to propose on the bitcoin-dev mailinglist, like it was [ten years ago](https://blog.bitmex.com/dapps-or-only-bitcoin-transactions-the-2014-debate/). You could recap the origin
...
πŸ’¬ sandakersmann commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656894747)
What happened to: "It's just opt-in"?
πŸ’¬ petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656919344)


On July 29, 2023 11:08:33 PM GMT+02:00, "Marius Kjærstad" ***@***.***> wrote:
>What happened to: "It's just opt-in"?

~40% of hash power decided to opt-in. You opting out is meaningless once that has happened, because unconfirmed transactions are well and truly insecure. So might as well enable it by default.

Miners can still opt out if they choose. Though there's no reason not to.