Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Mar 14, 2022

It has been pointed out that a bug in this function can prevent block template creation. ( #24080 (comment) ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b.

@glozow
Copy link
Member

glozow commented Mar 14, 2022

Could say "mempool validation helper functions" if your intent is to convey that these are only used for validating transactions being {,re-}added to the mempool.

@maflcko
Copy link
Member Author

maflcko commented Mar 14, 2022

An alternative follow-up suggested to use check the sequence locks before including a tx in a template. (#24080 (comment))

As that requires reading the utxo set, I am not sure about the performance impact. So I'll leave it for a follow-up.

@maflcko
Copy link
Member Author

maflcko commented Mar 14, 2022

Thx, done.

@luke-jr
Copy link
Member

luke-jr commented Mar 18, 2022

Concept NACK: Block template creation IS mere policy.

@maflcko
Copy link
Member Author

maflcko commented Mar 19, 2022

Maybe we need a separate word to describe block templates that are consensus invalid? It doesn't seem policy nor consensus, so I picked "validation".

@maflcko maflcko added the Docs label Jun 10, 2022
@fanquake fanquake requested a review from glozow August 5, 2022 08:34
Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fa86710

I agree this is clearer.

@glozow glozow requested a review from ajtowns August 8, 2022 15:27
@ajtowns
Copy link
Contributor

ajtowns commented Aug 9, 2022

ACK fa86710 - looks fine to me

@glozow glozow merged commit c012875 into bitcoin:master Aug 9, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 9, 2022
…alidation function

fa86710 Clarify that CheckSequenceLocksAtTip is a validation function (MarcoFalke)

Pull request description:

  It has been pointed out that a bug in this function can prevent block template creation. ( bitcoin#24080 (comment) ) So it seems that the scope of this function is more than "policy". Rename it back to "validation", to partially revert commit fa4e30b.

ACKs for top commit:
  ajtowns:
    ACK fa86710 - looks fine to me
  glozow:
    ACK fa86710

Tree-SHA512: 2e0df8c70df4cbea857977f140a8616cfa7505e74df66c9c9fbcf184670ce3ce7567183c3f76e6f3fe8ca6de0e065b9babde6352d6cb495e71ea077ddedbc3f4
@maflcko maflcko deleted the 2203-docVal-🍡 branch August 10, 2022 08:49
@bitcoin bitcoin locked and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants