The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• log: print unexpected version warning in validation log category #19898 (log: print unexpected version warning in validation log category by n-thumann)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

I think I'd rather (or additionally) ignore some bits for warning... the issue with bit-limiting it is that it could still false trigger.

Only if a majority of miners violate the protocol. To set aside bits for ASICBoost is a protocol change, that hasn't had community support demonstrated for. Developers alone don't get to make that call.

Tend to NACK. Seems a lot of code for a feature that has no use case.

NAK. Doesn't actually fix the thing it intends to fix.

The use case is to provide a proper warning that the protocol rules may be changing and the user should consider upgrading (either to the new rules, or a rule-blocking fork).

NAK. Doesn't actually fix the thing it intends to fix.

If there's a bug in it, report the bug...

If a future softfork is activated though versionbits on a single bit in less than 100 blocks, you might as well leave away signalling altogether. For other versionbits-activated (i.e. BIP-9 compatible) softforks we already warn appropriately before your change.

BIP 9 does not require the expected 95% for the other warning, and we currently lack any warning for different schemas.

currently lack any warning for different schemas

Indeed. And your code doens't offer a warning for any kind of schema either. So I'd prefer if we just embraced that fact and avoided any softfork deployments that are incompatible with the existing BIP9 warning mechanism.

Yes, it does... And again, the other warning doesn't work for all potential BIP 9 deployments either (they can use a lower threshold). On top of that, it's generally accepted that BIP 9 was a failure, and it's pretty likely future deployments will use other mechanisms.

Concept ACK. Miners not using the version numbers as intended should not be a reason to remove the warning.

Only if a majority of miners violate the protocol. To set aside bits for ASICBoost is a protocol change, that hasn't had community support demonstrated for. Developers alone don't get to make that call.

Setting aside bits for ASICBoost should have community support. I agree with Luke on that one.

Concept NACK for the reasons @MarcoFalke gives

Rebased

Rebased

Why? This has three NACKs and one Concept ACK, so at the least it seems controversial.

The NACKs are all based on the false claims, so should be ignored (or substantiated).

Per CONTRIBUTING.md:

NACK needs to include a rationale why the change is not worthwhile.
NACKs without accompanying reasoning may be disregarded.

We are talking about future events, so there is no clear true or false. My point is that we should build protocols that encourage safe use with little surprises. For example, using a threshold that is not recommended in BIP 9 is surprising in my eyes and might even lead to consensus deployment issues. Any BIP or code change that explicitly makes use of that or implicitly encourages it will be NACKed by me for exactly that reasoning.

You said it "has no use case", which it clearly does. You also claimed it "doesn't offer a warning for any kind of schema either", which is also false, as it has a check for different version schemas.

@gmaxwell said it doesn't fix the issue intended (lack of a warning), which again, it clearly does.

Neither of these deal with future events, just present realities (in which the claims are false).

@luke-jr If it "clearly" has a use case then why would someone be suggesting it does not? It seems it is clearly not clear to them. Also, "a check for different version schemas" does not seem equal to "a warning for any kind of version schema", so I am not sure you have rebutted the basis for the NACK here yet.

Seems so close to understanding being reached here.... just a little more effort from both sides..?

Rebased

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

• Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
• Is it no longer relevant? ➡️ Please close.
• Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Closing for now due to inactivity for more than 12 months. It can be reopened any time, just let us know with a comment either here or on IRC.