Open
Conversation
…Foundation/aptos-core into feature/dkg-integration
* feat(types,accumulator): Add features for transactions inclusions and events emissions proofs. - RLP's `Encodable` trait implementation for Contract Events. - `get_proof_by_position` method for Merkle Accumulator. * feat: add `EventV2` api type * chore: cargo feature optimization * refactor(aptos-api-types): transformed `Event` to `EventV1` and applied Full-Clone on `EventV2` * feat: add `hash` in `EventV1` with serialization disabled * docs: fix oai docs for `EventV1` * feat(aptos_api_types): add `try_into_v2_events` to convert `ContractEvent` to `EventV2` * feat: add `SUPRA_TRANSACTIONS_INCLUSION_PROOFS` feature flag * feat: replace rlp encoding with custom encoding to generate event hash * chore: replace `alloy` with `sha3` for `Keccak256` impl * fix: Keccack256 hash implementation using `sha3` --------- Co-authored-by: Nikita Puzankov <n.puzankov@supraoracles.com> Co-authored-by: Simon Chen <s.chen@supraoracles.com>
isaacdoidge
force-pushed
the
feature/dkg-integration
branch
from
50e66f6 to
2110584
Compare
…-qualified SSH submodule refs.
…. Added comments. Regenerated docs.
…re flags are set, so updated implementation accordingly and added notes. Also fixed supra_governance::reconfigure.
) * Started work towards adding leader ban registry config to genesis. * added remaining changes for the leader ban config at genesis * added deserilization step * added deserilization step * added deserilization step * fixed leader ban registry package for ban parameters * decoded correct version of params --------- Co-authored-by: Dhaval Purohit <d.purohit@supraoracles.com>
isaacdoidge
reviewed
Apr 1, 2026
Comment on lines
-33
to
+29
| enum ExpectedFailure { | ||
| // Move equivalent: `errors::invalid_argument(*)` | ||
| EpochNotCurrent = 0x10001, | ||
| TranscriptDeserializationFailed = 0x10002, | ||
| TranscriptVerificationFailed = 0x10003, | ||
|
|
||
| // Move equivalent: `errors::invalid_state(*)` | ||
| MissingResourceDKGState = 0x30001, | ||
| MissingResourceInprogressDKGSession = 0x30002, | ||
| MissingResourceConfiguration = 0x30003, | ||
| } | ||
|
|
||
| enum ExecutionFailure { | ||
| Expected(ExpectedFailure), | ||
| Expected(DiscardedVMStatus), |
There was a problem hiding this comment.
It seems that the previous expected errors were more precise. Is there any risk in categorizing errors incorrectly?
| }) | ||
| .map_err(|r| Unexpected(r.unwrap_err()))?; | ||
| .map_err(|e| expect_only_successful_execution(e, function_name.as_str(), log_context)) | ||
| .map_err(|r| ExecutionFailure::Unexpected(r.unwrap_err()))?; |
There was a problem hiding this comment.
What does the executor do when it receives an unexpected error? We need to ensure that malicious nodes cannot cause their peers to crash by sending invalid inputs.
Comment on lines
+2367
to
+2370
| session | ||
| .get_native_extensions() | ||
| .get_mut::<RandomnessContext>() | ||
| .mark_unbiasable(); |
There was a problem hiding this comment.
We need to gate this with the DKG feature flag and also add a comment explaining that the randomness is still biasable during the first epoch in which the feature is activated because threshold keys are not available until the end of that epoch.
Comment on lines
+66
to
+84
| // The order of the committee members is important for DKG. | ||
| // The order should correspond to the order of the validator committee. | ||
| // The output of the DKG has keys in the same order as the committee. | ||
| let dkg_committee = vector[]; | ||
| vector::for_each( | ||
| validator_committee, | ||
| |x| { | ||
| let validator_keys_bytes = validator_consensus_info::get_pk_bytes(&x); | ||
| let addr = validator_consensus_info::get_addr(&x); | ||
| vector::push_back( | ||
| &mut dkg_committee, | ||
| DkgNodeConfig { | ||
| addr, | ||
| identity: bcs::to_bytes(&addr), | ||
| dkg_pubkey: validator_keys_bytes | ||
| } | ||
| ); | ||
| } | ||
| ); |
There was a problem hiding this comment.
Seems like it might be more robust to sort by node index.
Comment on lines
+1849
to
+1853
| // on_new_epoch appends pending_active into active via pop_back, which reverses | ||
| // the order. Mirror that here so validator indices match after epoch transition. | ||
| let pending_idx = | ||
| num_cur_pending_actives - 1 - (candidate_idx - num_cur_actives); | ||
| vector::borrow(&cur_validator_set.pending_active, pending_idx) |
There was a problem hiding this comment.
Why do we not need to do this in next_validator_consensus_infos?
| /// validators that are already active. This is used when starting DKG: the committee | ||
| /// membership (active + pending_active) must reflect the next epoch, but node config should reflect | ||
| /// current-epoch for the active validators. | ||
| public fun next_epoch_validator_consensus_infos_for_dkg(): vector<ValidatorConsensusInfo> acquires ValidatorSet, ValidatorPerformance, StakePool, ValidatorFees, ValidatorConfig { |
There was a problem hiding this comment.
It seems like this function shares a lot of code with next_validator_consensus_infos. Is it possible to abstract the common functionality into new functions?
| pub bls_aggregate_signature: Vec<u8>, | ||
| pub signer_indices_clan_committee: Vec<u32>, | ||
| pub transaction_type: u8, | ||
| pub payload: HexEncodedBytes, |
There was a problem hiding this comment.
Please share this code if you've made the change.
| crate::gas_schedule::macros::define_gas_parameters!( | ||
| SupraStdlibGasParameters, | ||
| "supra_stdlib", | ||
| NativeGasParameters => .supra_stdlib, |
| @@ -1,5 +1,5 @@ | |||
| [toolchain] | |||
| channel = "1.78.0" | |||
| channel = "1.89.0" | |||
There was a problem hiding this comment.
Let's update to 1.93 if we can do so easily, since that's what we're moving to in smr-moonshot.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
No description provided.