From 7742836e118c8868ac42a22978578400720be615 Mon Sep 17 00:00:00 2001 From: Oleksandr Anyshchenko Date: Fri, 10 Feb 2023 18:00:20 +0100 Subject: [PATCH] chore: versioning state --- .../src/relayer_db/mod.rs | 7 +- engine-tests/src/tests/sanity.rs | 1 + engine-tests/src/tests/standalone/sanity.rs | 5 +- engine-types/src/types/mod.rs | 4 - engine/src/deposit_event.rs | 8 +- engine/src/engine.rs | 6 +- engine/src/lib.rs | 30 ++-- engine/src/state.rs | 144 +++++++++--------- 8 files changed, 102 insertions(+), 103 deletions(-) diff --git a/engine-standalone-storage/src/relayer_db/mod.rs b/engine-standalone-storage/src/relayer_db/mod.rs index e2a1d4a6e..9cfb4466a 100644 --- a/engine-standalone-storage/src/relayer_db/mod.rs +++ b/engine-standalone-storage/src/relayer_db/mod.rs @@ -198,6 +198,7 @@ pub mod error { mod test { use super::FallibleIterator; use crate::sync::types::{TransactionKind, TransactionMessage}; + use aurora_engine::state::EngineStateV2; use aurora_engine::{connector, parameters, state}; use aurora_engine_types::H256; @@ -210,12 +211,12 @@ mod test { fn test_fill_db() { let mut storage = crate::Storage::open("rocks_tmp/").unwrap(); let mut connection = super::connect_without_tls(&Default::default()).unwrap(); - let engine_state = state::EngineState { + let engine_state = state::EngineState::V2(EngineStateV2 { chain_id: aurora_engine_types::types::u256_to_arr(&1313161555.into()), owner_id: "aurora".parse().unwrap(), upgrade_delay_blocks: 0, default_gas_token: Default::default(), - }; + }); // Initialize engine and connector states in storage. // Use explicit scope so borrows against `storage` are dropped before processing DB rows. @@ -234,7 +235,7 @@ mod test { state::set_state(&mut local_io, engine_state.clone()).unwrap(); connector::EthConnectorContract::create_contract( io, - engine_state.owner_id.clone(), + engine_state.owner_id(), parameters::InitCallArgs { prover_account: "prover.bridge.near".parse().unwrap(), eth_custodian_address: "6bfad42cfc4efc96f529d786d643ff4a8b89fa52" diff --git a/engine-tests/src/tests/sanity.rs b/engine-tests/src/tests/sanity.rs index b94e14f86..cb7766765 100644 --- a/engine-tests/src/tests/sanity.rs +++ b/engine-tests/src/tests/sanity.rs @@ -141,6 +141,7 @@ fn test_state_format() { }; let state: aurora_engine::state::EngineState = args.into(); let expected_hex: String = [ + "00", // V2 - "00", V1 - "01", "000000000000000000000000000000000000000000000000000000000000029a", "04000000626f7373", "0300000000000000", diff --git a/engine-tests/src/tests/standalone/sanity.rs b/engine-tests/src/tests/standalone/sanity.rs index cf78d0d77..187d32372 100644 --- a/engine-tests/src/tests/standalone/sanity.rs +++ b/engine-tests/src/tests/standalone/sanity.rs @@ -1,3 +1,4 @@ +use aurora_engine::state::EngineStateV2; use aurora_engine::{engine, state}; use aurora_engine_sdk::env::DEFAULT_PREPAID_GAS; use aurora_engine_test_doubles::io::{Storage, StoragePointer}; @@ -15,12 +16,12 @@ fn test_deploy_code() { buf }; let owner_id: AccountId = "aurora".parse().unwrap(); - let state = state::EngineState { + let state = state::EngineState::V2(EngineStateV2 { chain_id, owner_id: owner_id.clone(), upgrade_delay_blocks: 0, default_gas_token: Default::default(), - }; + }); let origin = Address::new(H160([0u8; 20])); let storage = RefCell::new(Storage::default()); let io = StoragePointer(&storage); diff --git a/engine-types/src/types/mod.rs b/engine-types/src/types/mod.rs index 82e666199..2efe0b2b4 100644 --- a/engine-types/src/types/mod.rs +++ b/engine-types/src/types/mod.rs @@ -133,10 +133,6 @@ impl Stack { } } -pub fn str_from_slice(inp: &[u8]) -> &str { - str::from_utf8(inp).unwrap() -} - #[cfg(test)] mod tests { use super::*; diff --git a/engine/src/deposit_event.rs b/engine/src/deposit_event.rs index 496d24a2e..dc34a4fb0 100644 --- a/engine/src/deposit_event.rs +++ b/engine/src/deposit_event.rs @@ -527,10 +527,8 @@ mod tests { let parse_error = TokenMessageData::parse_event_message_and_prepare_token_message_data(message, fee) .unwrap_err(); - let actual_parse_error = std::str::from_utf8(parse_error.as_ref()).unwrap(); - let expected_parse_error = AddressError::IncorrectLength.to_string(); - assert_eq!(expected_parse_error, actual_parse_error); + assert_eq!(parse_error.as_ref(), AddressError::IncorrectLength.as_ref()); } #[test] @@ -541,10 +539,8 @@ mod tests { let parse_error = TokenMessageData::parse_event_message_and_prepare_token_message_data(message, fee) .unwrap_err(); - let actual_parse_error = std::str::from_utf8(parse_error.as_ref()).unwrap(); - let expected_parse_error = AddressError::FailedDecodeHex.to_string(); - assert_eq!(expected_parse_error, actual_parse_error); + assert_eq!(parse_error.as_ref(), AddressError::FailedDecodeHex.as_ref()); } #[test] diff --git a/engine/src/engine.rs b/engine/src/engine.rs index b00b8f7f4..c46cfa208 100644 --- a/engine/src/engine.rs +++ b/engine/src/engine.rs @@ -872,7 +872,7 @@ pub fn submit( // Validate the chain ID, if provided inside the signature: if let Some(chain_id) = transaction.chain_id { - if U256::from(chain_id) != U256::from(state.chain_id) { + if U256::from(chain_id) != U256::from(state.chain_id()) { return Err(EngineErrorKind::InvalidChainId.into()); } } @@ -1535,7 +1535,7 @@ impl<'env, I: IO + Copy, E: Env> Backend for Engine<'env, I, E> { if idx.saturating_sub(U256::from(256)) <= number && number < idx { // since `idx` comes from `u64` it is always safe to downcast `number` from `U256` compute_block_hash( - self.state.chain_id, + self.state.chain_id(), number.low_u64(), self.current_account_id.as_bytes(), ) @@ -1595,7 +1595,7 @@ impl<'env, I: IO + Copy, E: Env> Backend for Engine<'env, I, E> { /// Returns the states chain ID. fn chain_id(&self) -> U256 { - U256::from(self.state.chain_id) + U256::from(self.state.chain_id()) } /// Checks if an address exists. diff --git a/engine/src/lib.rs b/engine/src/lib.rs index a33e765fd..08fae60b6 100644 --- a/engine/src/lib.rs +++ b/engine/src/lib.rs @@ -94,7 +94,9 @@ mod contract { near_account_to_evm_address, SdkExpect, SdkProcess, SdkUnwrap, }; use crate::prelude::storage::{bytes_to_key, KeyPrefix}; - use crate::prelude::{sdk, u256_to_arr, Address, PromiseResult, Yocto, ERR_FAILED_PARSE, H256}; + use crate::prelude::{ + sdk, u256_to_arr, Address, PromiseResult, ToString, Yocto, ERR_FAILED_PARSE, H256, + }; use crate::{errors, pausables, state}; use aurora_engine_sdk::env::Env; use aurora_engine_sdk::io::{StorageIntermediate, IO}; @@ -140,7 +142,7 @@ mod contract { pub extern "C" fn get_owner() { let mut io = Runtime; let state = state::get_state(&io).sdk_unwrap(); - io.return_output(state.owner_id.as_bytes()); + io.return_output(state.owner_id().as_bytes()); } /// Get bridge prover id for this contract. @@ -155,7 +157,7 @@ mod contract { #[no_mangle] pub extern "C" fn get_chain_id() { let mut io = Runtime; - io.return_output(&state::get_state(&io).sdk_unwrap().chain_id) + io.return_output(&state::get_state(&io).sdk_unwrap().chain_id()) } #[no_mangle] @@ -163,7 +165,7 @@ mod contract { let mut io = Runtime; let state = state::get_state(&io).sdk_unwrap(); let index = internal_get_upgrade_index(); - io.return_output(&(index + state.upgrade_delay_blocks).to_le_bytes()) + io.return_output(&(index + state.upgrade_delay_blocks()).to_le_bytes()) } /// Stage new code for deployment. @@ -187,7 +189,7 @@ mod contract { let state = state::get_state(&io).sdk_unwrap(); require_owner_only(&state, &io.predecessor_account_id()); let index = internal_get_upgrade_index(); - if io.block_height() <= index + state.upgrade_delay_blocks { + if io.block_height() <= index + state.upgrade_delay_blocks() { sdk::panic_utf8(errors::ERR_NOT_ALLOWED_TOO_EARLY); } Runtime::self_deploy(&bytes_to_key(KeyPrefix::Config, CODE_KEY)); @@ -197,13 +199,7 @@ mod contract { /// to make any necessary changes to the state such that it aligns with the newly deployed /// code. #[no_mangle] - pub extern "C" fn state_migration() { - let mut io = Runtime; - io.assert_private_call().sdk_unwrap(); - - // TODO: Must be removed after migration. - state::legacy::migrate_state(&mut io).sdk_unwrap(); - } + pub extern "C" fn state_migration() {} /// Resumes previously [`paused`] precompiles. /// @@ -479,7 +475,7 @@ mod contract { let block_height = io.read_input_borsh().sdk_unwrap(); let account_id = io.current_account_id(); let chain_id = state::get_state(&io) - .map(|state| state.chain_id) + .map(|state| state.chain_id()) .sdk_unwrap(); let block_hash = crate::engine::compute_block_hash(chain_id, block_height, account_id.as_bytes()); @@ -531,7 +527,7 @@ mod contract { let mut state = state::get_state(&io).sdk_unwrap(); require_owner_only(&state, &io.predecessor_account_id()); let args: BeginChainArgs = io.read_input_borsh().sdk_unwrap(); - state.chain_id = args.chain_id; + state.set_chain_id(args.chain_id); state::set_state(&mut io, state).sdk_unwrap(); // set genesis block balances for account_balance in args.genesis_alloc { @@ -542,7 +538,7 @@ mod contract { ) } // return new chain ID - io.return_output(&state::get_state(&io).sdk_unwrap().chain_id) + io.return_output(&state::get_state(&io).sdk_unwrap().chain_id()) } #[cfg(feature = "evm_bully")] @@ -911,7 +907,7 @@ mod contract { #[no_mangle] pub extern "C" fn mint_account() { use crate::connector::ZERO_ATTACHED_BALANCE; - use crate::prelude::{NEP141Wei, ToString, U256}; + use crate::prelude::{NEP141Wei, U256}; use evm::backend::ApplyBackend; const GAS_FOR_VERIFY: NearGas = NearGas::new(20_000_000_000_000); const GAS_FOR_FINISH: NearGas = NearGas::new(50_000_000_000_000); @@ -987,7 +983,7 @@ mod contract { } fn require_owner_only(state: &state::EngineState, predecessor_account_id: &AccountId) { - if &state.owner_id != predecessor_account_id { + if &state.owner_id() != predecessor_account_id { sdk::panic_utf8(errors::ERR_NOT_ALLOWED); } } diff --git a/engine/src/state.rs b/engine/src/state.rs index e17ffe6fa..cef64a020 100644 --- a/engine/src/state.rs +++ b/engine/src/state.rs @@ -10,10 +10,59 @@ pub use error::EngineStateError; /// Key for storing the state of the engine. const STATE_KEY: &[u8; 5] = b"STATE"; +/// Engine internal state, mostly configuration. +#[derive(BorshSerialize, BorshDeserialize, Clone, PartialEq, Eq, Debug)] +pub enum EngineState { + V2(EngineStateV2), + V1(EngineStateV1), +} + +impl EngineState { + pub fn chain_id(&self) -> [u8; 32] { + match self { + EngineState::V2(state) => state.chain_id, + EngineState::V1(state) => state.chain_id, + } + } + + pub fn set_chain_id(&mut self, chain_id: [u8; 32]) { + match self { + EngineState::V2(state) => state.chain_id = chain_id, + EngineState::V1(state) => state.chain_id = chain_id, + } + } + + pub fn owner_id(&self) -> AccountId { + match self { + EngineState::V2(state) => state.owner_id.clone(), + EngineState::V1(state) => state.owner_id.clone(), + } + } + + pub fn upgrade_delay_blocks(&self) -> u64 { + match self { + EngineState::V2(state) => state.upgrade_delay_blocks, + EngineState::V1(state) => state.upgrade_delay_blocks, + } + } + + pub fn deserialize(bytes: &[u8]) -> Option { + Self::try_from_slice(bytes) + .or(EngineStateV1::try_from_slice(bytes).map(Self::V1)) + .ok() + } +} + +impl Default for EngineState { + fn default() -> Self { + Self::V2(EngineStateV2::default()) + } +} + /// Engine internal state V2, mostly configuration. /// Should not contain anything large or enumerable. #[derive(BorshSerialize, BorshDeserialize, Default, Clone, PartialEq, Eq, Debug)] -pub struct EngineState { +pub struct EngineStateV2 { /// Chain id, according to the EIP-155 / ethereum-lists spec. pub chain_id: [u8; 32], /// Account which can upgrade this contract. @@ -25,33 +74,49 @@ pub struct EngineState { pub default_gas_token: GasToken, } +/// Engine internal state V1, mostly configuration. +/// Should not contain anything large or enumerable. +#[derive(BorshSerialize, BorshDeserialize, Default, Clone, PartialEq, Eq, Debug)] +pub struct EngineStateV1 { + /// Chain id, according to the EIP-155 / ethereum-lists spec. + pub chain_id: [u8; 32], + /// Account which can upgrade this contract. + /// Use empty to disable updatability. + pub owner_id: AccountId, + /// Account of the bridge prover. + /// Use empty to not use base token as bridged asset. + pub bridge_prover_id: AccountId, + /// How many blocks after staging upgrade can deploy it. + pub upgrade_delay_blocks: u64, +} + impl From for EngineState { fn from(args: NewCallArgs) -> Self { - EngineState { + EngineState::V2(EngineStateV2 { chain_id: args.chain_id, owner_id: args.owner_id, upgrade_delay_blocks: args.upgrade_delay_blocks, default_gas_token: GasToken::from_array(args.default_gas_token), - } + }) } } /// Gets the state from storage, if it exists otherwise it will error. -pub fn get_state(io: &I) -> Result { +pub fn get_state(io: &I) -> Result { match io.read_storage(&bytes_to_key(KeyPrefix::Config, STATE_KEY)) { - None => Err(error::EngineStateError::NotFound), - Some(bytes) => EngineState::try_from_slice(&bytes.to_vec()) - .map_err(|_| error::EngineStateError::DeserializationFailed), + None => Err(EngineStateError::NotFound), + Some(bytes) => EngineState::deserialize(&bytes.to_vec()) + .ok_or_else(|| EngineStateError::DeserializationFailed), } } /// Saves state into the storage. Does not return the previous state. -pub fn set_state(io: &mut I, state: EngineState) -> Result<(), error::EngineStateError> { +pub fn set_state(io: &mut I, state: EngineState) -> Result<(), EngineStateError> { io.write_storage( &bytes_to_key(KeyPrefix::Config, STATE_KEY), &state .try_to_vec() - .map_err(|_| error::EngineStateError::SerializationFailed)?, + .map_err(|_| EngineStateError::SerializationFailed)?, ); Ok(()) @@ -85,58 +150,6 @@ pub mod error { } } -pub mod legacy { - use super::*; - - /// Migrates the state from the old format to the latest format. - /// - /// While this function works for a single v1 -> v2 migration, it would need to be - /// altered to handle all earlier migrations if needed. - pub fn migrate_state(io: &mut I) -> Result<(), error::EngineStateError> { - // Get the engine legacy state. - let engine_state_legacy = match io.read_storage(&bytes_to_key(KeyPrefix::Config, STATE_KEY)) - { - None => Err(error::EngineStateError::NotFound), - Some(bytes) => EngineStateLegacy::try_from_slice(&bytes.to_vec()) - .map_err(|_| error::EngineStateError::DeserializationFailed), - }?; - - // Migrate to new engine state. - let engine_state_new = EngineState { - chain_id: engine_state_legacy.chain_id, - owner_id: engine_state_legacy.owner_id, - upgrade_delay_blocks: 0, - default_gas_token: Default::default(), - }; - - // Set the new engine state. - io.write_storage( - &bytes_to_key(KeyPrefix::Config, STATE_KEY), - &engine_state_new - .try_to_vec() - .map_err(|_| error::EngineStateError::SerializationFailed)?, - ); - - Ok(()) - } - - /// Engine internal state V1, mostly configuration. - /// Should not contain anything large or enumerable. - #[derive(BorshSerialize, BorshDeserialize, Default, Clone, PartialEq, Eq, Debug)] - pub struct EngineStateLegacy { - /// Chain id, according to the EIP-155 / ethereum-lists spec. - pub chain_id: [u8; 32], - /// Account which can upgrade this contract. - /// Use empty to disable updatability. - pub owner_id: AccountId, - /// Account of the bridge prover. - /// Use empty to not use base token as bridged asset. - pub bridge_prover_id: AccountId, - /// How many blocks after staging upgrade can deploy it. - pub upgrade_delay_blocks: u64, - } -} - #[cfg(test)] #[cfg(feature = "std")] mod tests { @@ -148,12 +161,9 @@ mod tests { fn test_missing_engine_state_is_not_found() { let storage = RefCell::new(Storage::default()); let io = StoragePointer(&storage); - let actual_error = get_state(&io).unwrap_err(); - let actual_error = std::str::from_utf8(actual_error.as_ref()).unwrap(); - let expected_error = std::str::from_utf8(error::ERR_STATE_NOT_FOUND).unwrap(); - assert_eq!(expected_error, actual_error); + assert_eq!(actual_error.as_ref(), error::ERR_STATE_NOT_FOUND); } #[test] @@ -163,9 +173,7 @@ mod tests { io.write_storage(&bytes_to_key(KeyPrefix::Config, STATE_KEY), &[]); let actual_error = get_state(&io).unwrap_err(); - let actual_error = std::str::from_utf8(actual_error.as_ref()).unwrap(); - let expected_error = std::str::from_utf8(error::ERR_STATE_CORRUPTED).unwrap(); - assert_eq!(expected_error, actual_error); + assert_eq!(actual_error.as_ref(), error::ERR_STATE_CORRUPTED); } }