From aaf3c140fd77966815460ba45ecc3b7110ba1e05 Mon Sep 17 00:00:00 2001 From: Jakob Meier Date: Tue, 11 Apr 2023 11:25:37 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"feature:=20Implement=20compute=20usag?= =?UTF-8?q?e=20aggregation=20and=20limiting=20(#8=E2=80=A6=20(#8906)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …805)" This reverts commit 700ec29270f72f2e78a17029b4799a8228926c07. Canary nodes found an issue with this change: ``` thread '' panicked at 'assertion failed: `(left == right)` left: `2519443987198`, right: `2429311750949`: Compute usage must match burnt gas', ``` --- chain/chain/src/test_utils/kv_runtime.rs | 1 - chain/chain/src/types.rs | 2 -- core/primitives/src/transaction.rs | 12 +----------- runtime/near-vm-logic/src/logic.rs | 8 ++------ runtime/runtime/src/actions.rs | 7 ++----- runtime/runtime/src/config.rs | 6 +----- tools/state-viewer/src/contract_accounts.rs | 1 - 7 files changed, 6 insertions(+), 31 deletions(-) diff --git a/chain/chain/src/test_utils/kv_runtime.rs b/chain/chain/src/test_utils/kv_runtime.rs index 8f6e5b8ad21..7af4586d8b2 100644 --- a/chain/chain/src/test_utils/kv_runtime.rs +++ b/chain/chain/src/test_utils/kv_runtime.rs @@ -1170,7 +1170,6 @@ impl RuntimeAdapter for KeyValueRuntime { logs: vec![], receipt_ids: new_receipt_hashes, gas_burnt: 0, - compute_usage: Some(0), tokens_burnt: 0, executor_id: to.clone(), metadata: ExecutionMetadata::V1, diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs index 02674fe8d83..a88be70c394 100644 --- a/chain/chain/src/types.rs +++ b/chain/chain/src/types.rs @@ -631,7 +631,6 @@ mod tests { logs: vec!["outcome1".to_string()], receipt_ids: vec![hash(&[1])], gas_burnt: 100, - compute_usage: Some(200), tokens_burnt: 10000, executor_id: "alice".parse().unwrap(), metadata: ExecutionMetadata::V1, @@ -644,7 +643,6 @@ mod tests { logs: vec!["outcome2".to_string()], receipt_ids: vec![], gas_burnt: 0, - compute_usage: Some(0), tokens_burnt: 0, executor_id: "bob".parse().unwrap(), metadata: ExecutionMetadata::V1, diff --git a/core/primitives/src/transaction.rs b/core/primitives/src/transaction.rs index 23a6ddd6600..6ff6388b1ec 100644 --- a/core/primitives/src/transaction.rs +++ b/core/primitives/src/transaction.rs @@ -9,7 +9,6 @@ use borsh::{BorshDeserialize, BorshSerialize}; use near_crypto::{PublicKey, Signature}; use near_fmt::{AbbrBytes, Slice}; use near_primitives_core::profile::{ProfileDataV2, ProfileDataV3}; -use near_primitives_core::types::Compute; use std::borrow::Borrow; use std::fmt; use std::hash::{Hash, Hasher}; @@ -416,13 +415,6 @@ pub struct ExecutionOutcome { pub receipt_ids: Vec, /// The amount of the gas burnt by the given transaction or receipt. pub gas_burnt: Gas, - /// The amount of compute time spent by the given transaction or receipt. - // TODO(#8859): Treat this field in the same way as `gas_burnt`. - // At the moment this field is only set at runtime and is not persisted in the database. - // This means that when execution outcomes are read from the database, this value will not be - // set and any code that attempts to use it will crash. - #[borsh_skip] - pub compute_usage: Option, /// The amount of tokens burnt corresponding to the burnt gas amount. /// This value doesn't always equal to the `gas_burnt` multiplied by the gas price, because /// the prepaid gas price might be lower than the actual gas price and it creates a deficit. @@ -445,7 +437,7 @@ pub enum ExecutionMetadata { V1, /// V2: With ProfileData by legacy `Cost` enum V2(ProfileDataV2), - /// V3: With ProfileData by gas parameters + // V3: With ProfileData by gas parameters V3(ProfileDataV3), } @@ -461,7 +453,6 @@ impl fmt::Debug for ExecutionOutcome { .field("logs", &Slice(&self.logs)) .field("receipt_ids", &Slice(&self.receipt_ids)) .field("burnt_gas", &self.gas_burnt) - .field("compute_usage", &self.compute_usage.unwrap_or_default()) .field("tokens_burnt", &self.tokens_burnt) .field("status", &self.status) .field("metadata", &self.metadata) @@ -607,7 +598,6 @@ mod tests { logs: vec!["123".to_string(), "321".to_string()], receipt_ids: vec![], gas_burnt: 123, - compute_usage: Some(456), tokens_burnt: 1234000, executor_id: "alice".parse().unwrap(), metadata: ExecutionMetadata::V1, diff --git a/runtime/near-vm-logic/src/logic.rs b/runtime/near-vm-logic/src/logic.rs index 98c7d439cdd..32c72e9b4e0 100644 --- a/runtime/near-vm-logic/src/logic.rs +++ b/runtime/near-vm-logic/src/logic.rs @@ -15,9 +15,9 @@ use near_primitives_core::config::ExtCosts::*; use near_primitives_core::config::{ActionCosts, ExtCosts, VMConfig}; use near_primitives_core::runtime::fees::{transfer_exec_fee, transfer_send_fee}; use near_primitives_core::types::{ - AccountId, Balance, Compute, EpochHeight, Gas, GasDistribution, GasWeight, ProtocolVersion, - StorageUsage, + AccountId, Balance, EpochHeight, Gas, ProtocolVersion, StorageUsage, }; +use near_primitives_core::types::{GasDistribution, GasWeight}; use near_vm_errors::{FunctionCallError, InconsistentStateError}; use near_vm_errors::{HostError, VMLogicError}; use std::mem::size_of; @@ -2794,7 +2794,6 @@ impl<'a> VMLogic<'a> { let mut profile = self.gas_counter.profile_data(); profile.compute_wasm_instruction_cost(burnt_gas); - let compute_usage = profile.total_compute_usage(&self.config.ext_costs); VMOutcome { balance: self.current_account_balance, @@ -2802,7 +2801,6 @@ impl<'a> VMLogic<'a> { return_data: self.return_data, burnt_gas, used_gas, - compute_usage, logs: self.logs, profile, action_receipts: self.receipt_manager.action_receipts, @@ -2921,7 +2919,6 @@ pub struct VMOutcome { pub return_data: ReturnData, pub burnt_gas: Gas, pub used_gas: Gas, - pub compute_usage: Compute, pub logs: Vec, /// Data collected from making a contract call pub profile: ProfileDataV3, @@ -2955,7 +2952,6 @@ impl VMOutcome { return_data: ReturnData::None, burnt_gas: 0, used_gas: 0, - compute_usage: 0, logs: Vec::new(), profile: ProfileDataV3::default(), action_receipts: Vec::new(), diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index fa0e2191337..61a20c40020 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -1,6 +1,6 @@ use crate::config::{ - safe_add_compute, safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, - total_prepaid_send_fees, RuntimeConfig, + safe_add_gas, total_prepaid_exec_fees, total_prepaid_gas, total_prepaid_send_fees, + RuntimeConfig, }; use crate::ext::{ExternalError, RuntimeExt}; use crate::{metrics, ActionResult, ApplyState}; @@ -254,7 +254,6 @@ pub(crate) fn action_function_call( // return a real `gas_used` instead of the `gas_burnt` into `ActionResult` even for // `FunctionCall`s error. result.gas_used = safe_add_gas(result.gas_used, outcome.used_gas)?; - result.compute_usage = safe_add_compute(result.compute_usage, outcome.compute_usage)?; result.logs.extend(outcome.logs); result.profile.merge(&outcome.profile); if execution_succeeded { @@ -688,8 +687,6 @@ pub(crate) fn apply_delegate_action( // gas_used is incremented because otherwise the gas will be refunded. Refund function checks only gas_used. result.gas_used = safe_add_gas(result.gas_used, prepaid_send_fees)?; result.gas_burnt = safe_add_gas(result.gas_burnt, prepaid_send_fees)?; - // TODO(#8806): Support compute costs for actions. For now they match burnt gas. - result.compute_usage = safe_add_compute(result.compute_usage, prepaid_send_fees)?; result.new_receipts.push(new_receipt); Ok(()) diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index a95d7a10508..2d4228d2a32 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -14,7 +14,7 @@ use near_primitives::runtime::fees::{transfer_exec_fee, transfer_send_fee, Runti use near_primitives::transaction::{ Action, AddKeyAction, DeployContractAction, FunctionCallAction, Transaction, }; -use near_primitives::types::{AccountId, Balance, Compute, Gas}; +use near_primitives::types::{AccountId, Balance, Gas}; use near_primitives::version::{is_implicit_account_creation_enabled, ProtocolVersion}; /// Describes the cost of converting this transaction into a receipt. @@ -59,10 +59,6 @@ pub fn safe_add_balance(a: Balance, b: Balance) -> Result Result { - a.checked_add(b).ok_or_else(|| IntegerOverflowError {}) -} - #[macro_export] macro_rules! safe_add_balance_apply { ($x: expr) => {$x}; diff --git a/tools/state-viewer/src/contract_accounts.rs b/tools/state-viewer/src/contract_accounts.rs index ab910e09963..cc48ab4919c 100644 --- a/tools/state-viewer/src/contract_accounts.rs +++ b/tools/state-viewer/src/contract_accounts.rs @@ -679,7 +679,6 @@ mod tests { logs: vec![], receipt_ids, gas_burnt: 100, - compute_usage: Some(200), tokens_burnt: 2000, executor_id: "someone.near".parse().unwrap(), status: ExecutionStatus::SuccessValue(vec![]),