[event_v2] migrate the rest files in aptos-framework #11688
Merged
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
⏱️ 7h 11m total CI duration on this PR
🚨 2 jobs on the last run were significantly faster/slower than expected
|
lightmark
force-pushed
the
event_v2_migration_2
branch
from
cec4cd9
to
5a1f775
Compare
lightmark
requested review from
banool,
gregnazario and
0xmaayan
as code owners
lightmark
force-pushed
the
event_v2_migration_2
branch
from
5a1f775
to
3329fa6
Compare
lightmark
changed the title
lightmark
force-pushed
the
event_v2_migration_2
branch
3 times, most recently
from
da0e98b
to
e03a9c4
Compare
lightmark
force-pushed
the
event_v2_migration_2
branch
2 times, most recently
from
549a928
to
22f159f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11688 +/- ##
===========================================
- Coverage 69.2% 62.6% -6.7%
===========================================
Files 2296 821 -1475
Lines 438064 184173 -253891
===========================================
- Hits 303482 115391 -188091
+ Misses 134582 68782 -65800 ☔ View full report in Codecov by Sentry. |
lightmark
force-pushed
the
event_v2_migration_2
branch
from
22f159f
to
40f0989
Compare
lightmark
force-pushed
the
event_v2_migration_2
branch
2 times, most recently
from
6087009
to
e695fc1
Compare
davidiw
approved these changes
Mar 28, 2024
Comment on lines
+193
to
+203
| let new_block_event_v2 = NewBlock { | ||
| hash, | ||
| epoch, | ||
| round, | ||
| height: block_metadata_ref.height, | ||
| previous_block_votes_bitvec, | ||
| proposer, | ||
| failed_proposer_indices, | ||
| time_microseconds: timestamp, | ||
| }; | ||
| emit_new_block_event(vm, &mut block_metadata_ref.new_block_events, new_block_event, new_block_event_v2); |
There was a problem hiding this comment.
if we're doing this, we might incur a performance impact on each block before we even migrate.
There was a problem hiding this comment.
one more event per block? should not be an issue regarding perf.
Comment on lines
144
to
+150
| spec create_staking_contract( | ||
| staker: &signer, | ||
| operator: address, | ||
| voter: address, | ||
| amount: u64, | ||
| commission_percentage: u64, | ||
| contract_creation_seed: vector<u8>, | ||
| staker: &signer, | ||
| operator: address, | ||
| voter: address, | ||
| amount: u64, | ||
| commission_percentage: u64, | ||
| contract_creation_seed: vector<u8>, |
There was a problem hiding this comment.
intellij move plugin does this automatically and I have no way to disable it...
|
I will rebase once #11224 lands |
|
@grao1991 do you need to change the new block event tracking after this PR? |
Yes. It's protected by the sharding flag which hasn't been enabled. You go first and I can try to make the change later. FYI, you need to double check if other things (e.g. consensus) that depending on the new block event works correctly after your change. |
lightmark
force-pushed
the
event_v2_migration_2
branch
3 times, most recently
from
3c0145c
to
24f567d
Compare
bowenyang007
approved these changes
Apr 5, 2024
|
I trust you. Didn't carefully review. |
|
When are we removing the old events though? |
lightmark
force-pushed
the
event_v2_migration_2
branch
from
24f567d
to
f8d9432
Compare
Let's do it after 1-2 release. |
add feature flags use feature_flag to switch
lightmark
force-pushed
the
event_v2_migration_2
branch
from
f8d9432
to
356731a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
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.
Description
migrate other files
Test Plan