Remove Move Loader's use of GenericError (#8289)

automerge

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]]

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"

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" }

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<AccountResource> {
         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.

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 <StatusCode as Into<u64>>::into(status.major_status).try_into() {
         Ok(u) => InstructionError::CustomError(u),
-        Err(_) => InstructionError::GenericError,
+        Err(_) => InstructionError::InvalidError,
     }
 }

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<T> DecodeError<T> 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());