Skip to content
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

mempool: Remove old fix sequence lock rejection. #2496

Merged

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Dec 9, 2020

This removes code that was introduced to reject sequence locks from the mempool until the fix sequence lock agenda became active since that agenda is now active and buried deep in the chain and thus there is no longer any reason check for it.

It also reworks the sequence lock acceptance tests to test for the explicit reason for a failure to ensure that each test is actually testing the intended condition and to be more consistent with the rest of the code in preparation for removing the old fix sequence locks rejection code..

It is split into several commits to separate the test reformatting from the actual logic changes to ease review.

@davecgh davecgh added this to the 1.7.0 milestone Dec 9, 2020
@davecgh davecgh force-pushed the mempool_remove_old_fixseqlock_rejection branch from 8660996 to 0ae336b Compare December 9, 2020 09:37
Copy link
Member

@dnldd dnldd left a comment

Choose a reason for hiding this comment

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

Looks good. I don't think there is any documentation for this process, would be nice to have . Say after activation how many more activated agendas need to come after it in order to remove its associated explicit checks as was done in this PR?

@davecgh
Copy link
Member Author

davecgh commented Dec 9, 2020

Looks good. I don't think there is any documentation for this process, would be nice to have . Say after activation how many more activated agendas need to come after it in order to remove its associated explicit checks as was done in this PR?

There really can't be a concrete process because it largely depends on whatever the change(s) is/are doing and side effects, etc. In this particular case, the sequence locks that should have been valid were not being considered as such and thus any attempt to include them in a block template led to that block template being invalid. Since that is no longer the case and everybody must have updated now due to the associated hard fork vote, it can be removed from mempool now. If, on the other hand, the issue would've been that they were being calculated incorrectly and were valid when they shouldn't have been, this change wouldn't be possible in the same way.

@dnldd
Copy link
Member

dnldd commented Dec 9, 2020

Gotcha, thanks.

This updates the sequence lock mempool tests to be more consistent with
the rest of the code in preparation for removing the old fix sequence
locks rejection code.
This reworks the sequence lock acceptance tests to test for the explicit
reason for a failure to ensure that each test is actually testing the
intended condition.
This removes code that was introduced to reject sequence locks from the
mempool until the fix sequence lock agenda became active since that
agenda is now active and buried deep in the chain and thus there is no
longer any reason check for it.
@davecgh davecgh force-pushed the mempool_remove_old_fixseqlock_rejection branch from 0ae336b to 59c7f58 Compare December 14, 2020 21:38
@davecgh davecgh merged commit 59c7f58 into decred:master Dec 14, 2020
@davecgh davecgh deleted the mempool_remove_old_fixseqlock_rejection branch December 14, 2020 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants