-
Notifications
You must be signed in to change notification settings - Fork 232
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
fix: tx-pool refreshes caches with the incorrect VM version after hardfork #3090
Merged
bors
merged 5 commits into
nervosnetwork:develop
from
yangby-cryptape:pr/fix-tx-pool-refresh-caches
Oct 18, 2021
Merged
fix: tx-pool refreshes caches with the incorrect VM version after hardfork #3090
bors
merged 5 commits into
nervosnetwork:develop
from
yangby-cryptape:pr/fix-tx-pool-refresh-caches
Oct 18, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yangby-cryptape
force-pushed
the
pr/fix-tx-pool-refresh-caches
branch
from
October 12, 2021 22:51
426dc2f
to
fadc811
Compare
yangby-cryptape
force-pushed
the
pr/fix-tx-pool-refresh-caches
branch
from
October 13, 2021 08:10
1507401
to
43dd6b1
Compare
yangby-cryptape
requested review from
quake,
zhangsoledad,
driftluo and
keroro520
October 13, 2021 08:19
driftluo
approved these changes
Oct 14, 2021
quake
approved these changes
Oct 18, 2021
bors r=quake,driftluo |
bors bot
added a commit
that referenced
this pull request
Oct 18, 2021
3090: fix: tx-pool refreshes caches with the incorrect VM version after hardfork r=quake,driftluo a=yangby-cryptape ### What problem does this PR solve? Tx-pool refreshes caches with the incorrect VM version after hardfork if current tip block is the last block before hardfork. #### Problem Summary After #3058, all scripts will be verified under the environment of current tip block. So when tip block is the last block before hardfork, the hardfork features are not enabled, that means all scripts will be verified with old features. But in next block, all scripts should be verified with new hardfork features. #### PR Summary - test(hardfork): add a script and a unit test to test the change of the script's cycles after upgrade vm0 to vm1 For test propose, I need a **fixed-cycles** lock script to `assert` and it should has different cycles in vm0 and vm1. The cycles of secp256k1 lock is not stable. This unit test also ensures the VM version is correct: https://github.com/nervosnetwork/ckb/blob/e8f88fd73451c526f37e0879a5ed41272dbd3020/script/src/types.rs#L57-L58 - fix(test): fix `MockChain` that it always panics in `get_block_epoch(..)` when epoch changed - fix(hardfork): fix tx-pool that it refreshes caches with the incorrect VM version if it re-org to the last block before hardfork When the tip is the last block before hardfork, the transactions in block template should be verified with new hardfork features. So, we should always verify the transactions which are not committed, with the features for next block. - test(hardfork): add a unit test for the tx-pool caches (for #3079) - fix(hardfork): network should enable ckb2021 features when tip is the last block of ckb2019 ### Check List Tests - Unit test ### Release note ```release-note Title Only: Include only the PR title in the release note. ``` Co-authored-by: Boyu Yang <[email protected]>
Build failed: |
bors retry |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Tx-pool refreshes caches with the incorrect VM version after hardfork if current tip block is the last block before hardfork.
Problem Summary
After #3058, all scripts will be verified under the environment of current tip block.
So when tip block is the last block before hardfork, the hardfork features are not enabled, that means all scripts will be verified with old features.
But in next block, all scripts should be verified with new hardfork features.
PR Summary
test(hardfork): add a script and a unit test to test the change of the script's cycles after upgrade vm0 to vm1
For test propose, I need a fixed-cycles lock script to
assert
and it should has different cycles in vm0 and vm1.The cycles of secp256k1 lock is not stable.
This unit test also ensures the VM version is correct:
ckb/script/src/types.rs
Lines 57 to 58 in e8f88fd
fix(test): fix
MockChain
that it always panics inget_block_epoch(..)
when epoch changedfix(hardfork): fix tx-pool that it refreshes caches with the incorrect VM version if it re-org to the last block before hardfork
When the tip is the last block before hardfork, the transactions in block template should be verified with new hardfork features.
So, we should always verify the transactions which are not committed, with the features for next block.
test(hardfork): add a unit test for the tx-pool caches (for fix: don't use cache if tx-pool does re-org process during hardfork #3079)
fix(hardfork): network should enable ckb2021 features when tip is the last block of ckb2019
Check List
Tests
Release note