From 644a7f9a4459826994945d045261a93bf4bbffd8 Mon Sep 17 00:00:00 2001 From: Jack May Date: Fri, 14 Feb 2020 14:49:21 -0800 Subject: [PATCH] Remove Move Loader's use of GenericError (#8289) automerge --- Cargo.lock | 3 +++ programs/move_loader/Cargo.lock | 11 ++++----- programs/move_loader/Cargo.toml | 3 +++ programs/move_loader/src/data_store.rs | 4 +++- programs/move_loader/src/error_mappers.rs | 2 +- programs/move_loader/src/processor.rs | 27 ++++++++++++++++++----- 6 files changed, 38 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 428d9dfd7a..0bc2ff980f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4214,6 +4214,8 @@ dependencies = [ "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "indexmap 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "num-derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", + "num-traits 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", "serde_bytes 0.11.3 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", @@ -4231,6 +4233,7 @@ dependencies = [ "solana_libra_vm_cache_map 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", "solana_libra_vm_runtime 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", "solana_libra_vm_runtime_types 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", + "thiserror 1.0.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/programs/move_loader/Cargo.lock b/programs/move_loader/Cargo.lock index e987115e25..7c8883bed5 100644 --- a/programs/move_loader/Cargo.lock +++ b/programs/move_loader/Cargo.lock @@ -1017,7 +1017,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "memmap" -version = "0.6.2" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2097,10 +2097,10 @@ name = "solana-move-loader-program" version = "0.24.0" dependencies = [ "bincode 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "byteorder 1.3.2 (registry+https://github.com/rust-lang/crates.io-index)", "indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", - "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "num-derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", + "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", "serde_bytes 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)", "serde_derive 1.0.104 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2118,6 +2118,7 @@ dependencies = [ "solana_libra_vm_cache_map 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", "solana_libra_vm_runtime 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", "solana_libra_vm_runtime_types 0.0.1-sol4 (registry+https://github.com/rust-lang/crates.io-index)", + "thiserror 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -2135,7 +2136,7 @@ dependencies = [ "hmac 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", - "memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)", + "memmap 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "num-traits 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)", "pbkdf2 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3378,7 +3379,7 @@ dependencies = [ "checksum mach_o_sys 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3e854583a83f20cf329bb9283366335387f7db59d640d1412167e05fedb98826" "checksum matches 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "7ffc5c5338469d4d3ea17d269fa8ea3512ad247247c30bd2df69e68309ed0a08" "checksum memchr 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "88579771288728879b57485cc7d6b07d648c9f0141eb955f8ab7f9d45394468e" -"checksum memmap 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e2ffa2c986de11a9df78620c01eeaaf27d94d3ff02bf81bfcca953102dd0c6ff" +"checksum memmap 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "6585fd95e7bb50d6cc31e20d4cf9afb4e2ba16c5846fc76793f11218da9c475b" "checksum memoffset 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ce6075db033bbbb7ee5a0bbd3a3186bbae616f57fb001c485c7ff77955f8177f" "checksum memsec 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)" = "ccabb92f665f997bcb4f3ade019a8e07315148d8bcef3e65fbc5dbd65a22eb04" "checksum mime 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)" = "dd1d63acd1b78403cc0c325605908475dd9b9a3acbf65ed8bcab97e27014afcf" diff --git a/programs/move_loader/Cargo.toml b/programs/move_loader/Cargo.toml index fad4331b7d..a2199103c7 100644 --- a/programs/move_loader/Cargo.toml +++ b/programs/move_loader/Cargo.toml @@ -23,8 +23,11 @@ bytecode_verifier = { version = "0.0.1-sol4", package = "solana_libra_bytecode_v canonical_serialization = { version = "0.0.1-sol4", package = "solana_libra_canonical_serialization" } compiler = { version = "0.0.1-sol4", package = "solana_libra_compiler" } failure = { version = "0.0.1-sol4", package = "solana_libra_failure_ext" } +num-derive = { version = "0.3" } +num-traits = { version = "0.2" } state_view = { version = "0.0.1-sol4", package = "solana_libra_state_view" } stdlib = { version = "0.0.1-sol4", package = "solana_libra_stdlib" } +thiserror = "1.0" types = { version = "0.0.1-sol4", package = "solana_libra_types" } vm = { version = "0.0.1-sol4", package = "solana_libra_vm" } vm_cache_map = { version = "0.0.1-sol4", package = "solana_libra_vm_cache_map" } diff --git a/programs/move_loader/src/data_store.rs b/programs/move_loader/src/data_store.rs index 9f0a75c7c9..8b07bdc33d 100644 --- a/programs/move_loader/src/data_store.rs +++ b/programs/move_loader/src/data_store.rs @@ -65,7 +65,9 @@ impl DataStore { /// Read an account's resource pub fn read_account_resource(&self, addr: &AccountAddress) -> Option { let access_path = create_access_path(&addr, account_config::account_struct_tag()); - self.data.get(&access_path).and_then(|blob| { SimpleDeserializer::deserialize(blob).ok() }) + self.data + .get(&access_path) + .and_then(|blob| SimpleDeserializer::deserialize(blob).ok()) } /// Sets a (key, value) pair within this data store. diff --git a/programs/move_loader/src/error_mappers.rs b/programs/move_loader/src/error_mappers.rs index cbcb3e7faf..038e09b51f 100644 --- a/programs/move_loader/src/error_mappers.rs +++ b/programs/move_loader/src/error_mappers.rs @@ -42,6 +42,6 @@ pub fn map_err_vm_status(status: VMStatus) -> InstructionError { // The only defined StatusCode that fails is StatusCode::UNKNOWN_ERROR match >::into(status.major_status).try_into() { Ok(u) => InstructionError::CustomError(u), - Err(_) => InstructionError::GenericError, + Err(_) => InstructionError::InvalidError, } } diff --git a/programs/move_loader/src/processor.rs b/programs/move_loader/src/processor.rs index aa138b5f1d..b887c07016 100644 --- a/programs/move_loader/src/processor.rs +++ b/programs/move_loader/src/processor.rs @@ -3,16 +3,19 @@ use crate::data_store::DataStore; use crate::error_mappers::*; use bytecode_verifier::verifier::{VerifiedModule, VerifiedScript}; use log::*; +use num_derive::{FromPrimitive, ToPrimitive}; use serde_derive::{Deserialize, Serialize}; use solana_sdk::{ account::KeyedAccount, account_utils::State, instruction::InstructionError, move_loader::id, + program_utils::DecodeError, program_utils::{is_executable, limited_deserialize, next_keyed_account}, pubkey::Pubkey, sysvar::rent, }; +use thiserror::Error; use types::{ account_address::AccountAddress, account_config, @@ -36,6 +39,20 @@ use vm_runtime::{ }; use vm_runtime_types::value::Value; +#[derive(Error, Debug, Serialize, Clone, PartialEq, FromPrimitive, ToPrimitive)] +pub enum MoveError { + #[error("Invalid Move data store")] + InvalidDataStore, + #[error("Invaid Move script")] + InvalidScript, +} + +impl DecodeError for MoveError { + fn type_of() -> &'static str { + "MoveError" + } +} + /// Instruction data passed to perform a loader operation, must be based /// on solana_sdk::loader_instruction::LoaderInstruction #[derive(Serialize, Deserialize, Debug)] @@ -210,7 +227,7 @@ impl MoveProcessor { ) -> Result<(), InstructionError> { let mut write_sets = data_store .into_write_sets() - .map_err(|_| InstructionError::GenericError)?; + .map_err(|_| MoveError::InvalidDataStore)?; let mut keyed_accounts_iter = keyed_accounts.iter(); @@ -249,7 +266,7 @@ impl MoveProcessor { } if !write_sets.is_empty() { debug!("Error: Missing keyed accounts"); - return Err(InstructionError::GenericError); + return Err(InstructionError::NotEnoughAccountKeys); } Ok(()) } @@ -302,7 +319,7 @@ impl MoveProcessor { Ok(script) => script, Err((_, errors)) => { if errors.is_empty() { - return Err(InstructionError::GenericError); + return Err(MoveError::InvalidScript.into()); } else { return Err(map_err_vm_status(errors[0].clone())); } @@ -333,7 +350,7 @@ impl MoveProcessor { let mut write_sets = data_store .into_write_sets() - .map_err(|_| InstructionError::GenericError)?; + .map_err(|_| MoveError::InvalidDataStore)?; let write_set = write_sets .remove(&pubkey_to_address(finalized.unsigned_key())) @@ -345,7 +362,7 @@ impl MoveProcessor { if !write_sets.is_empty() { debug!("Error: Missing keyed accounts"); - return Err(InstructionError::GenericError); + return Err(InstructionError::NotEnoughAccountKeys); } info!("Finalize module: {:?}", finalized.unsigned_key());