Skip to content

Ensured that the Supra protocol configuration settings are stored in the Move state. #84

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

Merged
merged 4 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Updates missed in last commit.
  • Loading branch information
isaacdoidge committed Sep 26, 2024
commit 24c15964184472fbcf6d384ab0111f3d4859f6f1
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
spec supra_framework::supra_config {
/// <high-level-req>
/// No.: 1
/// Requirement: During genesis, the Supra framework account should be assigned the supra config resource.
/// Criticality: Medium
/// Implementation: The supra_config::initialize function calls the assert_supra_framework function to ensure
/// that the signer is the supra_framework and then assigns the SupraConfig resource to it.
/// Enforcement: Formally verified via [high-level-req-1](initialize).
///
/// No.: 2
/// Requirement: Only aptos framework account is allowed to update the supra protocol configuration.

Choose a reason for hiding this comment

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

Suggested change
/// Requirement: Only aptos framework account is allowed to update the supra protocol configuration.
/// Requirement: Only supra framework account is allowed to update the supra protocol configuration.

Should this be Supra framework?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I didn't update the docs very carefully. We'll have to update that in future PR.

/// Criticality: Medium
/// Implementation: The supra_config::set function ensures that the signer is supra_framework.
/// Enforcement: Formally verified via [high-level-req-2](set).
///
/// No.: 3
/// Requirement: Only a valid configuration can be used during initialization and update.
/// Criticality: Medium
/// Implementation: Both the initialize and set functions validate the config by ensuring its length to be greater
/// than 0.
/// Enforcement: Formally verified via [high-level-req-3.1](initialize) and [high-level-req-3.2](set).
/// </high-level-req>
///
spec module {
use supra_framework::chain_status;
pragma verify = true;
pragma aborts_if_is_strict;
invariant [suspendable] chain_status::is_operating() ==> exists<SupraConfig>(@supra_framework);
}

/// Ensure caller is admin.
/// Aborts if StateStorageUsage already exists.
spec initialize(supra_framework: &signer, config: vector<u8>) {
use std::signer;
let addr = signer::address_of(supra_framework);
/// [high-level-req-1]
aborts_if !system_addresses::is_supra_framework_address(addr);
aborts_if exists<SupraConfig>(@supra_framework);
/// [high-level-req-3.1]
aborts_if !(len(config) > 0);
ensures global<SupraConfig>(addr) == SupraConfig { config };
}

/// Ensure the caller is admin and `SupraConfig` should exist.
/// When setting now time must be later than last_reconfiguration_time.
spec set(account: &signer, config: vector<u8>) {
use supra_framework::chain_status;
use supra_framework::timestamp;
use std::signer;
use supra_framework::stake;
use supra_framework::coin::CoinInfo;
use supra_framework::supra_coin::SupraCoin;
use supra_framework::transaction_fee;
use supra_framework::staking_config;

// TODO: set because of timeout (property proved)
pragma verify_duration_estimate = 600;
include transaction_fee::RequiresCollectedFeesPerValueLeqBlockAptosSupply;
include staking_config::StakingRewardsConfigRequirement;
let addr = signer::address_of(account);
/// [high-level-req-2]
aborts_if !system_addresses::is_supra_framework_address(addr);
aborts_if !exists<SupraConfig>(@supra_framework);
/// [high-level-req-3.2]
aborts_if !(len(config) > 0);

requires chain_status::is_genesis();
requires timestamp::spec_now_microseconds() >= reconfiguration::last_reconfiguration_time();
requires exists<stake::ValidatorFees>(@supra_framework);
requires exists<CoinInfo<SupraCoin>>(@supra_framework);
ensures global<SupraConfig>(@supra_framework).config == config;
}

spec set_for_next_epoch(account: &signer, config: vector<u8>) {
include config_buffer::SetForNextEpochAbortsIf;
}

spec on_new_epoch(framework: &signer) {
requires @supra_framework == std::signer::address_of(framework);
include config_buffer::OnNewEpochRequirement<SupraConfig>;
aborts_if false;

Choose a reason for hiding this comment

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

Curious: What does this check do?

Copy link
Author

Choose a reason for hiding this comment

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

Not sure tbh. I just copied the spec for consensus_config and changed it in the same way that I changed the contract itself. @axiongsupra will double check to make sure everything's alright.

}
}

