From 6dab2573330354395edd80fbda260a3bab843698 Mon Sep 17 00:00:00 2001 From: Ed Hastings Date: Mon, 29 Apr 2024 15:15:05 -0700 Subject: [PATCH] adding test --- .../components/contract_runtime/operations.rs | 20 +-- node/src/reactor/main_reactor/tests.rs | 6 +- .../main_reactor/tests/transactions.rs | 146 +++++++++++++++++- storage/src/data_access_layer/balance.rs | 4 +- .../src/data_access_layer/handle_refund.rs | 19 ++- storage/src/global_state/state/mod.rs | 102 ++++++++++-- .../handle_payment/handle_payment_native.rs | 11 +- storage/src/system/mint/mint_native.rs | 2 +- storage/src/system/runtime_native.rs | 66 +++++++- storage/src/system/transfer.rs | 5 +- storage/src/tracking_copy/ext_entity.rs | 86 ++++++++--- types/src/system/handle_payment/error.rs | 8 + 12 files changed, 403 insertions(+), 72 deletions(-) diff --git a/node/src/components/contract_runtime/operations.rs b/node/src/components/contract_runtime/operations.rs index d115ae8f92..728ff0618a 100644 --- a/node/src/components/contract_runtime/operations.rs +++ b/node/src/components/contract_runtime/operations.rs @@ -1,7 +1,6 @@ use std::{collections::BTreeMap, convert::TryInto, sync::Arc, time::Instant}; use itertools::Itertools; -use num_rational::Ratio; use tracing::{debug, error, info, trace, warn}; use casper_execution_engine::engine_state::{ExecutionEngineV1, WasmV1Request, WasmV1Result}; @@ -191,13 +190,12 @@ pub fn execute_finalized_block( artifact_builder.with_added_cost(cost); let is_standard_payment = transaction.is_standard_payment(); - let refund_purse_active = !is_standard_payment && !refund_handling.skip_refund(); - // set up the refund purse for this transaction, if custom payment && refunds are on + let refund_purse_active = !is_standard_payment; //&& !refund_handling.skip_refund(); if refund_purse_active { - // if refunds are turned on we initialize the refund purse to the initiator's main - // purse before doing any processing. NOTE: when executed, custom payment logic - // has the option to call set_refund_purse on the handle payment contract to set - // up a different refund purse, if desired. + // if custom payment before doing any processing, initialize the initiator's main purse + // to be the refund purse for this transaction. + // NOTE: when executed, custom payment logic has the option to call set_refund_purse + // on the handle payment contract to set up a different refund purse, if desired. let handle_refund_request = HandleRefundRequest::new( native_runtime_config.clone(), state_root_hash, @@ -445,18 +443,12 @@ pub fn execute_finalized_block( // and then point the balance_identifier to the refund purse // this will result in the downstream no fee handling logic // placing a hold on the correct purse. - let source = Box::new(balance_identifier.clone()); - let target = Box::new(BalanceIdentifier::Refund); balance_identifier = BalanceIdentifier::Refund; - Some(HandleRefundMode::Refund { + Some(HandleRefundMode::CustomHold { initiator_addr: Box::new(initiator_addr.clone()), limit: gas_limit.value(), gas_price: current_gas_price, - consumed: U512::zero(), cost, - ratio: Ratio::new_raw(1, 1), - source, - target, }) } else { None diff --git a/node/src/reactor/main_reactor/tests.rs b/node/src/reactor/main_reactor/tests.rs index d2b8cc0ab7..44203bd165 100644 --- a/node/src/reactor/main_reactor/tests.rs +++ b/node/src/reactor/main_reactor/tests.rs @@ -366,7 +366,7 @@ impl TestFixture { if era_duration != TimeDiff::from_millis(0) { chainspec.core_config.era_duration = era_duration; } - info!("Foooo {block_gas_limit}"); + info!(?block_gas_limit); chainspec.core_config.minimum_block_time = minimum_block_time; chainspec.core_config.minimum_era_height = minimum_era_height; chainspec.core_config.unbonding_delay = unbonding_delay; @@ -415,9 +415,9 @@ impl TestFixture { chainspec.core_config.gas_hold_balance_handling = gas_hold_balance_handling; } - let limit = chainspec.transaction_config.block_gas_limit; + let applied_block_gas_limit = chainspec.transaction_config.block_gas_limit; - info!("THE LIMIT {limit}"); + info!(?applied_block_gas_limit); let mut fixture = TestFixture { rng, diff --git a/node/src/reactor/main_reactor/tests/transactions.rs b/node/src/reactor/main_reactor/tests/transactions.rs index 4cb33a1426..53b11e59b6 100644 --- a/node/src/reactor/main_reactor/tests/transactions.rs +++ b/node/src/reactor/main_reactor/tests/transactions.rs @@ -1,5 +1,5 @@ use super::*; -use casper_storage::data_access_layer::ProofHandling; +use casper_storage::data_access_layer::{BalanceIdentifier, ProofHandling}; use casper_types::GasLimited; use once_cell::sync::Lazy; @@ -156,6 +156,39 @@ fn get_balance( )) } +fn get_payment_purse_balance( + fixture: &mut TestFixture, + block_height: Option, +) -> BalanceResult { + let (_node_id, runner) = fixture.network.nodes().iter().next().unwrap(); + let protocol_version = fixture.chainspec.protocol_version(); + let block_height = block_height.unwrap_or( + runner + .main_reactor() + .storage() + .highest_complete_block_height() + .expect("missing highest completed block"), + ); + let block_header = runner + .main_reactor() + .storage() + .read_block_header_by_height(block_height, true) + .expect("failure to read block header") + .unwrap(); + let state_hash = *block_header.state_root_hash(); + runner + .main_reactor() + .contract_runtime() + .data_access_layer() + .balance(BalanceRequest::new( + state_hash, + protocol_version, + BalanceIdentifier::Payment, + BalanceHandling::Available, + ProofHandling::NoProofs, + )) +} + fn assert_exec_result_cost( exec_result: ExecutionResult, expected_cost: U512, @@ -768,6 +801,7 @@ struct SingleTransactionTestCase { charlie_public_key: PublicKey, } +#[derive(Debug, PartialEq)] struct BalanceAmount { available: U512, total: U512, @@ -981,7 +1015,7 @@ impl SingleTransactionTestCase { .balance(BalanceRequest::new( state_hash, protocol_version, - casper_storage::data_access_layer::BalanceIdentifier::Accumulate, + BalanceIdentifier::Accumulate, balance_handling, ProofHandling::NoProofs, )) @@ -1591,6 +1625,114 @@ async fn only_refunds_are_burnt_no_fee_custom_payment() { ); } +#[tokio::test] +async fn no_refund_no_fee_custom_payment() { + const MAX_GAS_PRICE: u8 = MIN_GAS_PRICE; + + let config = SingleTransactionTestCase::default_test_config() + .with_pricing_handling(PricingHandling::Classic) + .with_refund_handling(RefundHandling::NoRefund) + .with_fee_handling(FeeHandling::NoFee); + + let mut test = SingleTransactionTestCase::new( + ALICE_SECRET_KEY.clone(), + BOB_SECRET_KEY.clone(), + CHARLIE_SECRET_KEY.clone(), + Some(config), + ) + .await; + + // This contract uses custom payment. + let contract_file = RESOURCES_PATH + .join("..") + .join("target") + .join("wasm32-unknown-unknown") + .join("release") + .join("ee_601_regression.wasm"); + let module_bytes = Bytes::from(std::fs::read(contract_file).expect("cannot read module bytes")); + + let expected_transaction_gas = 1000u64; + let expected_transaction_cost = expected_transaction_gas * MIN_GAS_PRICE as u64; + + let mut txn = Transaction::from( + TransactionV1Builder::new_session(TransactionSessionKind::Standard, module_bytes, "call") + .with_chain_name(CHAIN_NAME) + .with_pricing_mode(PricingMode::Classic { + payment_amount: expected_transaction_gas, + gas_price_tolerance: MIN_GAS_PRICE, + standard_payment: false, + }) + .with_initiator_addr(BOB_PUBLIC_KEY.clone()) + .build() + .unwrap(), + ); + txn.sign(&BOB_SECRET_KEY); + + test.fixture + .run_until_consensus_in_era(ERA_ONE, ONE_MIN) + .await; + + let (alice_initial_balance, bob_initial_balance, _charlie_initial_balance) = + test.get_balances(None); + let initial_total_supply = test.get_total_supply(None); + let (_txn_hash, block_height, exec_result) = test.send_transaction(txn).await; + // expected to fail due to insufficient funding + assert!(!exec_result_is_success(&exec_result), "should have failed"); + match exec_result { + ExecutionResult::V2(exec_result_v2) => { + assert_eq!(exec_result_v2.cost, expected_transaction_cost.into()); + } + _ => { + panic!("Unexpected exec result version.") + } + } + + let payment_purse_balance = get_payment_purse_balance(&mut test.fixture, Some(block_height)); + assert_eq!( + *payment_purse_balance + .total_balance() + .expect("should have total balance"), + U512::zero(), + "payment purse should have a 0 balance" + ); + + // we're not burning anything, so total supply should be the same + assert_eq!( + test.get_total_supply(Some(block_height)), + initial_total_supply, + "total supply should be the same before and after" + ); + + // updated balances + let (alice_current_balance, bob_current_balance, _) = test.get_balances(Some(block_height)); + + // the proposer's balance should be the same because we are in no fee mode + assert_eq!( + alice_initial_balance, alice_current_balance, + "the proposers balance should be unchanged as we are in no fee mode" + ); + + // the initiator should have a hold equal to the cost + assert_eq!( + bob_current_balance.total.clone(), + bob_initial_balance.total, + "bob's total balance should be unchanged as we are in no fee mode" + ); + + assert_ne!( + bob_current_balance.available.clone(), + bob_initial_balance.total, + "bob's available balance and total balance should not be the same" + ); + + let bob_expected_available_balance = bob_initial_balance.total - expected_transaction_cost; + assert_eq!( + bob_current_balance.available.clone(), + bob_expected_available_balance, + "bob's available balance should reflect a hold for the cost" + ); +} + async fn transfer_fee_is_burnt_no_refund(txn_pricing_mode: PricingMode) { let (price_handling, min_gas_price, gas_limit) = match_pricing_mode(&txn_pricing_mode); diff --git a/storage/src/data_access_layer/balance.rs b/storage/src/data_access_layer/balance.rs index df64f0df06..63c5041260 100644 --- a/storage/src/data_access_layer/balance.rs +++ b/storage/src/data_access_layer/balance.rs @@ -89,14 +89,14 @@ impl BalanceIdentifier { BalanceIdentifier::Public(public_key) => { let account_hash = public_key.to_account_hash(); match tc.get_addressable_entity_by_account_hash(protocol_version, account_hash) { - Ok(entity) => entity.main_purse(), + Ok((_, entity)) => entity.main_purse(), Err(tce) => return Err(tce), } } BalanceIdentifier::Account(account_hash) | BalanceIdentifier::PenalizedAccount(account_hash) => { match tc.get_addressable_entity_by_account_hash(protocol_version, *account_hash) { - Ok(entity) => entity.main_purse(), + Ok((_, entity)) => entity.main_purse(), Err(tce) => return Err(tce), } } diff --git a/storage/src/data_access_layer/handle_refund.rs b/storage/src/data_access_layer/handle_refund.rs index e9bc739bae..ab7fb36038 100644 --- a/storage/src/data_access_layer/handle_refund.rs +++ b/storage/src/data_access_layer/handle_refund.rs @@ -27,6 +27,12 @@ pub enum HandleRefundMode { source: Box, target: Box, }, + CustomHold { + initiator_addr: Box, + limit: U512, + cost: U512, + gas_price: u8, + }, RefundAmount { limit: U512, cost: U512, @@ -48,6 +54,7 @@ impl HandleRefundMode { HandleRefundMode::ClearRefundPurse | HandleRefundMode::Burn { .. } | HandleRefundMode::Refund { .. } + | HandleRefundMode::CustomHold { .. } | HandleRefundMode::RefundAmount { .. } => Phase::FinalizePayment, HandleRefundMode::SetRefundPurse { .. } => Phase::Payment, } @@ -107,6 +114,7 @@ impl HandleRefundRequest { } } +#[derive(Debug)] pub enum HandleRefundResult { /// Invalid state root hash. RootNotFound, @@ -115,6 +123,8 @@ pub enum HandleRefundResult { effects: Effects, amount: Option, }, + /// Invalid phase selected (programmer error). + InvalidPhase, /// Handle refund request failed. Failure(TrackingCopyError), } @@ -123,7 +133,9 @@ impl HandleRefundResult { /// The effects, if any. pub fn effects(&self) -> Effects { match self { - HandleRefundResult::RootNotFound | HandleRefundResult::Failure(_) => Effects::new(), + HandleRefundResult::RootNotFound + | HandleRefundResult::InvalidPhase + | HandleRefundResult::Failure(_) => Effects::new(), HandleRefundResult::Success { effects, .. } => effects.clone(), } } @@ -131,7 +143,9 @@ impl HandleRefundResult { /// The refund amount. pub fn refund_amount(&self) -> U512 { match self { - HandleRefundResult::RootNotFound | HandleRefundResult::Failure(_) => U512::zero(), + HandleRefundResult::RootNotFound + | HandleRefundResult::InvalidPhase + | HandleRefundResult::Failure(_) => U512::zero(), HandleRefundResult::Success { amount: refund_amount, .. @@ -143,6 +157,7 @@ impl HandleRefundResult { pub fn error_message(&self) -> Option { match self { HandleRefundResult::RootNotFound => Some("root not found".to_string()), + HandleRefundResult::InvalidPhase => Some("invalid phase selected".to_string()), HandleRefundResult::Failure(tce) => Some(format!("{}", tce)), HandleRefundResult::Success { .. } => None, } diff --git a/storage/src/global_state/state/mod.rs b/storage/src/global_state/state/mod.rs index 9595cc6d66..9126782e1e 100644 --- a/storage/src/global_state/state/mod.rs +++ b/storage/src/global_state/state/mod.rs @@ -29,7 +29,7 @@ use casper_types::{ BalanceHoldAddr, BalanceHoldAddrTag, ARG_AMOUNT, ROUND_SEIGNIORAGE_RATE_KEY, TOTAL_SUPPLY_KEY, }, - AUCTION, MINT, + AUCTION, HANDLE_PAYMENT, MINT, }, Account, AddressableEntity, BlockGlobalAddr, CLValue, Digest, EntityAddr, HoldsEpoch, Key, KeyTag, Phase, PublicKey, RuntimeArgs, StoredValue, U512, @@ -1008,7 +1008,7 @@ pub trait StateProvider { }; let source_account_hash = initiator.account_hash(); - let (entity, entity_named_keys, entity_access_rights) = + let (entity_addr, entity, entity_named_keys, entity_access_rights) = match tc.borrow_mut().resolved_entity( protocol_version, source_account_hash, @@ -1020,6 +1020,7 @@ pub trait StateProvider { return BiddingResult::Failure(tce); } }; + let entity_key = Key::AddressableEntity(entity_addr); // IMPORTANT: this runtime _must_ use the payer's context. let mut runtime = RuntimeNative::new( @@ -1028,6 +1029,7 @@ pub trait StateProvider { Id::Transaction(transaction_hash), Rc::clone(&tc), source_account_hash, + entity_key, entity, entity_named_keys, entity_access_rights, @@ -1127,18 +1129,40 @@ pub trait StateProvider { Err(err) => return HandleRefundResult::Failure(TrackingCopyError::Storage(err)), }; - // this runtime uses the system's context - let mut runtime = match RuntimeNative::new_system_runtime( - config, - protocol_version, - Id::Transaction(transaction_hash), - Rc::clone(&tc), - refund_mode.phase(), - ) { - Ok(rt) => rt, - Err(tce) => { - return HandleRefundResult::Failure(tce); + let phase = refund_mode.phase(); + let mut runtime = match phase { + Phase::FinalizePayment => { + // this runtime uses the system's context + match RuntimeNative::new_system_runtime( + config, + protocol_version, + Id::Transaction(transaction_hash), + Rc::clone(&tc), + phase, + ) { + Ok(rt) => rt, + Err(tce) => { + return HandleRefundResult::Failure(tce); + } + } + } + Phase::Payment => { + // this runtime uses the handle payment contract's context + match RuntimeNative::new_system_contract_runtime( + config, + protocol_version, + Id::Transaction(transaction_hash), + Rc::clone(&tc), + phase, + HANDLE_PAYMENT, + ) { + Ok(rt) => rt, + Err(tce) => { + return HandleRefundResult::Failure(tce); + } + } } + Phase::System | Phase::Session => return HandleRefundResult::InvalidPhase, }; let result = match refund_mode { @@ -1223,6 +1247,53 @@ pub trait StateProvider { Err(err) => Err(err), } } + HandleRefundMode::CustomHold { + initiator_addr, + limit, + cost, + gas_price, + } => { + let source = BalanceIdentifier::Payment; + let source_purse = match source.purse_uref(&mut tc.borrow_mut(), protocol_version) { + Ok(value) => value, + Err(tce) => return HandleRefundResult::Failure(tce), + }; + let consumed = U512::zero(); + let ratio = Ratio::new_raw(U512::one(), U512::one()); + let refund_amount = match runtime.calculate_overpayment_and_fee( + limit, + gas_price, + cost, + consumed, + source_purse, + ratio, + ) { + Ok((refund, _)) => refund, + Err(hpe) => { + return HandleRefundResult::Failure(TrackingCopyError::SystemContract( + system::Error::HandlePayment(hpe), + )); + } + }; + let target = BalanceIdentifier::Refund; + let target_purse = match target.purse_uref(&mut tc.borrow_mut(), protocol_version) { + Ok(value) => value, + Err(tce) => return HandleRefundResult::Failure(tce), + }; + match runtime + .transfer( + Some(initiator_addr.account_hash()), + source_purse, + target_purse, + refund_amount, + None, + ) + .map_err(|_| Error::Transfer) + { + Ok(_) => Ok(Some(U512::zero())), // return 0 in this mode + Err(err) => Err(err), + } + } HandleRefundMode::Burn { limit, gas_price, @@ -1720,7 +1791,7 @@ pub trait StateProvider { } } - let (entity, entity_named_keys, entity_access_rights) = + let (entity_addr, entity, entity_named_keys, entity_access_rights) = match tc.borrow_mut().resolved_entity( protocol_version, source_account_hash, @@ -1732,7 +1803,7 @@ pub trait StateProvider { return TransferResult::Failure(TransferError::TrackingCopy(tce)); } }; - + let entity_key = Key::AddressableEntity(entity_addr); let id = Id::Transaction(request.transaction_hash()); // IMPORTANT: this runtime _must_ use the payer's context. let mut runtime = RuntimeNative::new( @@ -1741,6 +1812,7 @@ pub trait StateProvider { id, Rc::clone(&tc), source_account_hash, + entity_key, entity.clone(), entity_named_keys.clone(), entity_access_rights, diff --git a/storage/src/system/handle_payment/handle_payment_native.rs b/storage/src/system/handle_payment/handle_payment_native.rs index 055a91f8f7..26c516faee 100644 --- a/storage/src/system/handle_payment/handle_payment_native.rs +++ b/storage/src/system/handle_payment/handle_payment_native.rs @@ -142,19 +142,20 @@ where fn put_key(&mut self, name: &str, key: Key) -> Result<(), Error> { let name = name.to_string(); - let entity = self.addressable_entity(); - let addressable_entity_hash = AddressableEntityHash::new(self.address().value()); - let entity_addr = entity.entity_addr(addressable_entity_hash); + let entity_addr = self + .entity_key() + .as_entity_addr() + .ok_or(Error::UnexpectedKeyVariant)?; let named_key_value = StoredValue::NamedKey( NamedKeyValue::from_concrete_values(key, name.clone()).map_err(|_| Error::PutKey)?, ); let named_key_addr = NamedKeyAddr::new_from_string(entity_addr, name.clone()).map_err(|_| Error::PutKey)?; - let key = Key::NamedKey(named_key_addr); + let named_key = Key::NamedKey(named_key_addr); // write to both tracking copy and in-mem named keys cache self.tracking_copy() .borrow_mut() - .write(key, named_key_value); + .write(named_key, named_key_value); self.named_keys_mut().insert(name, key); Ok(()) } diff --git a/storage/src/system/mint/mint_native.rs b/storage/src/system/mint/mint_native.rs index 4d394fb086..8b308084ff 100644 --- a/storage/src/system/mint/mint_native.rs +++ b/storage/src/system/mint/mint_native.rs @@ -58,7 +58,7 @@ where .borrow_mut() .get_addressable_entity_by_account_hash(self.protocol_version(), account_hash) { - Ok(entity) => Ok(Some(entity)), + Ok((_, entity)) => Ok(Some(entity)), Err(tce) => { error!(%tce, "error reading addressable entity by account hash"); Err(ProviderError::AddressableEntityByAccountHash(account_hash)) diff --git a/storage/src/system/runtime_native.rs b/storage/src/system/runtime_native.rs index 87e0733b9c..a29f320f43 100644 --- a/storage/src/system/runtime_native.rs +++ b/storage/src/system/runtime_native.rs @@ -1,6 +1,6 @@ use crate::{ global_state::{error::Error as GlobalStateReader, state::StateReader}, - tracking_copy::{TrackingCopyEntityExt, TrackingCopyError}, + tracking_copy::{TrackingCopyEntityExt, TrackingCopyError, TrackingCopyExt}, AddressGenerator, TrackingCopy, }; use casper_types::{ @@ -9,6 +9,7 @@ use casper_types::{ StoredValue, TransactionHash, Transfer, URef, U512, }; use std::{cell::RefCell, collections::BTreeSet, rc::Rc}; +use tracing::error; #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct Config { @@ -235,6 +236,7 @@ pub struct RuntimeNative { tracking_copy: Rc>>, address: AccountHash, + entity_key: Key, addressable_entity: AddressableEntity, named_keys: NamedKeys, access_rights: ContextAccessRights, @@ -254,6 +256,7 @@ where id: Id, tracking_copy: Rc>>, address: AccountHash, + entity_key: Key, addressable_entity: AddressableEntity, named_keys: NamedKeys, access_rights: ContextAccessRights, @@ -271,6 +274,7 @@ where tracking_copy, address, + entity_key, addressable_entity, named_keys, access_rights, @@ -281,7 +285,6 @@ where } /// Creates a runtime with elevated permissions for systemic behaviors. - #[allow(clippy::too_many_arguments)] pub fn new_system_runtime( config: Config, protocol_version: ProtocolVersion, @@ -292,8 +295,60 @@ where let seed = id.seed(); let address_generator = AddressGenerator::new(&seed, phase); let transfers = vec![]; - let (addressable_entity, named_keys, access_rights) = + let (entity_addr, addressable_entity, named_keys, access_rights) = tracking_copy.borrow_mut().system_entity(protocol_version)?; + let address = PublicKey::System.to_account_hash(); + let entity_key = Key::AddressableEntity(entity_addr); + let remaining_spending_limit = U512::MAX; // system has no spending limit + Ok(RuntimeNative { + config, + id, + address_generator, + protocol_version, + + tracking_copy, + address, + entity_key, + addressable_entity, + named_keys, + access_rights, + remaining_spending_limit, + transfers, + phase, + }) + } + + /// Creates a runtime context for a system contract. + pub fn new_system_contract_runtime( + config: Config, + protocol_version: ProtocolVersion, + id: Id, + tracking_copy: Rc>>, + phase: Phase, + name: &str, + ) -> Result { + let seed = id.seed(); + let address_generator = AddressGenerator::new(&seed, phase); + let transfers = vec![]; + + let system_entity_registry = tracking_copy.borrow().get_system_entity_registry()?; + let hash = match system_entity_registry.get(name).copied() { + Some(hash) => hash, + None => { + error!("unexpected failure; system contract {} not found", name); + return Err(TrackingCopyError::MissingSystemContractHash( + name.to_string(), + )); + } + }; + let addressable_entity = tracking_copy + .borrow_mut() + .get_addressable_entity_by_hash(hash)?; + let entity_addr = addressable_entity.entity_addr(hash); + let entity_key = Key::AddressableEntity(entity_addr); + let named_keys = tracking_copy.borrow().get_named_keys(entity_addr)?; + let access_rights = addressable_entity.extract_access_rights(hash, &named_keys); + let address = PublicKey::System.to_account_hash(); let remaining_spending_limit = U512::MAX; // system has no spending limit Ok(RuntimeNative { @@ -304,6 +359,7 @@ where tracking_copy, address, + entity_key, addressable_entity, named_keys, access_rights, @@ -337,6 +393,10 @@ where self.address } + pub fn entity_key(&self) -> &Key { + &self.entity_key + } + pub fn addressable_entity(&self) -> &AddressableEntity { &self.addressable_entity } diff --git a/storage/src/system/transfer.rs b/storage/src/system/transfer.rs index 1fd9e5f30f..b6edd7a480 100644 --- a/storage/src/system/transfer.rs +++ b/storage/src/system/transfer.rs @@ -363,9 +363,8 @@ impl TransferRuntimeArgsBuilder { .borrow_mut() .get_addressable_entity_by_account_hash(protocol_version, account_hash) { - Ok(contract) => { - let main_purse_addable = - contract.main_purse().with_access_rights(AccessRights::ADD); + Ok((_, entity)) => { + let main_purse_addable = entity.main_purse().with_access_rights(AccessRights::ADD); Ok(NewTransferTargetMode::ExistingAccount { target_account_hash: account_hash, main_purse: main_purse_addable, diff --git a/storage/src/tracking_copy/ext_entity.rs b/storage/src/tracking_copy/ext_entity.rs index a78229d382..ffe9fc8769 100644 --- a/storage/src/tracking_copy/ext_entity.rs +++ b/storage/src/tracking_copy/ext_entity.rs @@ -58,7 +58,7 @@ pub trait TrackingCopyEntityExt { &mut self, protocol_version: ProtocolVersion, account_hash: AccountHash, - ) -> Result; + ) -> Result<(EntityAddr, AddressableEntity), Self::Error>; /// Get entity if authorized, else error. fn get_authorized_addressable_entity( @@ -109,7 +109,15 @@ pub trait TrackingCopyEntityExt { fn system_entity( &mut self, protocol_version: ProtocolVersion, - ) -> Result<(AddressableEntity, NamedKeys, ContextAccessRights), TrackingCopyError>; + ) -> Result< + ( + EntityAddr, + AddressableEntity, + NamedKeys, + ContextAccessRights, + ), + TrackingCopyError, + >; /// Returns entity, named keys, and access rights. fn resolved_entity( @@ -118,7 +126,15 @@ pub trait TrackingCopyEntityExt { initiating_address: AccountHash, authorization_keys: &BTreeSet, administrative_accounts: &BTreeSet, - ) -> Result<(AddressableEntity, NamedKeys, ContextAccessRights), TrackingCopyError>; + ) -> Result< + ( + EntityAddr, + AddressableEntity, + NamedKeys, + ContextAccessRights, + ), + TrackingCopyError, + >; /// Returns fee purse. fn fees_purse( @@ -192,13 +208,10 @@ where &mut self, protocol_version: ProtocolVersion, account_hash: AccountHash, - ) -> Result { + ) -> Result<(EntityAddr, AddressableEntity), Self::Error> { let account_key = Key::Account(account_hash); - let contract_key = match self.get(&account_key)? { - Some(StoredValue::CLValue(contract_key_as_cl_value)) => { - CLValue::into_t::(contract_key_as_cl_value)? - } + let entity_addr = match self.get(&account_key)? { Some(StoredValue::Account(account)) => { // do a legacy account migration let mut generator = @@ -241,7 +254,8 @@ where package }; - let entity_key = entity.entity_key(entity_hash); + let entity_addr = entity.entity_addr(entity_hash); + let entity_key = Key::AddressableEntity(entity_addr); self.write(entity_key, StoredValue::AddressableEntity(entity.clone())); self.write(package_hash.into(), package.into()); @@ -253,9 +267,14 @@ where self.write(account_key, StoredValue::CLValue(contract_by_account)); - return Ok(entity); + return Ok((entity_addr, entity)); } + Some(StoredValue::CLValue(contract_key_as_cl_value)) => { + let key = CLValue::into_t::(contract_key_as_cl_value)?; + key.as_entity_addr() + .ok_or(Self::Error::UnexpectedKeyVariant(key))? + } Some(other) => { return Err(TrackingCopyError::TypeMismatch( StoredValueTypeMismatch::new("Key".to_string(), other.type_name()), @@ -264,12 +283,14 @@ where None => return Err(TrackingCopyError::KeyNotFound(account_key)), }; - match self.get(&contract_key)? { - Some(StoredValue::AddressableEntity(contract)) => Ok(contract), + match self.get(&Key::AddressableEntity(entity_addr))? { + Some(StoredValue::AddressableEntity(contract)) => Ok((entity_addr, contract)), Some(other) => Err(TrackingCopyError::TypeMismatch( StoredValueTypeMismatch::new("Contract".to_string(), other.type_name()), )), - None => Err(TrackingCopyError::KeyNotFound(contract_key)), + None => Err(TrackingCopyError::KeyNotFound(Key::AddressableEntity( + entity_addr, + ))), } } @@ -280,7 +301,7 @@ where authorization_keys: &BTreeSet, administrative_accounts: &BTreeSet, ) -> Result<(AddressableEntity, AddressableEntityHash), Self::Error> { - let entity_record = + let (_, entity_record) = self.get_addressable_entity_by_account_hash(protocol_version, account_hash)?; let entity_hash = self.get_entity_hash_by_account_hash(account_hash)?; @@ -534,9 +555,17 @@ where fn system_entity( &mut self, protocol_version: ProtocolVersion, - ) -> Result<(AddressableEntity, NamedKeys, ContextAccessRights), TrackingCopyError> { + ) -> Result< + ( + EntityAddr, + AddressableEntity, + NamedKeys, + ContextAccessRights, + ), + TrackingCopyError, + > { let system_account_hash = PublicKey::System.to_account_hash(); - let system_entity = + let (system_entity_addr, system_entity) = self.get_addressable_entity_by_account_hash(protocol_version, system_account_hash)?; let system_entity_registry = self.get_system_entity_registry()?; @@ -603,7 +632,12 @@ where auction_access_rights.extend_access_rights(mint_access_rights.take_access_rights()); auction_access_rights.extend_access_rights(payment_access_rights.take_access_rights()); - Ok((system_entity, named_keys, auction_access_rights)) + Ok(( + system_entity_addr, + system_entity, + named_keys, + auction_access_rights, + )) } fn resolved_entity( @@ -612,7 +646,15 @@ where initiating_address: AccountHash, authorization_keys: &BTreeSet, administrative_accounts: &BTreeSet, - ) -> Result<(AddressableEntity, NamedKeys, ContextAccessRights), TrackingCopyError> { + ) -> Result< + ( + EntityAddr, + AddressableEntity, + NamedKeys, + ContextAccessRights, + ), + TrackingCopyError, + > { if initiating_address == PublicKey::System.to_account_hash() { return self.system_entity(protocol_version); } @@ -627,7 +669,7 @@ where let named_keys = self.get_named_keys(entity_addr)?; let access_rights = entity .extract_access_rights(AddressableEntityHash::new(entity_addr.value()), &named_keys); - Ok((entity, named_keys, access_rights)) + Ok((entity_addr, entity, named_keys, access_rights)) } fn fees_purse( @@ -640,10 +682,10 @@ where match fee_handling { FeesPurseHandling::None(uref) => Ok(uref), FeesPurseHandling::ToProposer(proposer) => { - let proposer_account: AddressableEntity = + let (_, entity) = self.get_addressable_entity_by_account_hash(protocol_version, proposer)?; - Ok(proposer_account.main_purse()) + Ok(entity.main_purse()) } FeesPurseHandling::Accumulate => { let registry = self.get_system_entity_registry()?; @@ -678,7 +720,7 @@ where } FeesPurseHandling::Burn => { // TODO: replace this with new burn logic once it merges - Ok(casper_types::URef::default()) + Ok(URef::default()) } } } diff --git a/types/src/system/handle_payment/error.rs b/types/src/system/handle_payment/error.rs index 449d5694a7..7eb8ea5910 100644 --- a/types/src/system/handle_payment/error.rs +++ b/types/src/system/handle_payment/error.rs @@ -261,6 +261,12 @@ pub enum Error { /// assert_eq!(38, Error::IncompatiblePaymentSettings as u8); /// ``` IncompatiblePaymentSettings = 38, + /// Unexpected key variant. + /// ``` + /// # use casper_types::system::handle_payment::Error; + /// assert_eq!(39, Error::UnexpectedKeyVariant as u8); + /// ``` + UnexpectedKeyVariant = 39, } impl Display for Error { @@ -335,6 +341,7 @@ impl Display for Error { Error::IncompatiblePaymentSettings => { formatter.write_str("Incompatible payment settings") } + Error::UnexpectedKeyVariant => formatter.write_str("Unexpected key variant"), } } } @@ -412,6 +419,7 @@ impl TryFrom for Error { v if v == Error::IncompatiblePaymentSettings as u8 => { Error::IncompatiblePaymentSettings } + v if v == Error::UnexpectedKeyVariant as u8 => Error::UnexpectedKeyVariant, _ => return Err(()), }; Ok(error)