Skip to content

Commit

Permalink
Revert "feature: Implement compute usage aggregation and limiting (#8… (
Browse files Browse the repository at this point in the history
#8906)

…805)"

This reverts commit 700ec29.

Canary nodes found an issue with this change:
```
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `2519443987198`,
 right: `2429311750949`: Compute usage must match burnt gas',
```
  • Loading branch information
jakmeier committed Apr 11, 2023
1 parent e32bb3a commit 194312e
Show file tree
Hide file tree
Showing 10 changed files with 31 additions and 222 deletions.
1 change: 0 additions & 1 deletion chain/chain/src/test_utils/kv_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions chain/chain/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
4 changes: 0 additions & 4 deletions core/primitives/src/runtime/parameter_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@ impl TryFrom<&ParameterValue> for ParameterCost {
fn try_from(value: &ParameterValue) -> Result<Self, Self::Error> {
match value {
ParameterValue::ParameterCost { gas, compute } => {
if !cfg!(feature = "protocol_feature_compute_costs") {
assert_eq!(compute, gas, "Compute cost must match gas cost");
}

Ok(ParameterCost { gas: *gas, compute: *compute })
}
// If not specified, compute costs default to gas costs.
Expand Down
12 changes: 1 addition & 11 deletions core/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -416,13 +415,6 @@ pub struct ExecutionOutcome {
pub receipt_ids: Vec<CryptoHash>,
/// 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<Compute>,
/// 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.
Expand All @@ -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),
}

Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 2 additions & 6 deletions runtime/near-vm-logic/src/logic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2794,15 +2794,13 @@ 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,
storage_usage: self.current_storage_usage,
return_data: self.return_data,
burnt_gas,
used_gas,
compute_usage,
logs: self.logs,
profile,
action_receipts: self.receipt_manager.action_receipts,
Expand Down Expand Up @@ -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<String>,
/// Data collected from making a contract call
pub profile: ProfileDataV3,
Expand Down Expand Up @@ -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(),
Expand Down
1 change: 0 additions & 1 deletion runtime/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ near-vm-runner.workspace = true
default = []
dump_errors_schema = ["near-vm-errors/dump_errors_schema"]
protocol_feature_flat_state = ["near-store/protocol_feature_flat_state", "near-vm-logic/protocol_feature_flat_state"]
protocol_feature_compute_costs = ["near-primitives/protocol_feature_compute_costs"]
nightly_protocol = ["near-primitives/nightly_protocol"]
no_cpu_compatibility_checks = ["near-vm-runner/no_cpu_compatibility_checks"]

Expand Down
7 changes: 2 additions & 5 deletions runtime/runtime/src/actions.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
Expand Down
6 changes: 1 addition & 5 deletions runtime/runtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -59,10 +59,6 @@ pub fn safe_add_balance(a: Balance, b: Balance) -> Result<Balance, IntegerOverfl
a.checked_add(b).ok_or_else(|| IntegerOverflowError {})
}

pub fn safe_add_compute(a: Compute, b: Compute) -> Result<Compute, IntegerOverflowError> {
a.checked_add(b).ok_or_else(|| IntegerOverflowError {})
}

#[macro_export]
macro_rules! safe_add_balance_apply {
($x: expr) => {$x};
Expand Down
Loading

0 comments on commit 194312e

Please sign in to comment.