From f16948e28224c2762aaa2c5b6a9b4404fa35a533 Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Wed, 30 Oct 2019 15:34:14 -0700 Subject: [PATCH 1/7] Introduce gas_limit to runtime and start delaying blocks --- chain/chain/src/chain.rs | 5 + chain/chain/src/test_utils.rs | 9 +- chain/chain/src/types.rs | 6 ++ chain/chain/src/validate.rs | 3 +- chain/client/src/client.rs | 19 ++-- chain/client/tests/challenges.rs | 4 + core/primitives/src/utils.rs | 8 ++ near/res/testnet.json | 1 + near/src/runtime.rs | 17 ++- .../runtime-params-estimator/src/testbed.rs | 1 + runtime/runtime/src/lib.rs | 100 +++++++++++++++--- runtime/runtime/tests/runtime_group_tools.rs | 1 + test-utils/testlib/src/user/runtime_user.rs | 1 + 13 files changed, 149 insertions(+), 26 deletions(-) diff --git a/chain/chain/src/chain.rs b/chain/chain/src/chain.rs index 9bc9bc19097..63f02b911e0 100644 --- a/chain/chain/src/chain.rs +++ b/chain/chain/src/chain.rs @@ -1803,6 +1803,7 @@ impl<'a> ChainUpdate<'a> { &prev_chunk.transactions, &prev_chunk.header.inner.validator_proposals, prev_block.header.inner.gas_price, + prev_chunk.header.inner.gas_limit, &challenges_result, true, ) @@ -1934,6 +1935,7 @@ impl<'a> ChainUpdate<'a> { &chunk.transactions, &chunk.header.inner.validator_proposals, block.header.inner.gas_price, + chunk.header.inner.gas_limit, &block.header.inner.challenges_result, ) .map_err(|e| ErrorKind::Other(e.to_string()))?; @@ -1990,6 +1992,7 @@ impl<'a> ChainUpdate<'a> { &vec![], &new_extra.validator_proposals, block.header.inner.gas_price, + new_extra.gas_limit, &block.header.inner.challenges_result, ) .map_err(|e| ErrorKind::Other(e.to_string()))?; @@ -2471,6 +2474,7 @@ impl<'a> ChainUpdate<'a> { &chunk.transactions, &chunk.header.inner.validator_proposals, block_header.inner.gas_price, + chunk.header.inner.gas_limit, &block_header.inner.challenges_result, )?; @@ -2545,6 +2549,7 @@ impl<'a> ChainUpdate<'a> { &vec![], &chunk_extra.validator_proposals, block_header.inner.gas_price, + chunk_extra.gas_limit, &block_header.inner.challenges_result, )?; diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index e4d6c4e90df..9c748dc0d06 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -21,7 +21,8 @@ use near_primitives::transaction::{ TransferAction, }; use near_primitives::types::{ - AccountId, Balance, BlockIndex, EpochId, MerkleHash, Nonce, ShardId, StateRoot, ValidatorStake, + AccountId, Balance, BlockIndex, EpochId, Gas, MerkleHash, Nonce, ShardId, StateRoot, + ValidatorStake, }; use near_primitives::views::QueryResponse; use near_store::test_utils::create_test_store; @@ -437,7 +438,8 @@ impl RuntimeAdapter for KeyValueRuntime { &self, _block_index: u64, _block_timestamp: u64, - _gas_price: u128, + _gas_price: Balance, + _gas_limit: Gas, _state_root: StateRoot, transactions: Vec, ) -> Vec { @@ -449,6 +451,7 @@ impl RuntimeAdapter for KeyValueRuntime { _block_index: BlockIndex, _block_timestamp: u64, _gas_price: Balance, + _gas_limit: Gas, _state_root: StateRoot, transaction: SignedTransaction, ) -> Result { @@ -482,6 +485,7 @@ impl RuntimeAdapter for KeyValueRuntime { transactions: &[SignedTransaction], _last_validator_proposals: &[ValidatorStake], gas_price: Balance, + _gas_limit: Gas, _challenges: &ChallengesResult, generate_storage_proof: bool, ) -> Result { @@ -673,6 +677,7 @@ impl RuntimeAdapter for KeyValueRuntime { _transactions: &[SignedTransaction], _last_validator_proposals: &[ValidatorStake], _gas_price: Balance, + _gas_limit: Gas, _challenges: &ChallengesResult, ) -> Result { unimplemented!(); diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs index f794d8dd418..cebcd82f8ff 100644 --- a/chain/chain/src/types.rs +++ b/chain/chain/src/types.rs @@ -139,6 +139,7 @@ pub trait RuntimeAdapter: Send + Sync { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, + gas_limit: Gas, state_root: StateRoot, transaction: SignedTransaction, ) -> Result; @@ -150,6 +151,7 @@ pub trait RuntimeAdapter: Send + Sync { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, + gas_limit: Gas, state_root: StateRoot, transactions: Vec, ) -> Vec; @@ -290,6 +292,7 @@ pub trait RuntimeAdapter: Send + Sync { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges_result: &ChallengesResult, ) -> Result { self.apply_transactions_with_optional_storage_proof( @@ -303,6 +306,7 @@ pub trait RuntimeAdapter: Send + Sync { transactions, last_validator_proposals, gas_price, + gas_limit, challenges_result, false, ) @@ -320,6 +324,7 @@ pub trait RuntimeAdapter: Send + Sync { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges_result: &ChallengesResult, generate_storage_proof: bool, ) -> Result; @@ -337,6 +342,7 @@ pub trait RuntimeAdapter: Send + Sync { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges_result: &ChallengesResult, ) -> Result; diff --git a/chain/chain/src/validate.rs b/chain/chain/src/validate.rs index e7a97d26f97..cb9d3224ff3 100644 --- a/chain/chain/src/validate.rs +++ b/chain/chain/src/validate.rs @@ -224,7 +224,8 @@ fn validate_chunk_state_challenge( &chunk_state.prev_chunk.receipts, &chunk_state.prev_chunk.transactions, &[], - 0, + prev_block_header.inner.gas_price, + chunk_state.prev_chunk.header.inner.gas_limit, &ChallengesResult::default(), ) .map_err(|_| Error::from(ErrorKind::MaliciousChallenge))?; diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 89cfad54691..945eb71dcbb 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -382,6 +382,7 @@ impl Client { next_height, prev_block_timestamp, block_header.inner.gas_price, + chunk_extra.gas_limit, chunk_extra.state_root.clone(), transactions, ); @@ -958,18 +959,20 @@ impl Client { ) .inner .gas_price; - let state_root = match self.chain.get_chunk_extra(&head.last_block_hash, shard_id) { - Ok(chunk_extra) => chunk_extra.state_root.clone(), - Err(_) => { - // Not being able to fetch a state root most likely implies that we haven't - // caught up with the next epoch yet. - return self.forward_tx(tx); - } - }; + let (state_root, gas_limit) = + match self.chain.get_chunk_extra(&head.last_block_hash, shard_id) { + Ok(chunk_extra) => (chunk_extra.state_root.clone(), chunk_extra.gas_limit), + Err(_) => { + // Not being able to fetch a state root most likely implies that we haven't + // caught up with the next epoch yet. + return self.forward_tx(tx); + } + }; match self.runtime_adapter.validate_tx( head.height + 1, cur_block_header.inner.timestamp, gas_price, + gas_limit, state_root, tx, ) { diff --git a/chain/client/tests/challenges.rs b/chain/client/tests/challenges.rs index 7368abe286d..e271ac00800 100644 --- a/chain/client/tests/challenges.rs +++ b/chain/client/tests/challenges.rs @@ -176,7 +176,11 @@ fn test_verify_chunk_invalid_proofs_challenge() { ); } +// TODO(#1604): Fix partial state construction in the test. We shouldn't hardcode the partial state, +// because it breaks the development of Runtime that affects state. +// Instead we should reconstruct the partial state as part of the test. #[test] +#[ignore] fn test_verify_chunk_invalid_state_challenge() { let store1 = create_test_store(); let genesis_config = GenesisConfig::test(vec!["test0", "test1"], 1); diff --git a/core/primitives/src/utils.rs b/core/primitives/src/utils.rs index 4ac9f7b589d..baad61cfca1 100644 --- a/core/primitives/src/utils.rs +++ b/core/primitives/src/utils.rs @@ -28,6 +28,8 @@ pub mod col { pub const POSTPONED_RECEIPT_ID: &[u8] = &[4]; pub const PENDING_DATA_COUNT: &[u8] = &[5]; pub const POSTPONED_RECEIPT: &[u8] = &[6]; + pub const DELAYED_RECEIPT_INDICES: &[u8] = &[7]; + pub const DELAYED_RECEIPT: &[u8] = &[8]; } fn key_for_column_account_id(column: &[u8], account_key: &AccountId) -> Vec { @@ -98,6 +100,12 @@ pub fn key_for_postponed_receipt(account_id: &AccountId, receipt_id: &CryptoHash key } +pub fn key_for_delayed_receipt(index: u64) -> Vec { + let mut key = col::DELAYED_RECEIPT.to_vec(); + key.extend_from_slice(&index.to_le_bytes()); + key +} + pub fn create_nonce_with_nonce(base: &CryptoHash, salt: u64) -> CryptoHash { let mut nonce: Vec = base.as_ref().to_owned(); nonce.append(&mut index_to_bytes(salt)); diff --git a/near/res/testnet.json b/near/res/testnet.json index e4adde8602d..baa8ebeb82d 100644 --- a/near/res/testnet.json +++ b/near/res/testnet.json @@ -104579,6 +104579,7 @@ "validator_kickout_threshold": 80, "gas_price_adjustment_rate": 1, "runtime_config": { + "max_chunk_gas_burnt": "account_length_baseline_cost_per_block": "6561", "transaction_costs": { "storage_usage_config": { diff --git a/near/src/runtime.rs b/near/src/runtime.rs index 8a9069e8de4..83397371fda 100644 --- a/near/src/runtime.rs +++ b/near/src/runtime.rs @@ -23,7 +23,7 @@ use near_primitives::serialize::from_base64; use near_primitives::sharding::ShardChunkHeader; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{ - AccountId, Balance, BlockIndex, EpochId, MerkleHash, ShardId, StateRoot, ValidatorStake, + AccountId, Balance, BlockIndex, EpochId, Gas, MerkleHash, ShardId, StateRoot, ValidatorStake, }; use near_primitives::utils::{prefix_for_access_key, ACCOUNT_DATA_SEPARATOR}; use near_primitives::views::{ @@ -206,6 +206,7 @@ impl NightshadeRuntime { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges_result: &ChallengesResult, ) -> Result { let validator_accounts_update = { @@ -273,6 +274,7 @@ impl NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, + gas_limit, }; let apply_result = self @@ -599,6 +601,7 @@ impl RuntimeAdapter for NightshadeRuntime { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, + gas_limit: Gas, state_root: StateRoot, transaction: SignedTransaction, ) -> Result { @@ -608,6 +611,7 @@ impl RuntimeAdapter for NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, + gas_limit, }; if let Err(err) = self.runtime.verify_and_charge_transaction( @@ -626,6 +630,7 @@ impl RuntimeAdapter for NightshadeRuntime { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, + gas_limit: Gas, state_root: StateRoot, transactions: Vec, ) -> Vec { @@ -635,6 +640,7 @@ impl RuntimeAdapter for NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, + gas_limit, }; transactions .into_iter() @@ -698,6 +704,7 @@ impl RuntimeAdapter for NightshadeRuntime { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges: &ChallengesResult, generate_storage_proof: bool, ) -> Result { @@ -717,6 +724,7 @@ impl RuntimeAdapter for NightshadeRuntime { transactions, last_validator_proposals, gas_price, + gas_limit, challenges, ) { Ok(result) => Ok(result), @@ -742,6 +750,7 @@ impl RuntimeAdapter for NightshadeRuntime { transactions: &[SignedTransaction], last_validator_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges: &ChallengesResult, ) -> Result { let trie = Arc::new(Trie::from_recorded_storage(partial_storage)); @@ -756,6 +765,7 @@ impl RuntimeAdapter for NightshadeRuntime { transactions, last_validator_proposals, gas_price, + gas_limit, challenges, ) } @@ -1007,7 +1017,7 @@ mod test { Action, CreateAccountAction, SignedTransaction, StakeAction, }; use near_primitives::types::{ - AccountId, Balance, BlockIndex, EpochId, Nonce, ShardId, StateRoot, ValidatorStake, + AccountId, Balance, BlockIndex, EpochId, Gas, Nonce, ShardId, StateRoot, ValidatorStake, }; use near_primitives::views::{AccountView, EpochValidatorInfo, QueryResponse}; use near_store::create_store; @@ -1051,6 +1061,7 @@ mod test { transactions: &[SignedTransaction], last_proposals: &[ValidatorStake], gas_price: Balance, + gas_limit: Gas, challenges: &ChallengesResult, ) -> (StateRoot, Vec, ReceiptResult) { let result = self @@ -1065,6 +1076,7 @@ mod test { transactions, last_proposals, gas_price, + gas_limit, challenges, ) .unwrap(); @@ -1168,6 +1180,7 @@ mod test { &transactions[i as usize], self.last_shard_proposals.get(&i).unwrap_or(&vec![]), self.runtime.genesis_config.gas_price, + u64::max_value(), &challenges_result, ); self.state_roots[i as usize] = state_root; diff --git a/runtime/runtime-params-estimator/src/testbed.rs b/runtime/runtime-params-estimator/src/testbed.rs index 493522bbb77..0db0112bb7d 100644 --- a/runtime/runtime-params-estimator/src/testbed.rs +++ b/runtime/runtime-params-estimator/src/testbed.rs @@ -59,6 +59,7 @@ impl RuntimeTestbed { epoch_length: 4, gas_price: 1, block_timestamp: 0, + gas_limit: 10_000_000, }; Self { workdir, trie, root, runtime, prev_receipts, apply_state } } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index fbdfe036522..b63503ba251 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -8,7 +8,7 @@ extern crate lazy_static; use std::collections::{HashMap, HashSet}; use std::convert::TryInto; -use borsh::BorshSerialize; +use borsh::{BorshDeserialize, BorshSerialize}; use kvdb::DBValue; use near_crypto::PublicKey; @@ -24,9 +24,9 @@ use near_primitives::types::{ AccountId, Balance, BlockIndex, Gas, Nonce, StateRoot, ValidatorStake, }; use near_primitives::utils::{ - create_nonce_with_nonce, is_valid_account_id, key_for_pending_data_count, - key_for_postponed_receipt, key_for_postponed_receipt_id, key_for_received_data, system_account, - ACCOUNT_DATA_SEPARATOR, + create_nonce_with_nonce, is_valid_account_id, key_for_delayed_receipt, + key_for_pending_data_count, key_for_postponed_receipt, key_for_postponed_receipt_id, + key_for_received_data, system_account, ACCOUNT_DATA_SEPARATOR, }; use near_runtime_fees::RuntimeFeesConfig; use near_store::{ @@ -47,6 +47,7 @@ pub use crate::store::StateRecord; use near_primitives::errors::{ ActionError, ExecutionError, InvalidAccessKeyError, InvalidTxError, RuntimeError, }; +use near_primitives::utils::col::DELAYED_RECEIPT_INDICES; use std::cmp::max; use std::sync::Arc; @@ -72,6 +73,8 @@ pub struct ApplyState { pub gas_price: Balance, /// A block timestamp pub block_timestamp: u64, + /// Gas limit for a given chunk + pub gas_limit: Gas, } /// Contains information to update validators accounts at the first block of a new epoch. @@ -113,6 +116,15 @@ pub struct ApplyResult { pub stats: ApplyStats, } +/// Stores indices for a persistent queue for delayed receipts that didn't fit into a block. +#[derive(Default, BorshSerialize, BorshDeserialize)] +pub struct DelayedReceiptIndices { + // First inclusive index in the queue. + first_index: u64, + // Exclusive end index of the queue + next_available_index: u64, +} + #[derive(Debug)] pub struct ActionResult { pub gas_burnt: Gas, @@ -913,6 +925,7 @@ impl Runtime { Ok(()) } + /// Applies new singed transactions and incoming receipts for some chunk/shard on top of /// given trie and the given state root. /// If the validator accounts update is provided, updates validators accounts. @@ -948,21 +961,32 @@ impl Runtime { let mut validator_proposals = vec![]; let mut local_receipts = vec![]; let mut tx_result = vec![]; + let mut total_gas_burnt = 0; for signed_transaction in transactions { - tx_result.push(self.process_transaction( + let result = self.process_transaction( &mut state_update, apply_state, signed_transaction, &mut local_receipts, &mut new_receipts, &mut stats, - )?); + )?; + total_gas_burnt += result.outcome.gas_burnt; + + tx_result.push(result); } - for receipt in local_receipts.iter().chain(prev_receipts.iter()) { + let mut delayed_receipts_indices: DelayedReceiptIndices = + get(&state_update, DELAYED_RECEIPT_INDICES)?.unwrap_or_default(); + let mut delayed_receipts_changed = false; + + let mut process_receipt = |receipt: &Receipt, + state_update: &mut TrieUpdate, + total_gas_burnt: &mut Gas| + -> Result<_, StorageError> { self.process_receipt( - &mut state_update, + state_update, apply_state, receipt, &mut new_receipts, @@ -970,7 +994,47 @@ impl Runtime { &mut stats, )? .into_iter() - .for_each(|res| tx_result.push(res)); + .for_each(|res| { + *total_gas_burnt += res.outcome.gas_burnt; + tx_result.push(res); + }); + Ok(()) + }; + + while delayed_receipts_indices.first_index < delayed_receipts_indices.next_available_index { + if total_gas_burnt >= apply_state.gas_limit { + break; + } + let key = key_for_delayed_receipt(delayed_receipts_indices.first_index); + let receipt: Receipt = get(&state_update, &key)?.ok_or_else(|| { + StorageError::StorageInconsistentState(format!( + "Delayed receipt #{} should be in the state", + delayed_receipts_indices.first_index + )) + })?; + state_update.remove(&key); + delayed_receipts_indices.first_index += 1; + process_receipt(&receipt, &mut state_update, &mut total_gas_burnt)?; + delayed_receipts_changed = true; + } + + for receipt in local_receipts.iter().chain(prev_receipts.iter()) { + if total_gas_burnt < apply_state.gas_limit { + process_receipt(&receipt, &mut state_update, &mut total_gas_burnt)?; + } else { + // Saving to the state as a delayed receipt. + set( + &mut state_update, + key_for_delayed_receipt(delayed_receipts_indices.next_available_index), + receipt, + ); + delayed_receipts_indices.next_available_index += 1; + delayed_receipts_changed = true; + } + } + + if delayed_receipts_changed { + set(&mut state_update, DELAYED_RECEIPT_INDICES.to_vec(), &delayed_receipts_indices); } check_balance( @@ -1201,8 +1265,13 @@ mod tests { let (store_update, root) = trie_changes.into(trie.clone()).unwrap(); store_update.commit().unwrap(); - let apply_state = - ApplyState { block_index: 0, epoch_length: 3, gas_price: 100, block_timestamp: 100 }; + let apply_state = ApplyState { + block_index: 0, + epoch_length: 3, + gas_price: 100, + block_timestamp: 100, + gas_limit: 10_000_000, + }; runtime.apply(trie, root, &None, &apply_state, &[], &[]).unwrap(); } @@ -1228,8 +1297,13 @@ mod tests { let (store_update, root) = trie_changes.into(trie.clone()).unwrap(); store_update.commit().unwrap(); - let apply_state = - ApplyState { block_index: 0, epoch_length: 3, gas_price: 100, block_timestamp: 100 }; + let apply_state = ApplyState { + block_index: 0, + epoch_length: 3, + gas_price: 100, + block_timestamp: 100, + gas_limit: 10_000_000, + }; let validator_accounts_update = ValidatorAccountsUpdate { stake_info: vec![(account_id.clone(), initial_locked)].into_iter().collect(), diff --git a/runtime/runtime/tests/runtime_group_tools.rs b/runtime/runtime/tests/runtime_group_tools.rs index f9b4ed7e70e..cd5e80fb068 100644 --- a/runtime/runtime/tests/runtime_group_tools.rs +++ b/runtime/runtime/tests/runtime_group_tools.rs @@ -49,6 +49,7 @@ impl StandaloneRuntime { epoch_length: 4, gas_price: 1, block_timestamp: 0, + gas_limit: 1_000_000_000, }; Self { apply_state, runtime, trie, signer, root: root.hash } diff --git a/test-utils/testlib/src/user/runtime_user.rs b/test-utils/testlib/src/user/runtime_user.rs index c28c71a8f60..59ed5f1385f 100644 --- a/test-utils/testlib/src/user/runtime_user.rs +++ b/test-utils/testlib/src/user/runtime_user.rs @@ -102,6 +102,7 @@ impl RuntimeUser { block_timestamp: 0, epoch_length: client.epoch_length, gas_price: INITIAL_GAS_PRICE, + gas_limit: u64::max_value(), } } From ff63704437764c3131ced9b9206b76444977d218 Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Wed, 30 Oct 2019 15:54:59 -0700 Subject: [PATCH 2/7] Rollbakc testnet.json --- near/res/testnet.json | 1 - 1 file changed, 1 deletion(-) diff --git a/near/res/testnet.json b/near/res/testnet.json index baa8ebeb82d..e4adde8602d 100644 --- a/near/res/testnet.json +++ b/near/res/testnet.json @@ -104579,7 +104579,6 @@ "validator_kickout_threshold": 80, "gas_price_adjustment_rate": 1, "runtime_config": { - "max_chunk_gas_burnt": "account_length_baseline_cost_per_block": "6561", "transaction_costs": { "storage_usage_config": { From 788f32032306a19c6f1af3dcabc72cc29cd43885 Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Thu, 31 Oct 2019 14:04:08 -0700 Subject: [PATCH 3/7] Fix balance checker. Add test for delayed receipts --- core/primitives/src/errors.rs | 8 ++ runtime/runtime/src/balance_checker.rs | 51 +++++++++++- runtime/runtime/src/lib.rs | 107 +++++++++++++++++-------- test-utils/testlib/src/fees_utils.rs | 2 +- 4 files changed, 130 insertions(+), 38 deletions(-) diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 3400494852c..dc8aeb1f88c 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -173,10 +173,12 @@ pub struct BalanceMismatchError { pub incoming_validator_rewards: Balance, pub initial_accounts_balance: Balance, pub incoming_receipts_balance: Balance, + pub processed_delayed_receipts_balance: Balance, pub initial_postponed_receipts_balance: Balance, // Output balances pub final_accounts_balance: Balance, pub outgoing_receipts_balance: Balance, + pub new_delayed_receipts_balance: Balance, pub final_postponed_receipts_balance: Balance, pub total_rent_paid: Balance, pub total_validator_reward: Balance, @@ -191,10 +193,12 @@ impl Display for BalanceMismatchError { .incoming_validator_rewards .saturating_add(self.initial_accounts_balance) .saturating_add(self.incoming_receipts_balance) + .saturating_add(self.processed_delayed_receipts_balance) .saturating_add(self.initial_postponed_receipts_balance); let final_balance = self .final_accounts_balance .saturating_add(self.outgoing_receipts_balance) + .saturating_add(self.new_delayed_receipts_balance) .saturating_add(self.final_postponed_receipts_balance) .saturating_add(self.total_rent_paid) .saturating_add(self.total_validator_reward) @@ -207,10 +211,12 @@ impl Display for BalanceMismatchError { \tIncoming validator rewards sum: {}\n\ \tInitial accounts balance sum: {}\n\ \tIncoming receipts balance sum: {}\n\ + \tProcessed delayed receipts balance sum: {}\n\ \tInitial postponed receipts balance sum: {}\n\ Outputs:\n\ \tFinal accounts balance sum: {}\n\ \tOutgoing receipts balance sum: {}\n\ + \tNew delayed receipts balance sum: {}\n\ \tFinal postponed receipts balance sum: {}\n\ \tTotal rent paid: {}\n\ \tTotal validators reward: {}\n\ @@ -221,9 +227,11 @@ impl Display for BalanceMismatchError { self.incoming_validator_rewards, self.initial_accounts_balance, self.incoming_receipts_balance, + self.processed_delayed_receipts_balance, self.initial_postponed_receipts_balance, self.final_accounts_balance, self.outgoing_receipts_balance, + self.new_delayed_receipts_balance, self.final_postponed_receipts_balance, self.total_rent_paid, self.total_validator_reward, diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 99555f74783..13b032ef71c 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -2,12 +2,15 @@ use crate::config::{ safe_add_balance, safe_add_gas, safe_gas_to_balance, total_deposit, total_exec_fees, total_prepaid_gas, }; -use crate::{ApplyStats, ValidatorAccountsUpdate, OVERFLOW_CHECKED_ERR}; +use crate::{ApplyStats, DelayedReceiptIndices, ValidatorAccountsUpdate, OVERFLOW_CHECKED_ERR}; use near_primitives::errors::{BalanceMismatchError, InvalidTxError, RuntimeError, StorageError}; use near_primitives::receipt::{Receipt, ReceiptEnum}; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, Balance}; -use near_primitives::utils::{key_for_postponed_receipt_id, system_account}; +use near_primitives::utils::col::DELAYED_RECEIPT_INDICES; +use near_primitives::utils::{ + key_for_delayed_receipt, key_for_postponed_receipt_id, system_account, +}; use near_runtime_fees::RuntimeFeesConfig; use near_store::{get, get_account, get_receipt, TrieUpdate}; use std::collections::HashSet; @@ -24,11 +27,42 @@ pub(crate) fn check_balance( new_receipts: &[Receipt], stats: &ApplyStats, ) -> Result<(), RuntimeError> { + // Delayed receipts + let initial_delayed_receipt_indices: DelayedReceiptIndices = + get(&initial_state, DELAYED_RECEIPT_INDICES)?.unwrap_or_default(); + let final_delayed_receipt_indices: DelayedReceiptIndices = + get(&final_state, DELAYED_RECEIPT_INDICES)?.unwrap_or_default(); + let get_delayed_receipts = |from_index, to_index, state| { + (from_index..to_index) + .map(|index| { + get(state, &key_for_delayed_receipt(index))?.ok_or_else(|| { + StorageError::StorageInconsistentState(format!( + "Delayed receipt #{} should be in the state", + index + )) + }) + }) + .collect::, StorageError>>() + }; + // Previously delayed receipts that were processed during this time. + let processed_delayed_receipts = get_delayed_receipts( + initial_delayed_receipt_indices.first_index, + final_delayed_receipt_indices.first_index, + &initial_state, + )?; + // Receipts that were not processed during this time and now delayed. + let new_delayed_receipts = get_delayed_receipts( + initial_delayed_receipt_indices.next_available_index, + final_delayed_receipt_indices.next_available_index, + &final_state, + )?; + // Accounts let mut all_accounts_ids: HashSet = transactions .iter() .map(|tx| tx.transaction.signer_id.clone()) .chain(prev_receipts.iter().map(|r| r.receiver_id.clone())) + .chain(processed_delayed_receipts.iter().map(|r| r.receiver_id.clone())) .collect(); let incoming_validator_rewards = if let Some(validator_accounts_update) = validator_accounts_update { @@ -86,12 +120,15 @@ pub(crate) fn check_balance( }; let incoming_receipts_balance = receipts_cost(prev_receipts); let outgoing_receipts_balance = receipts_cost(new_receipts); + let processed_delayed_receipts_balance = receipts_cost(&processed_delayed_receipts); + let new_delayed_receipts_balance = receipts_cost(&new_delayed_receipts); // Postponed actions receipts. The receipts can be postponed and stored with the receiver's // account ID when the input data is not received yet. // We calculate all potential receipts IDs that might be postponed initially or after the // execution. let all_potential_postponed_receipt_ids = prev_receipts .iter() + .chain(processed_delayed_receipts.iter()) .map(|receipt| { let account_id = &receipt.receiver_id; match &receipt.receipt { @@ -132,9 +169,11 @@ pub(crate) fn check_balance( let initial_balance = incoming_validator_rewards + initial_accounts_balance + incoming_receipts_balance + + processed_delayed_receipts_balance + initial_postponed_receipts_balance; let final_balance = final_accounts_balance + outgoing_receipts_balance + + new_delayed_receipts_balance + final_postponed_receipts_balance + stats.total_rent_paid + stats.total_validator_reward @@ -142,12 +181,16 @@ pub(crate) fn check_balance( + stats.total_balance_slashed; if initial_balance != final_balance { Err(BalanceMismatchError { + // Inputs incoming_validator_rewards, initial_accounts_balance, - final_accounts_balance, incoming_receipts_balance, - outgoing_receipts_balance, + processed_delayed_receipts_balance, initial_postponed_receipts_balance, + // Outputs + final_accounts_balance, + outgoing_receipts_balance, + new_delayed_receipts_balance, final_postponed_receipts_balance, total_rent_paid: stats.total_rent_paid, total_validator_reward: stats.total_validator_reward, diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index b63503ba251..07cef7483a4 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -1217,6 +1217,9 @@ mod tests { use testlib::runtime_utils::{alice_account, bob_account}; use super::*; + use near_crypto::KeyType; + use near_primitives::transaction::TransferAction; + use testlib::fees_utils::{gas_burnt_to_reward, GAS_PRICE}; #[test] fn test_get_and_set_accounts() { @@ -1248,18 +1251,20 @@ mod tests { /* Apply tests */ /***************/ - #[test] - fn test_apply_no_op() { + fn setup_runtime( + initial_balance: Balance, + initial_locked: Balance, + gas_limit: Gas, + ) -> (Runtime, Arc, CryptoHash, ApplyState) { let trie = create_trie(); let root = MerkleHash::default(); let runtime = Runtime::new(RuntimeConfig::default()); let account_id = alice_account(); - let initial_balance = 1_000_000; - let mut initial_state = TrieUpdate::new(trie.clone(), root); - let initial_account = Account::new(initial_balance, hash(&[]), 0); + let mut initial_account = Account::new(initial_balance, hash(&[]), 0); + initial_account.locked = initial_locked; set_account(&mut initial_state, &account_id, &initial_account); let trie_changes = initial_state.finalize().unwrap(); let (store_update, root) = trie_changes.into(trie.clone()).unwrap(); @@ -1268,46 +1273,31 @@ mod tests { let apply_state = ApplyState { block_index: 0, epoch_length: 3, - gas_price: 100, + gas_price: GAS_PRICE, block_timestamp: 100, - gas_limit: 10_000_000, + gas_limit, }; + (runtime, trie, root, apply_state) + } + + #[test] + fn test_apply_no_op() { + let (runtime, trie, root, apply_state) = setup_runtime(1_000_000, 0, 10_000_000); runtime.apply(trie, root, &None, &apply_state, &[], &[]).unwrap(); } #[test] fn test_apply_check_balance_validation_rewards() { - let trie = create_trie(); - let root = MerkleHash::default(); - let runtime = Runtime::new(RuntimeConfig::default()); - - let account_id = alice_account(); - - let initial_balance = 1_000_000; let initial_locked = 500_000; let reward = 10_000_000; let small_refund = 500; - - let mut initial_state = TrieUpdate::new(trie.clone(), root); - let mut initial_account = Account::new(initial_balance, hash(&[]), 0); - initial_account.locked = initial_locked; - set_account(&mut initial_state, &account_id, &initial_account); - let trie_changes = initial_state.finalize().unwrap(); - let (store_update, root) = trie_changes.into(trie.clone()).unwrap(); - store_update.commit().unwrap(); - - let apply_state = ApplyState { - block_index: 0, - epoch_length: 3, - gas_price: 100, - block_timestamp: 100, - gas_limit: 10_000_000, - }; + let (runtime, trie, root, apply_state) = + setup_runtime(1_000_000, initial_locked, 10_000_000); let validator_accounts_update = ValidatorAccountsUpdate { - stake_info: vec![(account_id.clone(), initial_locked)].into_iter().collect(), - validator_rewards: vec![(account_id.clone(), reward)].into_iter().collect(), + stake_info: vec![(alice_account(), initial_locked)].into_iter().collect(), + validator_rewards: vec![(alice_account(), reward)].into_iter().collect(), last_proposals: Default::default(), protocol_treasury_account_id: None, slashed_accounts: HashSet::default(), @@ -1319,9 +1309,60 @@ mod tests { root, &Some(validator_accounts_update), &apply_state, - &[Receipt::new_refund(&account_id, small_refund)], + &[Receipt::new_refund(&alice_account(), small_refund)], &[], ) .unwrap(); } + + #[test] + fn test_apply_delayed_receipts() { + let initial_balance = 1_000_000; + let initial_locked = 500_000; + let small_transfer = 10_000; + let gas_limit = 1; + let (runtime, trie, mut root, apply_state) = + setup_runtime(initial_balance, initial_locked, gas_limit); + + let n = 10; + let receipts: Vec<_> = (0..n) + .map(|i| Receipt { + predecessor_id: bob_account(), + receiver_id: alice_account(), + receipt_id: create_nonce_with_nonce(&CryptoHash::default(), i), + receipt: ReceiptEnum::Action(ActionReceipt { + signer_id: bob_account(), + signer_public_key: PublicKey::empty(KeyType::ED25519), + gas_price: 100, + output_data_receivers: vec![], + input_data_ids: vec![], + actions: vec![Action::Transfer(TransferAction { + deposit: small_transfer + Balance::from(i), + })], + }), + }) + .collect(); + + let reward_per_receipt = gas_burnt_to_reward( + runtime.config.transaction_costs.action_receipt_creation_config.exec_fee() + + runtime.config.transaction_costs.action_creation_config.transfer_cost.exec_fee(), + ); + + for i in 1..=n { + let prev_receipts: &[Receipt] = if i == 1 { &receipts } else { &[] }; + let apply_result = + runtime.apply(trie.clone(), root, &None, &apply_state, prev_receipts, &[]).unwrap(); + let (store_update, new_root) = apply_result.trie_changes.into(trie.clone()).unwrap(); + root = new_root; + store_update.commit().unwrap(); + let state = TrieUpdate::new(trie.clone(), root); + let account = get_account(&state, &alice_account()).unwrap().unwrap(); + assert_eq!( + account.amount, + initial_balance + + (small_transfer + reward_per_receipt) * Balance::from(i) + + Balance::from(i * (i - 1) / 2) + ) + } + } } diff --git a/test-utils/testlib/src/fees_utils.rs b/test-utils/testlib/src/fees_utils.rs index 1cee91cd0b6..25fa236bc34 100644 --- a/test-utils/testlib/src/fees_utils.rs +++ b/test-utils/testlib/src/fees_utils.rs @@ -4,7 +4,7 @@ use near::config::INITIAL_GAS_PRICE; use near_primitives::types::{Balance, Gas}; use near_runtime_fees::RuntimeFeesConfig; -const GAS_PRICE: u128 = INITIAL_GAS_PRICE; +pub const GAS_PRICE: u128 = INITIAL_GAS_PRICE; pub fn gas_burnt_to_reward(gas_burnt: Gas) -> Balance { let cfg = RuntimeFeesConfig::default(); From a62b4413cbc706b17c4f51f75fddf2ba6dd8f89d Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Tue, 5 Nov 2019 10:36:28 -0800 Subject: [PATCH 4/7] Re-enable test. Fix comments --- chain/client/tests/challenges.rs | 26 ++++++++++++++++---------- runtime/runtime/src/balance_checker.rs | 4 ++-- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/chain/client/tests/challenges.rs b/chain/client/tests/challenges.rs index fb4cafa0991..393154376e2 100644 --- a/chain/client/tests/challenges.rs +++ b/chain/client/tests/challenges.rs @@ -173,11 +173,7 @@ fn test_verify_chunk_invalid_proofs_challenge() { ); } -// TODO(#1604): Fix partial state construction in the test. We shouldn't hardcode the partial state, -// because it breaks the development of Runtime that affects state. -// Instead we should reconstruct the partial state as part of the test. #[test] -#[ignore] fn test_verify_chunk_invalid_state_challenge() { let store1 = create_test_store(); let genesis_config = GenesisConfig::test(vec!["test0", "test1"], 1); @@ -279,12 +275,22 @@ fn test_verify_chunk_invalid_state_challenge() { assert_eq!(prev_merkle_proofs[0], challenge_body.prev_merkle_proof); assert_eq!(merkle_proofs[0], challenge_body.merkle_proof); assert_eq!( - vec![vec![ - 3, 1, 0, 0, 0, 16, 54, 106, 135, 107, 146, 249, 30, 224, 4, 250, 77, 43, 107, 71, - 32, 36, 160, 74, 172, 80, 43, 254, 111, 201, 245, 124, 145, 98, 123, 210, 44, 242, - 167, 124, 2, 0, 0, 0, 0, 0, - ]], - challenge_body.partial_state + challenge_body.partial_state, + vec![ + vec![ + 1, 7, 0, 227, 6, 86, 139, 125, 37, 242, 104, 89, 182, 115, 113, 193, 120, 119, + 33, 26, 201, 6, 127, 176, 76, 7, 26, 49, 95, 52, 178, 159, 143, 117, 52, 30, + 175, 188, 91, 174, 142, 135, 98, 116, 150, 226, 129, 204, 53, 64, 77, 100, 76, + 30, 35, 91, 181, 116, 222, 89, 72, 223, 126, 155, 43, 85, 154, 123, 65, 104, + 88, 146, 81, 64, 114, 10, 155, 246, 47, 39, 58, 223, 4, 22, 25, 219, 175, 9, + 240, 3, 80, 88, 189, 162, 254, 21, 231, 234, 116, 125, 124, 2, 0, 0, 0, 0, 0 + ], + vec![ + 3, 1, 0, 0, 0, 16, 54, 106, 135, 107, 146, 249, 30, 224, 4, 250, 77, 43, 107, + 71, 32, 36, 160, 74, 172, 80, 43, 254, 111, 201, 245, 124, 145, 98, 123, 210, + 44, 242, 167, 124, 2, 0, 0, 0, 0, 0 + ] + ], ); } let challenge = diff --git a/runtime/runtime/src/balance_checker.rs b/runtime/runtime/src/balance_checker.rs index 13b032ef71c..15c4266333c 100644 --- a/runtime/runtime/src/balance_checker.rs +++ b/runtime/runtime/src/balance_checker.rs @@ -44,13 +44,13 @@ pub(crate) fn check_balance( }) .collect::, StorageError>>() }; - // Previously delayed receipts that were processed during this time. + // Previously delayed receipts that were processed this time. let processed_delayed_receipts = get_delayed_receipts( initial_delayed_receipt_indices.first_index, final_delayed_receipt_indices.first_index, &initial_state, )?; - // Receipts that were not processed during this time and now delayed. + // Receipts that were not processed this time and are delayed now. let new_delayed_receipts = get_delayed_receipts( initial_delayed_receipt_indices.next_available_index, final_delayed_receipt_indices.next_available_index, From ae8ddc32be31322ae61316b0692f18e3e2e83c23 Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Wed, 6 Nov 2019 13:38:59 -0800 Subject: [PATCH 5/7] Address comments. Add tests --- chain/chain/src/test_utils.rs | 1 - chain/chain/src/types.rs | 1 - chain/client/src/client.rs | 18 +-- near/src/runtime.rs | 4 +- .../runtime-params-estimator/src/testbed.rs | 2 +- runtime/runtime/src/lib.rs | 141 +++++++++++++++--- .../runtime/tests/runtime_group_tools/mod.rs | 2 +- 7 files changed, 131 insertions(+), 38 deletions(-) diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index 2e3d4997da0..2464058efd5 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -451,7 +451,6 @@ impl RuntimeAdapter for KeyValueRuntime { _block_index: BlockIndex, _block_timestamp: u64, _gas_price: Balance, - _gas_limit: Gas, _state_root: StateRoot, transaction: SignedTransaction, ) -> Result { diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs index add505bbbc2..cb022483d55 100644 --- a/chain/chain/src/types.rs +++ b/chain/chain/src/types.rs @@ -152,7 +152,6 @@ pub trait RuntimeAdapter: Send + Sync { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, - gas_limit: Gas, state_root: StateRoot, transaction: SignedTransaction, ) -> Result; diff --git a/chain/client/src/client.rs b/chain/client/src/client.rs index 3d55f45aa0d..3383a24e43b 100644 --- a/chain/client/src/client.rs +++ b/chain/client/src/client.rs @@ -1031,20 +1031,18 @@ impl Client { ) .inner .gas_price; - let (state_root, gas_limit) = - match self.chain.get_chunk_extra(&head.last_block_hash, shard_id) { - Ok(chunk_extra) => (chunk_extra.state_root.clone(), chunk_extra.gas_limit), - Err(_) => { - // Not being able to fetch a state root most likely implies that we haven't - // caught up with the next epoch yet. - return self.forward_tx(tx); - } - }; + let state_root = match self.chain.get_chunk_extra(&head.last_block_hash, shard_id) { + Ok(chunk_extra) => chunk_extra.state_root.clone(), + Err(_) => { + // Not being able to fetch a state root most likely implies that we haven't + // caught up with the next epoch yet. + return self.forward_tx(tx); + } + }; match self.runtime_adapter.validate_tx( head.height + 1, cur_block_header.inner.timestamp, gas_price, - gas_limit, state_root, tx, ) { diff --git a/near/src/runtime.rs b/near/src/runtime.rs index 5d6b0fca1a1..8b2c7e371eb 100644 --- a/near/src/runtime.rs +++ b/near/src/runtime.rs @@ -601,7 +601,6 @@ impl RuntimeAdapter for NightshadeRuntime { block_index: BlockIndex, block_timestamp: u64, gas_price: Balance, - gas_limit: Gas, state_root: StateRoot, transaction: SignedTransaction, ) -> Result { @@ -611,7 +610,8 @@ impl RuntimeAdapter for NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, - gas_limit, + // NOTE: verify transaction doesn't use gas limit + gas_limit: u64::max_value(), }; if let Err(err) = self.runtime.verify_and_charge_transaction( diff --git a/runtime/runtime-params-estimator/src/testbed.rs b/runtime/runtime-params-estimator/src/testbed.rs index 30af5642612..d2b72bbdb65 100644 --- a/runtime/runtime-params-estimator/src/testbed.rs +++ b/runtime/runtime-params-estimator/src/testbed.rs @@ -59,7 +59,7 @@ impl RuntimeTestbed { epoch_length: 4, gas_price: 1, block_timestamp: 0, - gas_limit: 10_000_000, + gas_limit: u64::max_value(), }; Self { workdir, trie, root, runtime, prev_receipts, apply_state } } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 4a235f9c387..c28e813d1a1 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -1315,7 +1315,7 @@ mod tests { } #[test] - fn test_apply_delayed_receipts() { + fn test_apply_delayed_receipts_feed_all_at_once() { let initial_balance = 1_000_000; let initial_locked = 500_000; let small_transfer = 10_000; @@ -1324,30 +1324,15 @@ mod tests { setup_runtime(initial_balance, initial_locked, gas_limit); let n = 10; - let receipts: Vec<_> = (0..n) - .map(|i| Receipt { - predecessor_id: bob_account(), - receiver_id: alice_account(), - receipt_id: create_nonce_with_nonce(&CryptoHash::default(), i), - receipt: ReceiptEnum::Action(ActionReceipt { - signer_id: bob_account(), - signer_public_key: PublicKey::empty(KeyType::ED25519), - gas_price: 100, - output_data_receivers: vec![], - input_data_ids: vec![], - actions: vec![Action::Transfer(TransferAction { - deposit: small_transfer + Balance::from(i), - })], - }), - }) - .collect(); + let receipts = generate_receipts(small_transfer, n); let reward_per_receipt = gas_burnt_to_reward( runtime.config.transaction_costs.action_receipt_creation_config.exec_fee() + runtime.config.transaction_costs.action_creation_config.transfer_cost.exec_fee(), ); - for i in 1..=n { + // Checking n receipts delayed by 1 + 3 extra + for i in 1..=n + 3 { let prev_receipts: &[Receipt] = if i == 1 { &receipts } else { &[] }; let apply_result = runtime.apply(trie.clone(), root, &None, &apply_state, prev_receipts, &[]).unwrap(); @@ -1356,12 +1341,124 @@ mod tests { store_update.commit().unwrap(); let state = TrieUpdate::new(trie.clone(), root); let account = get_account(&state, &alice_account()).unwrap().unwrap(); + let capped_i = std::cmp::min(i, n); assert_eq!( account.amount, initial_balance - + (small_transfer + reward_per_receipt) * Balance::from(i) - + Balance::from(i * (i - 1) / 2) - ) + + (small_transfer + reward_per_receipt) * Balance::from(capped_i) + + Balance::from(capped_i * (capped_i - 1) / 2) + ); + } + } + + #[test] + fn test_apply_delayed_receipts_add_more_using_chunks() { + let initial_balance = 1_000_000; + let initial_locked = 500_000; + let small_transfer = 10_000; + let (runtime, trie, mut root, mut apply_state) = + setup_runtime(initial_balance, initial_locked, 1); + + let receipt_gas_cost = + runtime.config.transaction_costs.action_receipt_creation_config.exec_fee() + + runtime.config.transaction_costs.action_creation_config.transfer_cost.exec_fee(); + apply_state.gas_limit = receipt_gas_cost * 3; + + let n = 40; + let receipts = generate_receipts(small_transfer, n); + let mut receipt_chunks = receipts.chunks_exact(4); + + let reward_per_receipt = gas_burnt_to_reward(receipt_gas_cost); + + // Every time we'll process 3 receipts, so we need n / 3 rounded up. Then we do 3 extra. + for i in 1..=n / 3 + 3 { + let prev_receipts: &[Receipt] = receipt_chunks.next().unwrap_or_default(); + let apply_result = + runtime.apply(trie.clone(), root, &None, &apply_state, prev_receipts, &[]).unwrap(); + let (store_update, new_root) = apply_result.trie_changes.into(trie.clone()).unwrap(); + root = new_root; + store_update.commit().unwrap(); + let state = TrieUpdate::new(trie.clone(), root); + let account = get_account(&state, &alice_account()).unwrap().unwrap(); + let capped_i = std::cmp::min(i * 3, n); + assert_eq!( + account.amount, + initial_balance + + (small_transfer + reward_per_receipt) * Balance::from(capped_i) + + Balance::from(capped_i * (capped_i - 1) / 2) + ); + } + } + + #[test] + fn test_apply_delayed_receipts_adjustable_gas_limit() { + let initial_balance = 1_000_000; + let initial_locked = 500_000; + let small_transfer = 10_000; + let (runtime, trie, mut root, mut apply_state) = + setup_runtime(initial_balance, initial_locked, 1); + + let receipt_gas_cost = + runtime.config.transaction_costs.action_receipt_creation_config.exec_fee() + + runtime.config.transaction_costs.action_creation_config.transfer_cost.exec_fee(); + + let n = 120; + let receipts = generate_receipts(small_transfer, n); + let mut receipt_chunks = receipts.chunks_exact(4); + + let reward_per_receipt = gas_burnt_to_reward(receipt_gas_cost); + + let mut num_receipts_given = 0; + let mut num_receipts_processed = 0; + let mut num_receipts_per_block = 1; + // Test adjusts gas limit based on the number of receipt given and number of receipts processed. + while num_receipts_processed < n { + if num_receipts_given > num_receipts_processed { + num_receipts_per_block += 1; + } else if num_receipts_per_block > 1 { + num_receipts_per_block -= 1; + } + apply_state.gas_limit = num_receipts_per_block * receipt_gas_cost; + let prev_receipts: &[Receipt] = receipt_chunks.next().unwrap_or_default(); + num_receipts_given += prev_receipts.len() as u64; + let apply_result = + runtime.apply(trie.clone(), root, &None, &apply_state, prev_receipts, &[]).unwrap(); + let (store_update, new_root) = apply_result.trie_changes.into(trie.clone()).unwrap(); + root = new_root; + store_update.commit().unwrap(); + let state = TrieUpdate::new(trie.clone(), root); + num_receipts_processed += apply_result.outcomes.len() as u64; + let account = get_account(&state, &alice_account()).unwrap().unwrap(); + assert_eq!( + account.amount, + initial_balance + + (small_transfer + reward_per_receipt) * Balance::from(num_receipts_processed) + + Balance::from(num_receipts_processed * (num_receipts_processed - 1) / 2) + ); + println!( + "{} processed out of {} given. With limit {} receipts per block", + num_receipts_processed, num_receipts_given, num_receipts_per_block + ); } } + + fn generate_receipts(small_transfer: u128, n: u64) -> Vec { + (0..n) + .map(|i| Receipt { + predecessor_id: bob_account(), + receiver_id: alice_account(), + receipt_id: create_nonce_with_nonce(&CryptoHash::default(), i), + receipt: ReceiptEnum::Action(ActionReceipt { + signer_id: bob_account(), + signer_public_key: PublicKey::empty(KeyType::ED25519), + gas_price: 100, + output_data_receivers: vec![], + input_data_ids: vec![], + actions: vec![Action::Transfer(TransferAction { + deposit: small_transfer + Balance::from(i), + })], + }), + }) + .collect() + } } diff --git a/runtime/runtime/tests/runtime_group_tools/mod.rs b/runtime/runtime/tests/runtime_group_tools/mod.rs index 87a33bc339f..0c39a5062f6 100644 --- a/runtime/runtime/tests/runtime_group_tools/mod.rs +++ b/runtime/runtime/tests/runtime_group_tools/mod.rs @@ -51,7 +51,7 @@ impl StandaloneRuntime { epoch_length: 4, gas_price: 100, block_timestamp: 0, - gas_limit: 1_000_000_000, + gas_limit: u64::max_value(), }; Self { apply_state, runtime, trie, signer, root: root.hash } From 4b3ac9f4201c054ffdb801f4c447048b5feef233 Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Wed, 6 Nov 2019 17:02:03 -0800 Subject: [PATCH 6/7] Switch to gas_limit to an option --- near/src/runtime.rs | 15 ++++++++++----- runtime/runtime-params-estimator/src/testbed.rs | 2 +- runtime/runtime/src/lib.rs | 17 ++++++++++------- .../runtime/tests/runtime_group_tools/mod.rs | 2 +- test-utils/testlib/src/user/runtime_user.rs | 2 +- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/near/src/runtime.rs b/near/src/runtime.rs index afe2ee6088c..48f5d409d03 100644 --- a/near/src/runtime.rs +++ b/near/src/runtime.rs @@ -277,7 +277,7 @@ impl NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, - gas_limit, + gas_limit: Some(gas_limit), }; let apply_result = self @@ -615,7 +615,7 @@ impl RuntimeAdapter for NightshadeRuntime { gas_price, block_timestamp, // NOTE: verify transaction doesn't use gas limit - gas_limit: u64::max_value(), + gas_limit: None, }; if let Err(err) = self.runtime.verify_and_charge_transaction( @@ -644,7 +644,7 @@ impl RuntimeAdapter for NightshadeRuntime { epoch_length: self.genesis_config.epoch_length, gas_price, block_timestamp, - gas_limit, + gas_limit: Some(gas_limit), }; transactions .into_iter() @@ -1927,8 +1927,13 @@ mod test { 10, CryptoHash::default(), ); - let apply_state = - ApplyState { block_index: 1, epoch_length: 2, gas_price: 10, block_timestamp: 100 }; + let apply_state = ApplyState { + block_index: 1, + epoch_length: 2, + gas_price: 10, + block_timestamp: 100, + gas_limit: None, + }; let mut prefixes = HashSet::new(); prefixes.insert(prefix); let apply_result = env diff --git a/runtime/runtime-params-estimator/src/testbed.rs b/runtime/runtime-params-estimator/src/testbed.rs index ac6a31f676d..a54d7f5f976 100644 --- a/runtime/runtime-params-estimator/src/testbed.rs +++ b/runtime/runtime-params-estimator/src/testbed.rs @@ -60,7 +60,7 @@ impl RuntimeTestbed { epoch_length: 4, gas_price: 1, block_timestamp: 0, - gas_limit: u64::max_value(), + gas_limit: None, }; Self { workdir, trie, root, runtime, prev_receipts, apply_state } } diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index 2092a29c463..c63cf4e6114 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -72,8 +72,9 @@ pub struct ApplyState { pub gas_price: Balance, /// A block timestamp pub block_timestamp: u64, - /// Gas limit for a given chunk - pub gas_limit: Gas, + /// Gas limit for a given chunk. + /// If None is given, assumes there is no gas limit. + pub gas_limit: Option, } /// Contains information to update validators accounts at the first block of a new epoch. @@ -1002,8 +1003,10 @@ impl Runtime { Ok(()) }; + let gas_limit = apply_state.gas_limit.unwrap_or(Gas::max_value()); + while delayed_receipts_indices.first_index < delayed_receipts_indices.next_available_index { - if total_gas_burnt >= apply_state.gas_limit { + if total_gas_burnt >= gas_limit { break; } let key = key_for_delayed_receipt(delayed_receipts_indices.first_index); @@ -1020,7 +1023,7 @@ impl Runtime { } for receipt in local_receipts.iter().chain(prev_receipts.iter()) { - if total_gas_burnt < apply_state.gas_limit { + if total_gas_burnt < gas_limit { process_receipt(&receipt, &mut state_update, &mut total_gas_burnt)?; } else { // Saving to the state as a delayed receipt. @@ -1280,7 +1283,7 @@ mod tests { epoch_length: 3, gas_price: GAS_PRICE, block_timestamp: 100, - gas_limit, + gas_limit: Some(gas_limit), }; (runtime, trie, root, apply_state) @@ -1370,7 +1373,7 @@ mod tests { let receipt_gas_cost = runtime.config.transaction_costs.action_receipt_creation_config.exec_fee() + runtime.config.transaction_costs.action_creation_config.transfer_cost.exec_fee(); - apply_state.gas_limit = receipt_gas_cost * 3; + apply_state.gas_limit = Some(receipt_gas_cost * 3); let n = 40; let receipts = generate_receipts(small_transfer, n); @@ -1427,7 +1430,7 @@ mod tests { } else if num_receipts_per_block > 1 { num_receipts_per_block -= 1; } - apply_state.gas_limit = num_receipts_per_block * receipt_gas_cost; + apply_state.gas_limit = Some(num_receipts_per_block * receipt_gas_cost); let prev_receipts: &[Receipt] = receipt_chunks.next().unwrap_or_default(); num_receipts_given += prev_receipts.len() as u64; let apply_result = runtime diff --git a/runtime/runtime/tests/runtime_group_tools/mod.rs b/runtime/runtime/tests/runtime_group_tools/mod.rs index b96973c0512..8f424259c11 100644 --- a/runtime/runtime/tests/runtime_group_tools/mod.rs +++ b/runtime/runtime/tests/runtime_group_tools/mod.rs @@ -51,7 +51,7 @@ impl StandaloneRuntime { epoch_length: 4, gas_price: 100, block_timestamp: 0, - gas_limit: u64::max_value(), + gas_limit: None, }; Self { apply_state, runtime, trie, signer, root: root.hash } diff --git a/test-utils/testlib/src/user/runtime_user.rs b/test-utils/testlib/src/user/runtime_user.rs index b95cfcb634f..2e0944c80c2 100644 --- a/test-utils/testlib/src/user/runtime_user.rs +++ b/test-utils/testlib/src/user/runtime_user.rs @@ -114,7 +114,7 @@ impl RuntimeUser { block_timestamp: 0, epoch_length: client.epoch_length, gas_price: INITIAL_GAS_PRICE, - gas_limit: u64::max_value(), + gas_limit: None, } } From 228abe0e1104d2aeda29cd2dcd0dd3fd917ba2ab Mon Sep 17 00:00:00 2001 From: Evgeny Kuzyakov Date: Thu, 14 Nov 2019 13:06:13 -0800 Subject: [PATCH 7/7] Revert pub GAS_PRICE --- Cargo.lock | 1 + runtime/runtime/Cargo.toml | 1 + runtime/runtime/src/lib.rs | 13 +++++++------ test-utils/testlib/src/fees_utils.rs | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e613b99c2d6..abd1ee08863 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2229,6 +2229,7 @@ dependencies = [ "kvdb 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", + "near 0.4.4", "near-crypto 0.1.0", "near-metrics 0.1.0", "near-primitives 0.1.0", diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index af06965e55b..3a82479b0cf 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -40,5 +40,6 @@ rayon = "1.1" assert_matches = "1.3.0" testlib = { path = "../../test-utils/testlib" } +near = { path = "../../near" } genesis-populate = { path = "../../genesis-tools/genesis-populate"} diff --git a/runtime/runtime/src/lib.rs b/runtime/runtime/src/lib.rs index c63cf4e6114..016dd653bef 100644 --- a/runtime/runtime/src/lib.rs +++ b/runtime/runtime/src/lib.rs @@ -1219,16 +1219,17 @@ impl Runtime { #[cfg(test)] mod tests { + use super::*; + + use near::config::INITIAL_GAS_PRICE; + use near_crypto::KeyType; use near_primitives::hash::hash; + use near_primitives::transaction::TransferAction; use near_primitives::types::MerkleHash; use near_store::test_utils::create_trie; + use testlib::fees_utils::gas_burnt_to_reward; use testlib::runtime_utils::{alice_account, bob_account}; - use super::*; - use near_crypto::KeyType; - use near_primitives::transaction::TransferAction; - use testlib::fees_utils::{gas_burnt_to_reward, GAS_PRICE}; - #[test] fn test_get_and_set_accounts() { let trie = create_trie(); @@ -1281,7 +1282,7 @@ mod tests { let apply_state = ApplyState { block_index: 0, epoch_length: 3, - gas_price: GAS_PRICE, + gas_price: INITIAL_GAS_PRICE, block_timestamp: 100, gas_limit: Some(gas_limit), }; diff --git a/test-utils/testlib/src/fees_utils.rs b/test-utils/testlib/src/fees_utils.rs index 25fa236bc34..1cee91cd0b6 100644 --- a/test-utils/testlib/src/fees_utils.rs +++ b/test-utils/testlib/src/fees_utils.rs @@ -4,7 +4,7 @@ use near::config::INITIAL_GAS_PRICE; use near_primitives::types::{Balance, Gas}; use near_runtime_fees::RuntimeFeesConfig; -pub const GAS_PRICE: u128 = INITIAL_GAS_PRICE; +const GAS_PRICE: u128 = INITIAL_GAS_PRICE; pub fn gas_burnt_to_reward(gas_burnt: Gas) -> Balance { let cfg = RuntimeFeesConfig::default();