Skip to content

Commit

Permalink
feature: Re-introduce Compute Costs (#8915)
Browse files Browse the repository at this point in the history
This PR re-introduces changes from #8805 together with #8892 after fixing #8908:

The PR addresses #8265 by introducing aggregation of compute usage across all operations performed during chunk application and limiting this compute usage to 1s.

This should not change the behavior of nodes in the short run because compute costs match gas costs which is validated by the assert, so any discrepancy should be caught on canary nodes.
  • Loading branch information
aborg-dev authored and nikurt committed Apr 25, 2023
1 parent 7d93098 commit 072a32b
Show file tree
Hide file tree
Showing 12 changed files with 272 additions and 46 deletions.
1 change: 1 addition & 0 deletions chain/chain/src/test_utils/kv_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,7 @@ 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
4 changes: 2 additions & 2 deletions chain/chain/src/tests/simple_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ fn build_chain() {
if cfg!(feature = "nightly") {
insta::assert_display_snapshot!(hash, @"3Dkg6hjpnYvMuoyEdSLnEXza6Ct2ZV9xoridA37AJzSz");
} else {
insta::assert_display_snapshot!(hash, @"3fK7Uu3HC9y9DPMsDNaAP8sv56UCK2ZV1txRhTriF9qb");
insta::assert_display_snapshot!(hash, @"DuT1f8dmu3xYvFfsEFiAycgeLpJWQM74PZ8JtjT7SGyK");
}

for i in 1..5 {
Expand Down Expand Up @@ -75,7 +75,7 @@ fn build_chain() {
if cfg!(feature = "nightly") {
insta::assert_display_snapshot!(hash, @"6uCZwfkpE8qV54n5MvZXqTt8RMHYDduX4eE7quNzgLNk");
} else {
insta::assert_display_snapshot!(hash, @"7hQ4bvvu2TmgFVhELFThZPHFoBPwa74VT6D8uk79xPzB");
insta::assert_display_snapshot!(hash, @"Ce8Ehs6S2RmXUdp2WnuyQFBGs4S3Pvs5sBwuSMZW7pqS");
}
}

Expand Down
2 changes: 2 additions & 0 deletions chain/chain/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ 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 @@ -643,6 +644,7 @@ 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: 2 additions & 2 deletions chain/jsonrpc/jsonrpc-tests/res/genesis_config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"protocol_version": 60,
"protocol_version": 61,
"genesis_time": "1970-01-01T00:00:00.000000000Z",
"chain_id": "sample",
"genesis_height": 0,
Expand Down Expand Up @@ -69,4 +69,4 @@
],
"use_production_config": false,
"records": []
}
}
2 changes: 0 additions & 2 deletions core/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,12 @@ protocol_feature_fix_staking_threshold = []
protocol_feature_fix_contract_loading_cost = []
protocol_feature_reject_blocks_with_outdated_protocol_version = []
protocol_feature_flat_state = []
protocol_feature_compute_costs = []
nightly = [
"nightly_protocol",
"protocol_feature_fix_staking_threshold",
"protocol_feature_fix_contract_loading_cost",
"protocol_feature_reject_blocks_with_outdated_protocol_version",
"protocol_feature_flat_state",
"protocol_feature_compute_costs",
]

nightly_protocol = []
Expand Down
12 changes: 11 additions & 1 deletion core/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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 @@ -415,6 +416,13 @@ 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 @@ -437,7 +445,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 @@ -453,6 +461,7 @@ 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 @@ -598,6 +607,7 @@ 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
13 changes: 8 additions & 5 deletions core/primitives/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ pub enum ProtocolFeature {
/// Meta Transaction NEP-366: https://github.com/near/NEPs/blob/master/neps/nep-0366.md
DelegateAction,

/// Decouple compute and gas costs of operations to safely limit the compute time it takes to
/// process the chunk.
///
/// Compute Costs NEP-455: https://github.com/near/NEPs/blob/master/neps/nep-0455.md
ComputeCosts,

/// In case not all validator seats are occupied our algorithm provide incorrect minimal seat
/// price - it reports as alpha * sum_stake instead of alpha * sum_stake / (1 - alpha), where
/// alpha is min stake ratio
Expand All @@ -152,8 +158,6 @@ pub enum ProtocolFeature {
RejectBlocksWithOutdatedProtocolVersions,
#[cfg(feature = "protocol_feature_flat_state")]
FlatStorageReads,
#[cfg(feature = "protocol_feature_compute_costs")]
ComputeCosts,
}

/// Both, outgoing and incoming tcp connections to peers, will be rejected if `peer's`
Expand All @@ -163,7 +167,7 @@ pub const PEER_MIN_ALLOWED_PROTOCOL_VERSION: ProtocolVersion = STABLE_PROTOCOL_V
/// Current protocol version used on the mainnet.
/// Some features (e. g. FixStorageUsage) require that there is at least one epoch with exactly
/// the corresponding version
const STABLE_PROTOCOL_VERSION: ProtocolVersion = 60;
const STABLE_PROTOCOL_VERSION: ProtocolVersion = 61;

/// Largest protocol version supported by the current binary.
pub const PROTOCOL_VERSION: ProtocolVersion = if cfg!(feature = "nightly_protocol") {
Expand Down Expand Up @@ -238,6 +242,7 @@ impl ProtocolFeature {
ProtocolFeature::Ed25519Verify
| ProtocolFeature::ZeroBalanceAccount
| ProtocolFeature::DelegateAction => 59,
ProtocolFeature::ComputeCosts => 61,

// Nightly features
#[cfg(feature = "protocol_feature_fix_staking_threshold")]
Expand All @@ -248,8 +253,6 @@ impl ProtocolFeature {
ProtocolFeature::RejectBlocksWithOutdatedProtocolVersions => 132,
#[cfg(feature = "protocol_feature_flat_state")]
ProtocolFeature::FlatStorageReads => 135,
#[cfg(feature = "protocol_feature_compute_costs")]
ProtocolFeature::ComputeCosts => 136,
}
}
}
Expand Down
8 changes: 6 additions & 2 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, EpochHeight, Gas, ProtocolVersion, StorageUsage,
AccountId, Balance, Compute, EpochHeight, Gas, GasDistribution, GasWeight, 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,13 +2794,15 @@ 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 @@ -2919,6 +2921,7 @@ 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 @@ -2952,6 +2955,7 @@ 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
7 changes: 5 additions & 2 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_gas, total_prepaid_exec_fees, total_prepaid_gas, total_prepaid_send_fees,
RuntimeConfig,
safe_add_compute, 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,6 +254,7 @@ 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 @@ -687,6 +688,8 @@ 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: 5 additions & 1 deletion 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, Gas};
use near_primitives::types::{AccountId, Balance, Compute, 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,6 +59,10 @@ 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 072a32b

Please sign in to comment.