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

💬 sandakersmann commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656946537)
@petertodd Why didn't any hash power opt-in to RBF back in 2015?

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656966330)


On July 30, 2023 1:00:32 AM GMT+02:00, "Marius Kjærstad" ***@***.***> wrote:
>@petertodd Why didn't any hash power opt-in to RBF back in 2015?

They did. I created a full-rbf patch years ago that some miners were running.

See also: https://petertodd.org/2016/are-wallets-ready-for-rbf

💬 sandakersmann commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656967556)
@petertodd Why didn't any hash power opt-in to RBF before you made that patch?

💬 kevkevinpal commented on pull request "qa, doc: Fix comment":
(https://github.com/bitcoin/bitcoin/pull/28181#issuecomment-1656981944)
Concept ACK [011c77d](https://github.com/bitcoin/bitcoin/pull/28181/commits/011c77dd8d109b6b4d1291a778fa9c7f85750c77)

💬 zkfrio commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1656989393)
Concept ACK

There are no answers for questions that do not make sense. Also there are no arguments left against full RBF as it's used regularly.

💬 luke-jr commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1278476394)
nit: There used to be a blank line before this. IMO it was nicer.

💬 luke-jr commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1278476346)
Why was the order here swapped? Seems like override should be the very last keyword IMO

💬 luke-jr commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1278476957)
Do we want to monitor only the active chainstate? Or should we be paying attention to the background one too?

💬 luke-jr commented on pull request "validation: Flush state after initial sync":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1278477065)
`-1` is always going to be `!= current_height` anyway...?