dkg rounding on chain #15071
Draft
dkg rounding on chain #15071
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
⏱️ 7m total CI duration on this PR
|
zjma
force-pushed
the
zjma/rounding_in_framework
branch
from
3be1a84 to
e8d05a7
Compare
zjma
added
the
CICD:run-e2e-tests
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
❌ Forge suite
|
✅ Forge suite
|
alinush
reviewed
Dec 9, 2024
| @@ -133,6 +133,7 @@ pub enum FeatureFlag { | |||
| CollectionOwner, | |||
| NativeMemoryOperations, | |||
| EnableLoaderV2, | |||
| NewFlag, | |||
There was a problem hiding this comment.
This needs a proper name, no?
There was a problem hiding this comment.
yup, probably rounding v2?
alinush
reviewed
Dec 9, 2024
| let mut function_name = BLOCK_PROLOGUE_EXT; | ||
|
|
||
| if self.features().is_enabled(FeatureFlag::NEW_FLAG) { | ||
| args.insert(0, MoveValue::Signer(AccountAddress::ONE)); |
There was a problem hiding this comment.
Giving a framework signer to the prologue that wasn't there before?
Security issues?
There was a problem hiding this comment.
today: call block_prologue_ext without framework signer.
After the feature is enabled: call block_prologue_ext_v2 with the signer.
should be fine?
There was a problem hiding this comment.
in principle fine for now
but in the future, block_prologue_ext_v2 implementations could misues the signer...
alinush
reviewed
Dec 9, 2024
| @@ -246,6 +248,38 @@ module aptos_framework::block { | |||
| }; | |||
| } | |||
|
|
|||
| /// `block_prologue()` but trigger reconfiguration with DKG after epoch timed out. | |||
There was a problem hiding this comment.
- Shouldn't this comment be "
block_prologue_**ext**()but callstry_start_v2instead oftry_start?
2." - Can you add a comment to the old
block_prologue_ext()to indicate when it is called and when it isn't? Same to this function?
alinush
reviewed
Dec 9, 2024
| @@ -123,8 +125,24 @@ module aptos_framework::randomness_config { | |||
| } | |||
| } | |||
|
|
|||
| #[test_only] | |||
| use aptos_std::fixed_point64; | |||
| /// Return the typy name, the secrecy threshold, the reconstruction threshold and the fast-path secrecy threshold. | |||
alinush
reviewed
Dec 9, 2024
| let v2 = copyable_any::unpack<ConfigV2>(config.variant); | ||
| (type_name, v2.secrecy_threshold, v2.reconstruction_threshold, v2.fast_path_secrecy_threshold) | ||
| } else { | ||
| let zero = fixed_point64::create_from_u128(0); |
There was a problem hiding this comment.
Shouldn't this return a Result::Err instead of zero's?
alinush
reviewed
Dec 9, 2024
| @@ -39,6 +43,32 @@ module aptos_framework::reconfiguration_with_dkg { | |||
| ); | |||
| } | |||
|
|
|||
| public(friend) fun try_start_v2(framework: &signer) { | |||
There was a problem hiding this comment.
Add comment and explain how this differs from try_start
alinush
reviewed
Dec 9, 2024
alinush
reviewed
Dec 9, 2024
alinush
approved these changes
Dec 9, 2024
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
How Has This Been Tested?
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?
Checklist