Skip to content

Commit

Permalink
[libra_vm] Get rid of PublishModule capability
Browse files Browse the repository at this point in the history
Closes: diem#5051
Approved by: tzakian
  • Loading branch information
Runtian Zhou authored and bors-libra committed Jul 13, 2020
1 parent e17036b commit aa7d989
Show file tree
Hide file tree
Showing 27 changed files with 101 additions and 205 deletions.
2 changes: 1 addition & 1 deletion config/management/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Genesis {
let genesis = vm_genesis::encode_genesis_transaction_with_validator(
association_key,
&validators,
Some(libra_types::on_chain_config::VMPublishingOption::Open),
Some(libra_types::on_chain_config::VMPublishingOption::open()),
);

if let Some(path) = self.path {
Expand Down
2 changes: 1 addition & 1 deletion config/src/config/test_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl TestConfig {
operator_keypair: None,
execution_keypair: None,
temp_dir: None,
publishing_option: Some(VMPublishingOption::Open),
publishing_option: Some(VMPublishingOption::open()),
}
}

Expand Down
4 changes: 2 additions & 2 deletions execution/executor/tests/storage_integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ fn test_change_publishing_option_to_custom() {
genesis_key.clone(),
genesis_key.public_key(),
Some(encode_modify_publishing_option_script(
VMPublishingOption::CustomScripts,
VMPublishingOption::custom_scripts(),
)),
);

Expand Down Expand Up @@ -544,7 +544,7 @@ fn test_extend_whitelist() {
genesis_key.clone(),
genesis_key.public_key(),
Some(encode_modify_publishing_option_script(
VMPublishingOption::Locked(new_whitelist),
VMPublishingOption::locked(new_whitelist),
)),
);

Expand Down
6 changes: 3 additions & 3 deletions language/e2e-tests/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use libra_types::{
access_path::AccessPath,
account_config::{AccountResource, BalanceResource, CORE_CODE_ADDRESS},
block_metadata::{new_block_event_key, BlockMetadata, NewBlockEvent},
on_chain_config::{OnChainConfig, VMPublishingOption, ValidatorSet},
on_chain_config::{OnChainConfig, ScriptPublishingOption, VMPublishingOption, ValidatorSet},
transaction::{
SignedTransaction, Transaction, TransactionOutput, TransactionStatus, VMValidatorResult,
},
Expand Down Expand Up @@ -74,15 +74,15 @@ impl FakeExecutor {
Self::custom_genesis(
stdlib_modules(StdLibOptions::Compiled).to_vec(),
None,
VMPublishingOption::Locked(StdlibScript::whitelist()),
VMPublishingOption::locked(StdlibScript::whitelist()),
)
}

/// Creates an executor from the genesis file GENESIS_FILE_LOCATION with script/module
/// publishing options given by `publishing_options`. These can only be either `Open` or
/// `CustomScript`.
pub fn from_genesis_with_options(publishing_options: VMPublishingOption) -> Self {
if let VMPublishingOption::Locked(_) = publishing_options {
if let ScriptPublishingOption::Locked(_) = publishing_options.script_option {
panic!("Whitelisted transactions are not supported as a publishing option")
}

Expand Down
11 changes: 6 additions & 5 deletions language/e2e-tests/src/tests/module_publishing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use libra_types::{
// A module with an address different from the sender's address should be rejected
#[test]
fn bad_module_address() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

// create a transaction trying to publish a new module.
let account1 = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -68,7 +68,7 @@ fn bad_module_address() {
// Publishing a module named M under the same address twice should be rejected
#[test]
fn duplicate_module() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

let sequence_number = 2;
let account = AccountData::new(1_000_000, sequence_number);
Expand Down Expand Up @@ -120,7 +120,8 @@ fn duplicate_module() {
#[test]
pub fn test_publishing_no_modules_non_whitelist_script() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let mut executor =
FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create a transaction trying to publish a new module.
let sender = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -153,7 +154,7 @@ pub fn test_publishing_no_modules_non_whitelist_script() {
#[test]
pub fn test_publishing_no_modules_non_whitelist_script_proper_sender() {
// create a FakeExecutor with a genesis from file
let executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create a transaction trying to publish a new module.
let sender = Account::new_libra_root();
Expand Down Expand Up @@ -283,7 +284,7 @@ pub fn test_publishing_no_modules_invalid_sender() {
#[test]
pub fn test_publishing_allow_modules() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

// create a transaction trying to publish a new module.
let sender = AccountData::new(1_000_000, 10);
Expand Down
3 changes: 2 additions & 1 deletion language/e2e-tests/src/tests/peer_to_peer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ fn single_peer_to_peer_with_event() {
fn single_peer_to_peer_with_padding() {
::libra_logger::Logger::new().environment_only(true).init();
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let mut executor =
FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create and publish a sender with 1_000_000 coins and a receiver with 100_000 coins
let sender = AccountData::new(1_000_000, 10);
Expand Down
8 changes: 4 additions & 4 deletions language/e2e-tests/src/tests/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use vm::file_format::{

#[test]
fn script_code_unverifiable() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());
// create and publish sender
let sender = AccountData::new(1_000_000, 10);
executor.add_account_data(&sender);
Expand Down Expand Up @@ -62,7 +62,7 @@ fn script_code_unverifiable() {

#[test]
fn script_none_existing_module_dep() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());
// create and publish sender
let sender = AccountData::new(1_000_000, 10);
executor.add_account_data(&sender);
Expand Down Expand Up @@ -134,7 +134,7 @@ fn script_none_existing_module_dep() {

#[test]
fn script_non_existing_function_dep() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());
// create and publish sender
let sender = AccountData::new(1_000_000, 10);
executor.add_account_data(&sender);
Expand Down Expand Up @@ -206,7 +206,7 @@ fn script_non_existing_function_dep() {

#[test]
fn script_bad_sig_function_dep() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());
// create and publish sender
let sender = AccountData::new(1_000_000, 10);
executor.add_account_data(&sender);
Expand Down
14 changes: 8 additions & 6 deletions language/e2e-tests/src/tests/verify_txn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,8 @@ pub fn test_whitelist() {
#[test]
pub fn test_arbitrary_script_execution() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let mut executor =
FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create an empty transaction
let sender = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -402,7 +403,8 @@ pub fn test_arbitrary_script_execution() {
#[test]
pub fn test_publish_from_libra_root() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let mut executor =
FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create a transaction trying to publish a new module.
let sender = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -443,7 +445,7 @@ pub fn test_publish_from_libra_root() {
#[test]
pub fn test_no_publishing_libra_root_sender() {
// create a FakeExecutor with a genesis from file
let executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::CustomScripts);
let executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::custom_scripts());

// create a transaction trying to publish a new module.
let sender = Account::new_libra_root();
Expand Down Expand Up @@ -482,7 +484,7 @@ pub fn test_no_publishing_libra_root_sender() {
#[test]
pub fn test_open_publishing_invalid_address() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

// create a transaction trying to publish a new module.
let sender = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -535,7 +537,7 @@ pub fn test_open_publishing_invalid_address() {
#[test]
pub fn test_open_publishing() {
// create a FakeExecutor with a genesis from file
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

// create a transaction trying to publish a new module.
let sender = AccountData::new(1_000_000, 10);
Expand Down Expand Up @@ -575,7 +577,7 @@ pub fn test_open_publishing() {

#[test]
fn test_dependency_fails_verification() {
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::Open);
let mut executor = FakeExecutor::from_genesis_with_options(VMPublishingOption::open());

// Get a module that fails verification into the store.
let bad_module_code = "
Expand Down
2 changes: 1 addition & 1 deletion language/functional-tests/src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ pub fn eval<TComp: Compiler>(
})
.to_vec(),
Some(config.validator_accounts),
VMPublishingOption::Open,
VMPublishingOption::open(),
)
};
for data in config.accounts.values() {
Expand Down
2 changes: 1 addition & 1 deletion language/libra-vm/src/libra_transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl LibraVM {
)?;

// Publish the module
let module_address = if self.0.on_chain_config()?.publishing_option.is_open() {
let module_address = if self.0.on_chain_config()?.publishing_option.is_open_module() {
txn_data.sender()
} else {
account_config::CORE_CODE_ADDRESS
Expand Down
19 changes: 5 additions & 14 deletions language/libra-vm/src/libra_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use crate::{
access_path_cache::AccessPathCache,
counters::*,
create_access_path,
data_cache::{RemoteStorage, StateViewCache},
system_module_names::*,
transaction_metadata::TransactionMetadata,
Expand All @@ -13,7 +12,6 @@ use crate::{
use libra_logger::prelude::*;
use libra_state_view::StateView;
use libra_types::{
account_address::AccountAddress,
account_config,
contract_event::ContractEvent,
event::EventKey,
Expand Down Expand Up @@ -200,10 +198,12 @@ impl LibraVMImpl {
pub(crate) fn is_allowed_module(
&self,
txn_data: &TransactionMetadata,
remote_cache: &StateViewCache,
_remote_cache: &StateViewCache,
) -> Result<(), VMStatus> {
if !&self.on_chain_config()?.publishing_option.is_open()
&& !can_publish_modules(txn_data.sender(), remote_cache)
if !&self
.on_chain_config()?
.publishing_option
.is_allowed_module(&txn_data.sender)
{
warn!("[VM] Custom modules not allowed");
Err(VMStatus::Error(StatusCode::INVALID_MODULE_PUBLISHER))
Expand Down Expand Up @@ -418,15 +418,6 @@ impl<'a> LibraVMInternals<'a> {
}
}

fn can_publish_modules(sender: AccountAddress, remote_cache: &StateViewCache) -> bool {
let module_publishing_priv_path =
create_access_path(sender, module_publishing_capability_struct_tag());
match remote_cache.get(&module_publishing_priv_path) {
Ok(Some(_)) => true,
_ => false,
}
}

pub fn txn_effects_to_writeset_and_events_cached<C: AccessPathCache>(
ap_cache: &mut C,
effects: TransactionEffects,
Expand Down
16 changes: 1 addition & 15 deletions language/libra-vm/src/system_module_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@
//! Names of modules, functions, and types used by Libra System.

use libra_types::account_config;
use move_core_types::{
identifier::Identifier,
language_storage::{ModuleId, StructTag},
};
use move_core_types::{identifier::Identifier, language_storage::ModuleId};
use once_cell::sync::Lazy;

// Data to resolve basic account and transaction flow functions and structs
Expand Down Expand Up @@ -38,14 +35,3 @@ pub static BUMP_SEQUENCE_NUMBER_NAME: Lazy<Identifier> =
Lazy::new(|| Identifier::new("bump_sequence_number").unwrap());
pub static BLOCK_PROLOGUE: Lazy<Identifier> =
Lazy::new(|| Identifier::new("block_prologue").unwrap());
pub static MODULE_PUBLISHING_TYPE_NAME: Lazy<Identifier> =
Lazy::new(|| Identifier::new("PublishModule").unwrap());

pub fn module_publishing_capability_struct_tag() -> StructTag {
StructTag {
address: account_config::CORE_CODE_ADDRESS,
module: account_config::ACCOUNT_MODULE_IDENTIFIER.to_owned(),
name: MODULE_PUBLISHING_TYPE_NAME.to_owned(),
type_params: vec![],
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fun main() {}

//! new-transaction
//! sender: libraroot
//! args: b"01",
//! args: b"0100010000000000000000000000000a550c18",
script {
use 0x1::LibraVMConfig;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module FooConfig {

//! new-transaction
//! sender: libraroot
//! args: b"01",
//! args: b"0100010000000000000000000000000a550c18",
// Step 2: Change option to CustomScript
script {
use 0x1::LibraVMConfig;
Expand Down
Binary file modified language/stdlib/compiled/stdlib.mv
Binary file not shown.
1 change: 0 additions & 1 deletion language/stdlib/modules/Genesis.move
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module Genesis {
let dummy_auth_key_prefix = x"00000000000000000000000000000000";

Roles::grant_libra_root_role(lr_account);
LibraAccount::grant_module_publishing_privilege(lr_account);
Roles::grant_treasury_compliance_role(tc_account, lr_account);

// Event and On-chain config setup
Expand Down
14 changes: 0 additions & 14 deletions language/stdlib/modules/LibraAccount.move
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ module LibraAccount {
use 0x1::Option::{Self, Option};
use 0x1::Roles;

resource struct PublishModule {}

/// Every Libra account has a LibraAccount resource
resource struct LibraAccount {
/// The current authentication key.
Expand Down Expand Up @@ -129,18 +127,6 @@ module LibraAccount {
const EPROLOGUE_CANT_PAY_GAS_DEPOSIT: u64 = 5;
const EPROLOGUE_TRANSACTION_EXPIRED: u64 = 6;

/// Grants the ability to publish modules to the libra root account. The VM
/// will look for this resource to determine whether the sending account can publish modules.
/// Aborts if the `account` does not have the correct role (libra root).
public fun grant_module_publishing_privilege(account: &signer) {
// This is also an operational constraint since the VM will look for
// this specific address and publish these modules under the core code
// address.
assert(Roles::has_libra_root_role(account), ENOT_LIBRA_ROOT);
assert(Signer::address_of(account) == CoreAddresses::LIBRA_ROOT_ADDRESS(), EINVALID_SINGLETON_ADDRESS);
move_to(account, PublishModule{});
}

/// Initialize this module. This is only callable from genesis.
public fun initialize(
lr_account: &signer,
Expand Down
1 change: 0 additions & 1 deletion language/stdlib/modules/doc/Genesis.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
<b>let</b> dummy_auth_key_prefix = x"00000000000000000000000000000000";

<a href="Roles.md#0x1_Roles_grant_libra_root_role">Roles::grant_libra_root_role</a>(lr_account);
<a href="LibraAccount.md#0x1_LibraAccount_grant_module_publishing_privilege">LibraAccount::grant_module_publishing_privilege</a>(lr_account);
<a href="Roles.md#0x1_Roles_grant_treasury_compliance_role">Roles::grant_treasury_compliance_role</a>(tc_account, lr_account);

// <a href="Event.md#0x1_Event">Event</a> and On-chain config setup
Expand Down
Loading

0 comments on commit aa7d989

Please sign in to comment.