Skip to content

Commit

Permalink
refactor: cleaning up RuntimeConfig (near#8100)
Browse files Browse the repository at this point in the history
refactor: cleaning up RuntimeConfig

Sorry. This is a humongous change.
But it's really just a lot of the same boilerplate code changes.

The key changes are:
- Action fees are now accessed through one single function 
  `RuntimeFeesConfig::fee(&self, cost: ActionCosts)` instead of
  selecting a specific field deep down in the structure
- Action fees are stored in an EnumMap instead of by hard-coded fields
- The JSON RPC now has `RuntimeConfigView` to keep compatibility
- Creation of `RuntimeConfig` no longer goes thorough serde JSON
  serialization + deserialization

The goal of this PR is to make gas profiling based on parameters
possible, which in turn makes parameter weights possible to implement
much more easily. The key here is that we no longer have to lookup the
gas value up independently from the profile key.

Today:
```rust
self.gas_counter.pay_action_base(
    &self.fees_config.action_creation_config.deploy_contract_cost,
    sir,
    ActionCosts::deploy_contract_base,
)?;
```
After PR:
```rust
self.pay_action_base(ActionCosts::deploy_contract_base, sir)?;
```

Also it gives us simplified fee lookup on `RuntimeFeesConfig`.
Before, we need things like:
```rust
let exec_gas = self.cfg.action_receipt_creation_config.exec_fee()
    + self.cfg.action_creation_config.add_key_cost.function_call_cost.exec_fee()
    + num_bytes
        * self
            .cfg
            .action_creation_config
            .add_key_cost
            .function_call_cost_per_byte
            .exec_fee();
```

which becomes

```rust
let exec_gas = self.cfg.fee(ActionCosts::new_action_receipt).exec_fee()
    + self.cfg.fee(ActionCosts::add_function_call_key_base).exec_fee()
     + num_bytes * self.cfg.fee(ActionCosts::add_function_call_key_byte).exec_fee();
```

The final state after all refactoring is done is described in near#8033.
This PR contains step 2 and 4 combined because I was not able to
separate them due to unforeseen dependencies we have in the code.

The hardest part should be done after the current PR and what follows
should be small PRs that get rid of types one by one.

# Test
We have to make sure this is a pure refactoring change without
side-effects. Luckily, we have `test_json_unchanged` that detects
changes in any protocol version.
Otherwise, we rely on tests that have gas profile snapshots to make
sure the gas fee lookup translation did not have errors.
  • Loading branch information
jakmeier authored Nov 24, 2022
1 parent 37232db commit 045498b
Show file tree
Hide file tree
Showing 41 changed files with 785 additions and 587 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion chain/indexer/src/streamer/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ pub(crate) async fn convert_transactions_sir_into_local_receipts(
let prev_block = fetch_block(&client, block.header.prev_hash).await?;
let prev_block_gas_price = prev_block.header.gas_price;

let runtime_config =
node_runtime::config::RuntimeConfig::from(protocol_config.runtime_config.clone());
let local_receipts: Vec<views::ReceiptView> =
txs.into_iter()
.map(|tx| {
let cost = tx_cost(
&protocol_config.runtime_config.transaction_costs,
&runtime_config.fees,
&near_primitives::transaction::Transaction {
signer_id: tx.transaction.signer_id.clone(),
public_key: tx.transaction.public_key.clone(),
Expand Down
4 changes: 3 additions & 1 deletion chain/rosetta-rpc/src/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,13 @@ mod tests {
use actix::System;
use near_actix_test_utils::run_actix;
use near_client::test_utils::setup_no_network;
use near_primitives::runtime::config::RuntimeConfig;
use near_primitives::views::RuntimeConfigView;

#[test]
fn test_convert_block_changes_to_transactions() {
run_actix(async {
let runtime_config = near_primitives::runtime::config::RuntimeConfig::test();
let runtime_config: RuntimeConfigView = RuntimeConfig::test().into();
let (_client, view_client) = setup_no_network(
vec!["test".parse().unwrap()],
"other".parse().unwrap(),
Expand Down
6 changes: 3 additions & 3 deletions chain/rosetta-rpc/src/adapters/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ impl<'a> RosettaTransactions<'a> {
/// Returns Rosetta transactions which map to given account changes.
pub(crate) async fn convert_block_changes_to_transactions(
view_client_addr: &Addr<near_client::ViewClientActor>,
runtime_config: &near_primitives::runtime::config::RuntimeConfig,
runtime_config: &near_primitives::views::RuntimeConfigView,
block_hash: &CryptoHash,
accounts_changes: near_primitives::views::StateChangesView,
mut accounts_previous_state: std::collections::HashMap<
Expand Down Expand Up @@ -315,7 +315,7 @@ pub(crate) async fn convert_block_changes_to_transactions(
}

fn convert_account_update_to_operations(
runtime_config: &near_primitives::runtime::config::RuntimeConfig,
runtime_config: &near_primitives::views::RuntimeConfigView,
operations: &mut Vec<crate::models::Operation>,
account_id: &near_primitives::types::AccountId,
previous_account_state: Option<&near_primitives::views::AccountView>,
Expand Down Expand Up @@ -455,7 +455,7 @@ fn convert_account_update_to_operations(
}

fn convert_account_delete_to_operations(
runtime_config: &near_primitives::runtime::config::RuntimeConfig,
runtime_config: &near_primitives::views::RuntimeConfigView,
operations: &mut Vec<crate::models::Operation>,
account_id: &near_primitives::types::AccountId,
previous_account_state: Option<near_primitives::views::AccountView>,
Expand Down
17 changes: 9 additions & 8 deletions chain/rosetta-rpc/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,14 @@ where
}
}

/// Tokens not locked due to staking (=liquid) but reserved for state.
fn get_liquid_balance_for_storage(
mut account: near_primitives::account::Account,
runtime_config: &near_primitives::runtime::config::RuntimeConfig,
account: &near_primitives::account::Account,
storage_amount_per_byte: near_primitives::types::Balance,
) -> near_primitives::types::Balance {
account.set_amount(0);
near_primitives::runtime::get_insufficient_storage_stake(&account, runtime_config)
.expect("get_insufficient_storage_stake never fails when state is consistent")
.unwrap_or(0)
let required_amount =
near_primitives::types::Balance::from(account.storage_usage()) * storage_amount_per_byte;
required_amount.saturating_sub(account.locked())
}

pub(crate) struct RosettaAccountBalances {
Expand All @@ -292,12 +292,13 @@ impl RosettaAccountBalances {

pub fn from_account<T: Into<near_primitives::account::Account>>(
account: T,
runtime_config: &near_primitives::runtime::config::RuntimeConfig,
runtime_config: &near_primitives::views::RuntimeConfigView,
) -> Self {
let account = account.into();
let amount = account.amount();
let locked = account.locked();
let liquid_for_storage = get_liquid_balance_for_storage(account, runtime_config);
let liquid_for_storage =
get_liquid_balance_for_storage(&account, runtime_config.storage_amount_per_byte);

Self { liquid_for_storage, liquid: amount.saturating_sub(liquid_for_storage), locked }
}
Expand Down
6 changes: 4 additions & 2 deletions core/chain-configs/src/genesis_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::{fmt, io};

use anyhow::Context;
use chrono::{DateTime, Utc};
use near_primitives::views::RuntimeConfigView;
use num_rational::Rational32;
use serde::de::{self, DeserializeSeed, IgnoredAny, MapAccess, SeqAccess, Visitor};
use serde::{Deserialize, Deserializer, Serialize};
Expand Down Expand Up @@ -594,6 +595,7 @@ impl GenesisChangeConfig {
// Note: this type cannot be placed in primitives/src/view.rs because of `RuntimeConfig` dependency issues.
// Ideally we should create `RuntimeConfigView`, but given the deeply nested nature and the number of fields inside
// `RuntimeConfig`, it should be its own endeavor.
// TODO: This has changed, there is now `RuntimeConfigView`. Reconsider if moving this is possible now.
#[derive(Serialize, Deserialize, Debug)]
pub struct ProtocolConfigView {
/// Current Protocol Version
Expand Down Expand Up @@ -636,7 +638,7 @@ pub struct ProtocolConfigView {
/// Gas price adjustment rate
pub gas_price_adjustment_rate: Rational32,
/// Runtime configuration (mostly economics constants).
pub runtime_config: RuntimeConfig,
pub runtime_config: RuntimeConfigView,
/// Number of blocks for which a given transaction is valid
pub transaction_validity_period: NumBlocks,
/// Protocol treasury rate
Expand Down Expand Up @@ -683,7 +685,7 @@ impl From<ProtocolConfig> for ProtocolConfigView {
online_min_threshold: genesis_config.online_min_threshold,
online_max_threshold: genesis_config.online_max_threshold,
gas_price_adjustment_rate: genesis_config.gas_price_adjustment_rate,
runtime_config,
runtime_config: RuntimeConfigView::from(runtime_config),
transaction_validity_period: genesis_config.transaction_validity_period,
protocol_reward_rate: genesis_config.protocol_reward_rate,
max_inflation_rate: genesis_config.max_inflation_rate,
Expand Down
1 change: 1 addition & 0 deletions core/primitives-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ base64.workspace = true
borsh.workspace = true
bs58.workspace = true
derive_more.workspace = true
enum-map.workspace = true
num-rational.workspace = true
serde.workspace = true
serde_repr.workspace = true
Expand Down
22 changes: 20 additions & 2 deletions core/primitives-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ use std::collections::hash_map::DefaultHasher;
use std::hash::{Hash, Hasher};
use strum::{Display, EnumCount};

/// Dynamic configuration parameters required for the WASM runtime to
/// execute a smart contract.
///
/// This (`VMConfig`) and `RuntimeFeesConfig` combined are sufficient to define
/// protocol specific behavior of the contract runtime. The former contains
/// configuration for the WASM runtime specifically, while the latter contains
/// configuration for the transaction runtime and WASM runtime.
#[derive(Clone, Debug, Hash, Serialize, Deserialize, PartialEq, Eq)]
pub struct VMConfig {
/// Costs for runtime externals
Expand Down Expand Up @@ -128,7 +135,7 @@ pub enum StackLimiterVersion {
}

impl StackLimiterVersion {
fn v0() -> StackLimiterVersion {
pub fn v0() -> StackLimiterVersion {
StackLimiterVersion::V0
}
}
Expand Down Expand Up @@ -650,7 +657,18 @@ pub enum ExtCosts {

// Type of an action, used in fees logic.
#[derive(
Copy, Clone, Hash, PartialEq, Eq, Debug, PartialOrd, Ord, EnumCount, Display, strum::EnumIter,
Copy,
Clone,
Hash,
PartialEq,
Eq,
Debug,
PartialOrd,
Ord,
EnumCount,
Display,
strum::EnumIter,
enum_map::Enum,
)]
#[allow(non_camel_case_types)]
pub enum ActionCosts {
Expand Down
26 changes: 26 additions & 0 deletions core/primitives-core/src/parameter.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::slice;

use crate::config::ActionCosts;

/// Protocol configuration parameter which may change between protocol versions.
#[derive(
Clone,
Expand Down Expand Up @@ -313,3 +315,27 @@ impl Parameter {
.iter()
}
}

// TODO: consider renaming parameters to "action_{ActionCosts}" and deleting
// `FeeParameter` all together.
impl From<ActionCosts> for FeeParameter {
fn from(other: ActionCosts) -> Self {
match other {
ActionCosts::create_account => Self::ActionCreateAccount,
ActionCosts::delete_account => Self::ActionDeleteAccount,
ActionCosts::deploy_contract_base => Self::ActionDeployContract,
ActionCosts::deploy_contract_byte => Self::ActionDeployContractPerByte,
ActionCosts::function_call_base => Self::ActionFunctionCall,
ActionCosts::function_call_byte => Self::ActionFunctionCallPerByte,
ActionCosts::transfer => Self::ActionTransfer,
ActionCosts::stake => Self::ActionStake,
ActionCosts::add_full_access_key => Self::ActionAddFullAccessKey,
ActionCosts::add_function_call_key_base => Self::ActionAddFunctionCallKey,
ActionCosts::add_function_call_key_byte => Self::ActionAddFunctionCallKeyPerByte,
ActionCosts::delete_key => Self::ActionDeleteKey,
ActionCosts::new_action_receipt => Self::ActionReceiptCreation,
ActionCosts::new_data_receipt_base => Self::DataReceiptCreationBase,
ActionCosts::new_data_receipt_byte => Self::DataReceiptCreationPerByte,
}
}
}
Loading

0 comments on commit 045498b

Please sign in to comment.