Fix(connector): Return an error when storage cannot be read instead o…

…f panicking (#501)

engine-standalone-storage/src/sync/mod.rs

@@ -201,7 +201,8 @@ fn non_submit_execute<'db>(
                 engine::Engine::new(relayer_address, env.current_account_id(), io, &env)?;
 
             if env.predecessor_account_id == env.current_account_id {
-                connector::EthConnectorContract::init_instance(io).ft_on_transfer(&engine, args)?;
+                connector::EthConnectorContract::init_instance(io)?
+                    .ft_on_transfer(&engine, args)?;
             } else {
                 engine.receive_erc20_tokens(
                     &env.predecessor_account_id,
@@ -216,7 +217,7 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::FtTransferCall(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             let promise_args = connector.ft_transfer_call(
                 env.predecessor_account_id.clone(),
                 env.current_account_id.clone(),
@@ -228,21 +229,21 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::ResolveTransfer(args, promise_result) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             connector.ft_resolve_transfer(args.clone(), promise_result.clone());
 
             None
         }
 
         TransactionKind::FtTransfer(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             connector.ft_transfer(&env.predecessor_account_id, args.clone())?;
 
             None
         }
 
         TransactionKind::Withdraw(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             connector.withdraw_eth_from_near(
                 &env.current_account_id,
                 &env.predecessor_account_id,
@@ -253,7 +254,7 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::Deposit(raw_proof) => {
-            let connector_contract = connector::EthConnectorContract::init_instance(io);
+            let connector_contract = connector::EthConnectorContract::init_instance(io)?;
             let promise_args = connector_contract.deposit(
                 raw_proof.clone(),
                 env.current_account_id(),
@@ -264,7 +265,7 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::FinishDeposit(finish_args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             let maybe_promise_args = connector.finish_deposit(
                 env.predecessor_account_id(),
                 env.current_account_id(),
@@ -276,7 +277,7 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::StorageDeposit(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             let _ = connector.storage_deposit(
                 env.predecessor_account_id,
                 Yocto::new(env.attached_deposit),
@@ -287,21 +288,21 @@ fn non_submit_execute<'db>(
         }
 
         TransactionKind::StorageUnregister(force) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             let _ = connector.storage_unregister(env.predecessor_account_id, *force)?;
 
             None
         }
 
         TransactionKind::StorageWithdraw(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             connector.storage_withdraw(&env.predecessor_account_id, args.clone())?;
 
             None
         }
 
         TransactionKind::SetPausedFlags(args) => {
-            let mut connector = connector::EthConnectorContract::init_instance(io);
+            let mut connector = connector::EthConnectorContract::init_instance(io)?;
             connector.set_paused_flags(args.clone());
 
             None
@@ -398,6 +399,7 @@ pub mod error {
         FtStorageFunding(fungible_token::error::StorageFundingError),
         InvalidAddress(aurora_engine_types::types::address::error::AddressError),
         ConnectorInit(connector::error::InitContractError),
+        ConnectorStorage(connector::error::StorageReadError),
     }
 
     impl From<engine::EngineStateError> for Error {
@@ -465,4 +467,10 @@ pub mod error {
             Self::ConnectorInit(e)
         }
     }
+
+    impl From<connector::error::StorageReadError> for Error {
+        fn from(e: connector::error::StorageReadError) -> Self {
+            Self::ConnectorStorage(e)
+        }
+    }
 }

engine-tests/src/test_utils/standalone/mocks/mod.rs

@@ -115,6 +115,7 @@ pub fn mint_evm_account<I: IO + Copy, E: Env>(
     io.remove_storage(&proof_key);
 
     aurora_engine::connector::EthConnectorContract::init_instance(io)
+        .unwrap()
         .finish_deposit(
             aurora_account_id.clone(),
             aurora_account_id.clone(),

engine/src/connector.rs

@@ -62,14 +62,14 @@ impl<I: IO + Copy> EthConnectorContract<I> {
     /// Init Eth-connector contract instance.
     /// Load contract data from storage and init I/O handler.
     /// Used as single point of contract access for various contract actions
-    pub fn init_instance(io: I) -> Self {
-        Self {
-            contract: get_contract_data(&io, &EthConnectorStorageId::Contract),
-            ft: get_contract_data::<FungibleToken, I>(&io, &EthConnectorStorageId::FungibleToken)
+    pub fn init_instance(io: I) -> Result<Self, error::StorageReadError> {
+        Ok(Self {
+            contract: get_contract_data(&io, &EthConnectorStorageId::Contract)?,
+            ft: get_contract_data::<FungibleToken, I>(&io, &EthConnectorStorageId::FungibleToken)?
                 .ops(io),
-            paused_mask: get_contract_data(&io, &EthConnectorStorageId::PausedMask),
+            paused_mask: get_contract_data(&io, &EthConnectorStorageId::PausedMask)?,
             io,
-        }
+        })
     }
 
     /// Create contract data - init eth-connector contract specific data.
@@ -687,11 +687,16 @@ fn construct_contract_key(suffix: &EthConnectorStorageId) -> Vec<u8> {
     crate::prelude::bytes_to_key(KeyPrefix::EthConnector, &[*suffix as u8])
 }
 
-fn get_contract_data<T: BorshDeserialize, I: IO>(io: &I, suffix: &EthConnectorStorageId) -> T {
+fn get_contract_data<T: BorshDeserialize, I: IO>(
+    io: &I,
+    suffix: &EthConnectorStorageId,
+) -> Result<T, error::StorageReadError> {
     io.read_storage(&construct_contract_key(suffix))
-        .expect("Failed read storage")
-        .to_value()
-        .unwrap()
+        .ok_or(error::StorageReadError::KeyNotFound)
+        .and_then(|x| {
+            x.to_value()
+                .map_err(|_| error::StorageReadError::BorshDeserialize)
+        })
 }
 
 /// Sets the contract data and returns it back
@@ -735,6 +740,21 @@ pub mod error {
 
     const PROOF_EXIST: &[u8; 15] = b"ERR_PROOF_EXIST";
 
+    #[cfg_attr(not(target_arch = "wasm32"), derive(Debug))]
+    pub enum StorageReadError {
+        KeyNotFound,
+        BorshDeserialize,
+    }
+
+    impl AsRef<[u8]> for StorageReadError {
+        fn as_ref(&self) -> &[u8] {
+            match self {
+                Self::KeyNotFound => b"ERR_CONNECTOR_STORAGE_KEY_NOT_FOUND",
+                Self::BorshDeserialize => b"ERR_FAILED_DESERIALIZE_CONNECTOR_DATA",
+            }
+        }
+    }
+
     #[cfg_attr(not(target_arch = "wasm32"), derive(Debug))]
     pub enum DepositError {
         Paused,

engine/src/engine.rs

@@ -1277,12 +1277,16 @@ pub fn set_balance<I: IO>(io: &mut I, address: &Address, balance: &Wei) {
 
 pub fn remove_balance<I: IO + Copy>(io: &mut I, address: &Address) {
     let balance = get_balance(io, address);
-    // Apply changes for eth-connector. The `unwrap` is safe here because (a) if the connector
-    // is implemented correctly then the total supply wll never underflow and (b) we are passing
-    // in the balance directly so there will always be enough balance.
+    // Apply changes for eth-connector. We ignore the `StorageReadError` intentionally since
+    // if we cannot read the storage then there is nothing to remove.
     EthConnectorContract::init_instance(*io)
-        .internal_remove_eth(address, balance)
-        .unwrap();
+        .map(|mut connector| {
+            // The `unwrap` is safe here because (a) if the connector
+            // is implemented correctly then the total supply will never underflow and (b) we are passing
+            // in the balance directly so there will always be enough balance.
+            connector.internal_remove_eth(address, balance).unwrap();
+        })
+        .ok();
     io.remove_storage(&address_to_key(KeyPrefix::Balance, address));
 }
 

engine/src/lib.rs

@@ -340,6 +340,7 @@ mod contract {
 
         if predecessor_account_id == current_account_id {
             EthConnectorContract::init_instance(io)
+                .sdk_unwrap()
                 .ft_on_transfer(&engine, &args)
                 .sdk_unwrap();
         } else {
@@ -522,6 +523,7 @@ mod contract {
         let current_account_id = io.current_account_id();
         let predecessor_account_id = io.predecessor_account_id();
         let result = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .withdraw_eth_from_near(&current_account_id, &predecessor_account_id, args)
             .sdk_unwrap();
         let result_bytes = result.try_to_vec().sdk_expect("ERR_SERIALIZE");
@@ -535,6 +537,7 @@ mod contract {
         let current_account_id = io.current_account_id();
         let predecessor_account_id = io.predecessor_account_id();
         let promise_args = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .deposit(raw_proof, current_account_id, predecessor_account_id)
             .sdk_unwrap();
         let promise_id = io.promise_create_with_callback(&promise_args);
@@ -564,6 +567,7 @@ mod contract {
         let current_account_id = io.current_account_id();
         let predecessor_account_id = io.predecessor_account_id();
         let maybe_promise_args = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .finish_deposit(
                 predecessor_account_id,
                 current_account_id,
@@ -583,27 +587,35 @@ mod contract {
         let mut io = Runtime;
         let args: IsUsedProofCallArgs = io.read_input_borsh().sdk_unwrap();
 
-        let is_used_proof = EthConnectorContract::init_instance(io).is_used_proof(args.proof);
+        let is_used_proof = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .is_used_proof(args.proof);
         let res = is_used_proof.try_to_vec().unwrap();
         io.return_output(&res[..]);
     }
 
     #[no_mangle]
     pub extern "C" fn ft_total_supply() {
         let io = Runtime;
-        EthConnectorContract::init_instance(io).ft_total_eth_supply_on_near();
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .ft_total_eth_supply_on_near();
     }
 
     #[no_mangle]
     pub extern "C" fn ft_total_eth_supply_on_near() {
         let io = Runtime;
-        EthConnectorContract::init_instance(io).ft_total_eth_supply_on_near();
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .ft_total_eth_supply_on_near();
     }
 
     #[no_mangle]
     pub extern "C" fn ft_total_eth_supply_on_aurora() {
         let io = Runtime;
-        EthConnectorContract::init_instance(io).ft_total_eth_supply_on_aurora();
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .ft_total_eth_supply_on_aurora();
     }
 
     #[no_mangle]
@@ -613,14 +625,17 @@ mod contract {
             parse_json(&io.read_input().to_vec()).sdk_unwrap(),
         )
         .sdk_unwrap();
-        EthConnectorContract::init_instance(io).ft_balance_of(args);
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .ft_balance_of(args);
     }
 
     #[no_mangle]
     pub extern "C" fn ft_balance_of_eth() {
         let io = Runtime;
         let args: parameters::BalanceOfEthCallArgs = io.read_input().to_value().sdk_unwrap();
         EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .ft_balance_of_eth_on_aurora(args)
             .sdk_unwrap();
     }
@@ -635,6 +650,7 @@ mod contract {
         )
         .sdk_unwrap();
         EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .ft_transfer(&predecessor_account_id, args)
             .sdk_unwrap();
     }
@@ -651,7 +667,9 @@ mod contract {
         let args: ResolveTransferCallArgs = io.read_input().to_value().sdk_unwrap();
         let promise_result = io.promise_result(0).sdk_unwrap();
 
-        EthConnectorContract::init_instance(io).ft_resolve_transfer(args, promise_result);
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .ft_resolve_transfer(args, promise_result);
     }
 
     #[no_mangle]
@@ -668,6 +686,7 @@ mod contract {
         let current_account_id = io.current_account_id();
         let predecessor_account_id = io.predecessor_account_id();
         let promise_args = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .ft_transfer_call(
                 predecessor_account_id,
                 current_account_id,
@@ -686,6 +705,7 @@ mod contract {
         let predecessor_account_id = io.predecessor_account_id();
         let amount = Yocto::new(io.attached_deposit());
         let maybe_promise = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .storage_deposit(predecessor_account_id, amount, args)
             .sdk_unwrap();
         if let Some(promise) = maybe_promise {
@@ -700,6 +720,7 @@ mod contract {
         let predecessor_account_id = io.predecessor_account_id();
         let force = parse_json(&io.read_input().to_vec()).and_then(|args| args.bool("force").ok());
         let maybe_promise = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .storage_unregister(predecessor_account_id, force)
             .sdk_unwrap();
         if let Some(promise) = maybe_promise {
@@ -715,6 +736,7 @@ mod contract {
             StorageWithdrawCallArgs::from(parse_json(&io.read_input().to_vec()).sdk_unwrap());
         let predecessor_account_id = io.predecessor_account_id();
         EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
             .storage_withdraw(&predecessor_account_id, args)
             .sdk_unwrap()
     }
@@ -726,13 +748,17 @@ mod contract {
             parse_json(&io.read_input().to_vec()).sdk_unwrap(),
         )
         .sdk_unwrap();
-        EthConnectorContract::init_instance(io).storage_balance_of(args)
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .storage_balance_of(args)
     }
 
     #[no_mangle]
     pub extern "C" fn get_paused_flags() {
         let mut io = Runtime;
-        let paused_flags = EthConnectorContract::init_instance(io).get_paused_flags();
+        let paused_flags = EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .get_paused_flags();
         let data = paused_flags.try_to_vec().expect(ERR_FAILED_PARSE);
         io.return_output(&data[..]);
     }
@@ -743,13 +769,17 @@ mod contract {
         io.assert_private_call().sdk_unwrap();
 
         let args: PauseEthConnectorCallArgs = io.read_input_borsh().sdk_unwrap();
-        EthConnectorContract::init_instance(io).set_paused_flags(args);
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .set_paused_flags(args);
     }
 
     #[no_mangle]
     pub extern "C" fn get_accounts_counter() {
         let io = Runtime;
-        EthConnectorContract::init_instance(io).get_accounts_counter();
+        EthConnectorContract::init_instance(io)
+            .sdk_unwrap()
+            .get_accounts_counter();
     }
 
     #[no_mangle]