-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Version 2 transactions remain non-standard until CSV activates #7835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Before activation, such transactions might not be mined, so don't allow into the mempool.
utACK |
utACK da5fdbb |
utACK da5fdbb |
ACK da5fdbb |
tACK da5fdbb |
// Don't relay version 2 transactions until CSV is active, and we can be | ||
// sure that such transactions will be mined (unless we're on | ||
// -testnet/-regtest). | ||
const CChainParams& chainparams = Params(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this to the top of the function (other parts of the function before this also call Params() and could use the variable in the future)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing above in this function calls Params(); I guess I didn't check the functions called from here to see if they call Params(), but can we please fix that later when we change those functions? This has many ACKs now and is holding up 0.12.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sdaftuar I agree, it's not important for this pull.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I was in another branch, You are right for master. If you need to change the code for any other reason, I would appreciate moving this line too though.
tACK da5fdbb |
utACK da5fdbb |
utACK da5fdbb |
This was backported as 46898e7. Removing label |
Before activation, such transactions might not be reliably mined, so don't allow into the mempool.
Please tag for backport to 0.12.1