Choose a reason for hiding this comment

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

@isaacdoidge did you run the prover on this? If not, perhaps this file can be dropped for now or file an issue and assign it to @axiongsupra who will rewrite (if necessary) the specs.

Copy link
Author

Choose a reason for hiding this comment

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

I changed the values that should have needed to change according to the modifications that I made to the contract. What command do I use to run the prover?

Copy link
Author

Choose a reason for hiding this comment

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

As we discussed, I'll raise a new issue so that @axiongsupra can double check.

Choose a reason for hiding this comment

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

@isaacdoidge @sjoshisupra I confirm the prover works fine with the spec if I merge #63. I will looking to the details and see whether the property is correct.

2 changes: 1 addition & 1 deletion aptos-move/framework/supra-framework/sources/genesis.move
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ module supra_framework::genesis {

consensus_config::initialize(&supra_framework_account, consensus_config);
execution_config::set(&supra_framework_account, execution_config);
supra_config::set(&supra_framework_account, supra_config);
supra_config::initialize(&supra_framework_account, supra_config);
version::initialize(&supra_framework_account, initial_version);
stake::initialize(&supra_framework_account);
staking_config::initialize(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ spec supra_framework::genesis {
ensures exists<supra_governance::GovernanceResponsbility>(@supra_framework);
ensures exists<consensus_config::ConsensusConfig>(@supra_framework);
ensures exists<execution_config::ExecutionConfig>(@supra_framework);
ensures exists<supra_config::SupraConfig>(@supra_framework);
ensures exists<version::Version>(@supra_framework);
ensures exists<stake::ValidatorSet>(@supra_framework);
ensures exists<stake::ValidatorPerformance>(@supra_framework);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ module supra_framework::reconfiguration_with_dkg {
use supra_framework::reconfiguration;
use supra_framework::reconfiguration_state;
use supra_framework::stake;
use supra_framework::supra_config;
use supra_framework::system_addresses;
friend supra_framework::block;
friend supra_framework::supra_governance;
Expand Down Expand Up @@ -48,6 +49,7 @@ module supra_framework::reconfiguration_with_dkg {
dkg::try_clear_incomplete_session(framework);
consensus_config::on_new_epoch(framework);
execution_config::on_new_epoch(framework);
supra_config::on_new_epoch(framework);
gas_schedule::on_new_epoch(framework);
std::version::on_new_epoch(framework);
features::on_new_epoch(framework);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ spec supra_framework::reconfiguration_with_dkg {
use supra_framework::jwks;
use supra_framework::randomness_config;
use supra_framework::jwk_consensus_config;
use supra_framework::supra_config;
framework: signer;
requires signer::address_of(framework) == @supra_framework;
requires chain_status::is_operating();
Expand All @@ -53,6 +54,7 @@ spec supra_framework::reconfiguration_with_dkg {
include config_buffer::OnNewEpochRequirement<gas_schedule::GasScheduleV2>;
include config_buffer::OnNewEpochRequirement<execution_config::ExecutionConfig>;
include config_buffer::OnNewEpochRequirement<consensus_config::ConsensusConfig>;
include config_buffer::OnNewEpochRequirement<supra_config::SupraConfig>;
include config_buffer::OnNewEpochRequirement<jwks::SupportedOIDCProviders>;
include config_buffer::OnNewEpochRequirement<randomness_config::RandomnessConfig>;
include config_buffer::OnNewEpochRequirement<randomness_config_seqnum::RandomnessConfigSeqNum>;
Expand Down