-
Notifications
You must be signed in to change notification settings - Fork 619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(jsonrpc): extract config primitives #7063
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as is, though we should kill old ProtocolConfigView
in this PR or in the immediate follow up.
Also, I believe this is only used in an experimental RPC, so it would makes sense to maybe take advantage of experementality and to re-design the struct such that it's more forward-compatible. I would thing that something like HashMap<String, serde_json::Value>
could probably make more sense as a JSON format.
@@ -583,7 +583,7 @@ impl Genesis { | |||
// 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. | |||
#[derive(Serialize, Deserialize, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we can kill this now completely?
diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs
index ec0975b9d..954b34dbb 100644
--- a/chain/client-primitives/src/types.rs
+++ b/chain/client-primitives/src/types.rs
@@ -6,7 +6,7 @@ use actix::Message;
use chrono::DateTime;
use near_primitives::time::Utc;
-use near_chain_configs::ProtocolConfigView;
+use near_chain_configs::ProtocolConfig;
use near_network_primitives::types::{AccountOrPeerIdOrHash, KnownProducer, PeerInfo};
use near_primitives::errors::InvalidTxError;
use near_primitives::hash::CryptoHash;
@@ -801,7 +801,7 @@ impl Message for GetReceipt {
pub struct GetProtocolConfig(pub BlockReference);
impl Message for GetProtocolConfig {
- type Result = Result<ProtocolConfigView, GetProtocolConfigError>;
+ type Result = Result<ProtocolConfig, GetProtocolConfigError>;
}
#[derive(thiserror::Error, Debug)]
diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs
index ad5a520be..ea065583d 100644
--- a/chain/client/src/view_client.rs
+++ b/chain/client/src/view_client.rs
@@ -17,7 +17,7 @@ use near_chain::{
get_epoch_block_producers_view, Chain, ChainGenesis, ChainStoreAccess, DoomslugThresholdMode,
RuntimeAdapter,
};
-use near_chain_configs::{ClientConfig, ProtocolConfigView};
+use near_chain_configs::{ClientConfig, ProtocolConfig};
use near_client_primitives::types::{
Error, GetBlock, GetBlockError, GetBlockHash, GetBlockProof, GetBlockProofError,
GetBlockProofResponse, GetBlockWithMerkleTree, GetChunkError, GetExecutionOutcome,
@@ -977,7 +977,7 @@ impl Handler<GetBlockProof> for ViewClientActor {
}
impl Handler<GetProtocolConfig> for ViewClientActor {
- type Result = Result<ProtocolConfigView, GetProtocolConfigError>;
+ type Result = Result<ProtocolConfig, GetProtocolConfigError>;
#[perf]
fn handle(&mut self, msg: GetProtocolConfig, _: &mut Self::Context) -> Self::Result {
diff --git a/chain/jsonrpc/src/api/config.rs b/chain/jsonrpc/src/api/config.rs
index f6fd44057..c4240df0e 100644
--- a/chain/jsonrpc/src/api/config.rs
+++ b/chain/jsonrpc/src/api/config.rs
@@ -37,37 +37,47 @@ impl RpcFrom<GetProtocolConfigError> for RpcProtocolConfigError {
}
}
-impl RpcFrom<near_chain_configs::ProtocolConfigView>
+impl RpcFrom<near_chain_configs::ProtocolConfig>
for near_jsonrpc_primitives::types::config::ProtocolConfigView
{
- fn rpc_from(config: near_chain_configs::ProtocolConfigView) -> Self {
+ fn rpc_from(config: near_chain_configs::ProtocolConfig) -> Self {
near_jsonrpc_primitives::types::config::ProtocolConfigView {
- protocol_version: config.protocol_version,
- genesis_time: config.genesis_time,
- chain_id: config.chain_id,
- genesis_height: config.genesis_height,
- num_block_producer_seats: config.num_block_producer_seats,
- num_block_producer_seats_per_shard: config.num_block_producer_seats_per_shard,
- avg_hidden_validator_seats_per_shard: config.avg_hidden_validator_seats_per_shard,
- dynamic_resharding: config.dynamic_resharding,
- protocol_upgrade_stake_threshold: config.protocol_upgrade_stake_threshold,
- epoch_length: config.epoch_length,
- gas_limit: config.gas_limit,
- min_gas_price: config.min_gas_price,
- max_gas_price: config.max_gas_price,
- block_producer_kickout_threshold: config.block_producer_kickout_threshold,
- chunk_producer_kickout_threshold: config.chunk_producer_kickout_threshold,
- online_min_threshold: config.online_min_threshold,
- online_max_threshold: config.online_max_threshold,
- gas_price_adjustment_rate: config.gas_price_adjustment_rate,
+ protocol_version: config.genesis_config.protocol_version,
+ genesis_time: config.genesis_config.genesis_time,
+ chain_id: config.genesis_config.chain_id,
+ genesis_height: config.genesis_config.genesis_height,
+ num_block_producer_seats: config.genesis_config.num_block_producer_seats,
+ num_block_producer_seats_per_shard: config
+ .genesis_config
+ .num_block_producer_seats_per_shard,
+ avg_hidden_validator_seats_per_shard: config
+ .genesis_config
+ .avg_hidden_validator_seats_per_shard,
+ dynamic_resharding: config.genesis_config.dynamic_resharding,
+ protocol_upgrade_stake_threshold: config
+ .genesis_config
+ .protocol_upgrade_stake_threshold,
+ epoch_length: config.genesis_config.epoch_length,
+ gas_limit: config.genesis_config.gas_limit,
+ min_gas_price: config.genesis_config.min_gas_price,
+ max_gas_price: config.genesis_config.max_gas_price,
+ block_producer_kickout_threshold: config
+ .genesis_config
+ .block_producer_kickout_threshold,
+ chunk_producer_kickout_threshold: config
+ .genesis_config
+ .chunk_producer_kickout_threshold,
+ online_min_threshold: config.genesis_config.online_min_threshold,
+ online_max_threshold: config.genesis_config.online_max_threshold,
+ gas_price_adjustment_rate: config.genesis_config.gas_price_adjustment_rate,
runtime_config: config.runtime_config.rpc_into(),
- transaction_validity_period: config.transaction_validity_period,
- protocol_reward_rate: config.protocol_reward_rate,
- max_inflation_rate: config.max_inflation_rate,
- num_blocks_per_year: config.num_blocks_per_year,
- protocol_treasury_account: config.protocol_treasury_account,
- fishermen_threshold: config.fishermen_threshold,
- minimum_stake_divisor: config.minimum_stake_divisor,
+ transaction_validity_period: config.genesis_config.transaction_validity_period,
+ protocol_reward_rate: config.genesis_config.protocol_reward_rate,
+ max_inflation_rate: config.genesis_config.max_inflation_rate,
+ num_blocks_per_year: config.genesis_config.num_blocks_per_year,
+ protocol_treasury_account: config.genesis_config.protocol_treasury_account,
+ fishermen_threshold: config.genesis_config.fishermen_threshold,
+ minimum_stake_divisor: config.genesis_config.minimum_stake_divisor,
}
}
}
diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs
index 324a5a28a..a26ef9223 100644
--- a/chain/rosetta-rpc/src/utils.rs
+++ b/chain/rosetta-rpc/src/utils.rs
@@ -1,7 +1,7 @@
use actix::Addr;
use futures::StreamExt;
-use near_chain_configs::ProtocolConfigView;
+use near_chain_configs::ProtocolConfig;
use near_client::ViewClientActor;
use near_primitives::borsh::{BorshDeserialize, BorshSerialize};
@@ -412,7 +412,7 @@ pub(crate) async fn query_access_key(
pub(crate) async fn query_protocol_config(
block_hash: near_primitives::hash::CryptoHash,
view_client_addr: &Addr<ViewClientActor>,
-) -> crate::errors::Result<ProtocolConfigView> {
+) -> crate::errors::Result<ProtocolConfig> {
view_client_addr
.send(near_client::GetProtocolConfig(near_primitives::types::BlockReference::from(
near_primitives::types::BlockId::Hash(block_hash),
diff --git a/core/chain-configs/src/genesis_config.rs b/core/chain-configs/src/genesis_config.rs
index 13299cf4a..c946b9629 100644
--- a/core/chain-configs/src/genesis_config.rs
+++ b/core/chain-configs/src/genesis_config.rs
@@ -580,107 +580,11 @@ impl Genesis {
}
}
-// 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.
-#[derive(Debug)]
-pub struct ProtocolConfigView {
- /// Current Protocol Version
- pub protocol_version: ProtocolVersion,
- /// Official time of blockchain start.
- pub genesis_time: DateTime<Utc>,
- /// ID of the blockchain. This must be unique for every blockchain.
- /// If your testnet blockchains do not have unique chain IDs, you will have a bad time.
- pub chain_id: String,
- /// Height of genesis block.
- pub genesis_height: BlockHeight,
- /// Number of block producer seats at genesis.
- pub num_block_producer_seats: NumSeats,
- /// Defines number of shards and number of block producer seats per each shard at genesis.
- pub num_block_producer_seats_per_shard: Vec<NumSeats>,
- /// Expected number of hidden validators per shard.
- pub avg_hidden_validator_seats_per_shard: Vec<NumSeats>,
- /// Enable dynamic re-sharding.
- pub dynamic_resharding: bool,
- /// Threshold of stake that needs to indicate that they ready for upgrade.
- pub protocol_upgrade_stake_threshold: Rational,
- /// Epoch length counted in block heights.
- pub epoch_length: BlockHeightDelta,
- /// Initial gas limit.
- pub gas_limit: Gas,
- /// Minimum gas price. It is also the initial gas price.
- pub min_gas_price: Balance,
- /// Maximum gas price.
- pub max_gas_price: Balance,
- /// Criterion for kicking out block producers (this is a number between 0 and 100)
- pub block_producer_kickout_threshold: u8,
- /// Criterion for kicking out chunk producers (this is a number between 0 and 100)
- pub chunk_producer_kickout_threshold: u8,
- /// Online minimum threshold below which validator doesn't receive reward.
- pub online_min_threshold: Rational,
- /// Online maximum threshold above which validator gets full reward.
- pub online_max_threshold: Rational,
- /// Gas price adjustment rate
- pub gas_price_adjustment_rate: Rational,
- /// Runtime configuration (mostly economics constants).
- pub runtime_config: RuntimeConfig,
- /// Number of blocks for which a given transaction is valid
- pub transaction_validity_period: NumBlocks,
- /// Protocol treasury rate
- pub protocol_reward_rate: Rational,
- /// Maximum inflation on the total supply every epoch.
- pub max_inflation_rate: Rational,
- /// Expected number of blocks per year
- pub num_blocks_per_year: NumBlocks,
- /// Protocol treasury account
- pub protocol_treasury_account: AccountId,
- /// Fishermen stake threshold.
- pub fishermen_threshold: Balance,
- /// The minimum stake required for staking is last seat price divided by this number.
- pub minimum_stake_divisor: u64,
-}
-
pub struct ProtocolConfig {
pub genesis_config: GenesisConfig,
pub runtime_config: RuntimeConfig,
}
-impl From<ProtocolConfig> for ProtocolConfigView {
- fn from(protocol_config: ProtocolConfig) -> Self {
- let ProtocolConfig { genesis_config, runtime_config } = protocol_config;
-
- ProtocolConfigView {
- protocol_version: genesis_config.protocol_version,
- genesis_time: genesis_config.genesis_time,
- chain_id: genesis_config.chain_id,
- genesis_height: genesis_config.genesis_height,
- num_block_producer_seats: genesis_config.num_block_producer_seats,
- num_block_producer_seats_per_shard: genesis_config.num_block_producer_seats_per_shard,
- avg_hidden_validator_seats_per_shard: genesis_config
- .avg_hidden_validator_seats_per_shard,
- dynamic_resharding: genesis_config.dynamic_resharding,
- protocol_upgrade_stake_threshold: genesis_config.protocol_upgrade_stake_threshold,
- epoch_length: genesis_config.epoch_length,
- gas_limit: genesis_config.gas_limit,
- min_gas_price: genesis_config.min_gas_price,
- max_gas_price: genesis_config.max_gas_price,
- block_producer_kickout_threshold: genesis_config.block_producer_kickout_threshold,
- chunk_producer_kickout_threshold: genesis_config.chunk_producer_kickout_threshold,
- 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,
- transaction_validity_period: genesis_config.transaction_validity_period,
- protocol_reward_rate: genesis_config.protocol_reward_rate,
- max_inflation_rate: genesis_config.max_inflation_rate,
- num_blocks_per_year: genesis_config.num_blocks_per_year,
- protocol_treasury_account: genesis_config.protocol_treasury_account,
- fishermen_threshold: genesis_config.fishermen_threshold,
- minimum_stake_divisor: genesis_config.minimum_stake_divisor,
- }
- }
-}
-
pub fn get_initial_supply(records: &[StateRecord]) -> Balance {
let mut total_supply = 0;
for record in records {
diff --git a/core/chain-configs/src/lib.rs b/core/chain-configs/src/lib.rs
index e62d53f38..3ab57b8a2 100644
--- a/core/chain-configs/src/lib.rs
+++ b/core/chain-configs/src/lib.rs
@@ -8,5 +8,5 @@ pub use client_config::{
};
pub use genesis_config::{
get_initial_supply, Genesis, GenesisConfig, GenesisRecords, GenesisValidationMode,
- ProtocolConfig, ProtocolConfigView,
+ ProtocolConfig,
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*View structs served as an API layer for the view-client. I am not sure whether it is still worth to keep the view-client API stable, but I would decide about it in a separate PR that is outside of JSON RPC scope.
near-primitives = { path = "../../core/primitives" } | ||
near-chain-configs = { path = "../../core/chain-configs" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
@@ -10,7 +16,7 @@ pub struct RpcProtocolConfigRequest { | |||
#[derive(Serialize, Deserialize, Debug)] | |||
pub struct RpcProtocolConfigResponse { | |||
#[serde(flatten)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are #[serde(flatten)]
this anyway,do we need a separate ProtocolConfigView
struct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought about that, but thought I'd retain an interface downstream users were already familiar with. But you're right. Since we're redesigning the crate anyway, might as well go all the way.
To clarify, it would make sense to do that once we have time to do nice things, ha-ha |
#[serde(default = "StackLimiterVersion::v0")] | ||
pub stack_limiter_version: StackLimiterVersion, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this can be just an Option
? For all new fields we add in the future, I think they will be options in this struct, but not in the actual config. And if we are doing that for new fields, might as well do for fields which we added before?
Note to self: after this is merged:
|
This PR has been automatically marked as stale because it has not had recent activity in the 2 weeks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely! ❤️
@@ -583,7 +583,7 @@ impl Genesis { | |||
// 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. | |||
#[derive(Serialize, Deserialize, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*View structs served as an API layer for the view-client. I am not sure whether it is still worth to keep the view-client API stable, but I would decide about it in a separate PR that is outside of JSON RPC scope.
I believe this PR is no longer actively being worked on. |
Tracking issue: #6850
Drops the dependency requirement of
near-chain-configs
by duplicating all its structures withinnear-jsonrpc-primitives
.