TheCharlatan
commented at 12:04 pm on November 11, 2024:
contributor
The *_RECENT_CONSENSUS_CHANGE variants in the validation result enumerations were always unused. They seem to have been kept around speculatively for a soft fork after segwit, however they were never used for taproot either. This points at them not having a clear purpose. Based on the original pull requests’ comments their usage was never entirely clear:
#11639 (comment)#15141 (review)
Since they are part of the validation interface and need to be exposed by the kernel library keeping them around may also be confusing to future users of the library.
validation: Remove RECENT_CONSENSUS_CHANGE validation result
The *_RECENT_CONSENSUS_CHANGE variants in the validation result
enumerations were always unused. They seem to have been kept around
speculatively for a soft fork after segwit, however they were never used
for taproot either. This points at them not having a clear purpose.
Based on the original pull requests' comments their usage was never
entirely clear:
https://github.com/bitcoin/bitcoin/pull/11639#issuecomment-370234133
https://github.com/bitcoin/bitcoin/pull/15141#discussion_r271039747
Since they are part of the validation interface and need to exposed by
the kernel library keeping them around may also be confusing to future
users of the library.
e80e4c6ff9
DrahtBot
commented at 12:04 pm on November 11, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#31112 (Improve parallel script validation error debug logging by sipa)
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.
DrahtBot added the label
Validation
on Nov 11, 2024
dergoegge approved
dergoegge
commented at 2:17 pm on November 11, 2024:
member
ACKe80e4c6ff91e27d7d40f099a2d7942c29085234c
We can always re-introduce the concept and actually make use of it if needed. For now it seems better to remove it from the interface.
ajtowns
commented at 3:52 am on November 13, 2024:
contributor
To be able to differentiate RECENT_CONSENSUS_CHANGE errors, you’d need to revalidate txs up to three times instead of two (did this fail standardness? because of consensus rules? because of recent consensus rules?), which is probably more hassle (it’s marginally easier for an attacker to use up CPU cycles) than it’s worth (non-consensus compatible nodes stay well-connected for longer so are less likely to follow low-work chains). It’s probably better dealt with via things like block-only-peers (even if you relay bad txs, your block-only peers won’t disconnect you), long LOCKED_IN periods (more time to upgrade to new consensus changes), and the general policy of consensus changes only affecting txs that are already non-standard. IMHO anyway.
Concept ACK.
sipa
commented at 1:23 pm on November 13, 2024:
member
ACKe80e4c6ff91e27d7d40f099a2d7942c29085234c
DrahtBot requested review from ajtowns
on Nov 13, 2024
instagibbs
commented at 2:51 pm on November 13, 2024:
member
@sdaftuar since you’re the one who introduced this flag IIRC, mind weighing in?
makes sense to me personally, concept ACK
sdaftuar
commented at 5:28 pm on November 13, 2024:
member
As far as I can tell, I was also in agreement with not needing this RECENT_CONSENSUS_CHANGE enum in our code until it was time to use it! @TheBlueMatt was the one who thought we needed it, and I came around to thinking that there was a potential use case for it in the event of another soft fork.
But I’m fine with dropping it; if someone decides to rework our net_processing code to take advantage of subtler differences in validation failures, we can reintroduce it at that point.
ajtowns approved
ajtowns
commented at 2:17 am on November 14, 2024:
contributor
ACKe80e4c6ff91e27d7d40f099a2d7942c29085234c
DrahtBot requested review from instagibbs
on Nov 14, 2024
naumenkogs
commented at 9:08 am on November 14, 2024:
contributor
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-11-26 06:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me