From c46d82af2c14198fde9087f5dac1b3cd7f906a3b Mon Sep 17 00:00:00 2001 From: Aatif Syed <38045910+aatifsyed@users.noreply.github.com> Date: Mon, 8 Apr 2024 16:20:50 +0100 Subject: [PATCH] refactor: migrate `beacon_api` to use `trait RpcMethod` (#4155) --- src/lotus_json/mod.rs | 4 +- src/rpc/auth_layer.rs | 2 +- src/rpc/beacon_api.rs | 41 ++++--- src/rpc/mod.rs | 4 +- src/rpc/reflect/mod.rs | 2 +- .../forest_filecoin__rpc__tests__openrpc.snap | 21 ++++ src/rpc_client/beacon_ops.rs | 12 -- src/rpc_client/mod.rs | 9 +- src/tool/subcommands/api_cmd.rs | 113 ++++++++---------- 9 files changed, 109 insertions(+), 99 deletions(-) delete mode 100644 src/rpc_client/beacon_ops.rs diff --git a/src/lotus_json/mod.rs b/src/lotus_json/mod.rs index 3a383bd4f6d1..7edf4839183f 100644 --- a/src/lotus_json/mod.rs +++ b/src/lotus_json/mod.rs @@ -392,7 +392,9 @@ where } /// A domain struct that is (de) serialized through its lotus JSON representation. -#[derive(Debug, Serialize, Deserialize, From, Default, Clone)] +#[derive( + Debug, Serialize, Deserialize, From, Default, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, +)] #[serde(bound = "T: HasLotusJson + Clone")] pub struct LotusJson(#[serde(with = "self")] pub T); diff --git a/src/rpc/auth_layer.rs b/src/rpc/auth_layer.rs index 9bfcec672d60..4a0fa5403e4d 100644 --- a/src/rpc/auth_layer.rs +++ b/src/rpc/auth_layer.rs @@ -39,7 +39,7 @@ static ACCESS_MAP: Lazy> = Lazy::new(|| { access.insert(auth_api::AuthVerify::NAME, Access::Read); // Beacon API - access.insert(beacon_api::BEACON_GET_ENTRY, Access::Read); + access.insert(beacon_api::BeaconGetEntry::NAME, Access::Read); // Chain API access.insert(chain_api::CHAIN_GET_MESSAGE, Access::Read); diff --git a/src/rpc/beacon_api.rs b/src/rpc/beacon_api.rs index 177889cafb01..9dd836c696dc 100644 --- a/src/rpc/beacon_api.rs +++ b/src/rpc/beacon_api.rs @@ -1,27 +1,38 @@ // Copyright 2019-2024 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -use crate::rpc::error::JsonRpcError; -use crate::rpc::Ctx; +use crate::rpc::{ + reflect::SelfDescribingRpcModule, ApiVersion, Ctx, JsonRpcError, RPCState, RpcMethod, + RpcMethodExt as _, +}; use crate::{beacon::BeaconEntry, lotus_json::LotusJson, shim::clock::ChainEpoch}; use anyhow::Result; use fvm_ipld_blockstore::Blockstore; -use jsonrpsee::types::Params; -pub const BEACON_GET_ENTRY: &str = "Filecoin.BeaconGetEntry"; +pub fn register_all( + module: &mut SelfDescribingRpcModule>, +) { + BeaconGetEntry::register(module); +} /// `BeaconGetEntry` returns the beacon entry for the given Filecoin epoch. If /// the entry has not yet been produced, the call will block until the entry /// becomes available -pub async fn beacon_get_entry( - params: Params<'_>, - data: Ctx, -) -> Result, JsonRpcError> { - let (first,): (ChainEpoch,) = params.parse()?; - - let (_, beacon) = data.beacon.beacon_for_epoch(first)?; - let rr = - beacon.max_beacon_round_for_epoch(data.state_manager.get_network_version(first), first); - let e = beacon.entry(rr).await?; - Ok(e.into()) +pub enum BeaconGetEntry {} +impl RpcMethod<1> for BeaconGetEntry { + const NAME: &'static str = "Filecoin.BeaconGetEntry"; + const PARAM_NAMES: [&'static str; 1] = ["first"]; + const API_VERSION: ApiVersion = ApiVersion::V0; + type Params = (ChainEpoch,); + type Ok = LotusJson; + async fn handle( + ctx: Ctx, + (first,): Self::Params, + ) -> Result { + let (_, beacon) = ctx.beacon.beacon_for_epoch(first)?; + let rr = + beacon.max_beacon_round_for_epoch(ctx.state_manager.get_network_version(first), first); + let e = beacon.entry(rr).await?; + Ok(e.into()) + } } diff --git a/src/rpc/mod.rs b/src/rpc/mod.rs index d3e20e5df2be..9db12a2a3fd9 100644 --- a/src/rpc/mod.rs +++ b/src/rpc/mod.rs @@ -168,6 +168,7 @@ where ChainGetPath::register(&mut module); mpool_api::register_all(&mut module); auth_api::register_all(&mut module); + beacon_api::register_all(&mut module); module.finish() } @@ -181,7 +182,6 @@ fn register_methods( where DB: Blockstore + Send + Sync + 'static, { - use beacon_api::*; use chain_api::*; use common_api::*; use eth_api::*; @@ -191,8 +191,6 @@ where use sync_api::*; use wallet_api::*; - // Beacon API - module.register_async_method(BEACON_GET_ENTRY, beacon_get_entry::)?; // Chain API module.register_async_method(CHAIN_GET_MESSAGE, chain_get_message::)?; module.register_async_method(CHAIN_EXPORT, chain_export::)?; diff --git a/src/rpc/reflect/mod.rs b/src/rpc/reflect/mod.rs index 5e20b4554749..8f41ab3d5fbf 100644 --- a/src/rpc/reflect/mod.rs +++ b/src/rpc/reflect/mod.rs @@ -221,7 +221,7 @@ pub trait RpcMethodExt: RpcMethod { // TODO(aatifsyed): https://github.com/ChainSafe/forest/issues/4032 // Client::call has an inappropriate HasLotusJson // bound, work around it for now. - let json = client.call(Self::request(params)?.lower()).await?; + let json = client.call(Self::request(params)?.map_ty()).await?; Ok(serde_json::from_value(json)?) } } diff --git a/src/rpc/snapshots/forest_filecoin__rpc__tests__openrpc.snap b/src/rpc/snapshots/forest_filecoin__rpc__tests__openrpc.snap index df4c66175f2c..3e4fd3927857 100644 --- a/src/rpc/snapshots/forest_filecoin__rpc__tests__openrpc.snap +++ b/src/rpc/snapshots/forest_filecoin__rpc__tests__openrpc.snap @@ -179,6 +179,27 @@ methods: items: type: string required: true + - name: Filecoin.BeaconGetEntry + params: + - name: first + schema: + type: integer + format: int64 + required: true + paramStructure: by-position + result: + name: "Filecoin.BeaconGetEntry::Result" + schema: + type: object + required: + - Data + - Round + properties: + Data: + $ref: "#/components/schemas/VecU8LotusJson2" + Round: + $ref: "#/components/schemas/uint64" + required: true components: schemas: CidLotusJsonGeneric_for_64: diff --git a/src/rpc_client/beacon_ops.rs b/src/rpc_client/beacon_ops.rs deleted file mode 100644 index c090ca18719a..000000000000 --- a/src/rpc_client/beacon_ops.rs +++ /dev/null @@ -1,12 +0,0 @@ -// Copyright 2019-2024 ChainSafe Systems -// SPDX-License-Identifier: Apache-2.0, MIT - -use super::{ApiInfo, RpcRequest}; -use crate::beacon::beacon_entries::BeaconEntry; -use crate::rpc::beacon_api::*; - -impl ApiInfo { - pub fn beacon_get_entry_req(first: i64) -> RpcRequest { - RpcRequest::new(BEACON_GET_ENTRY, (first,)) - } -} diff --git a/src/rpc_client/mod.rs b/src/rpc_client/mod.rs index 37c378caecbc..a9bbffc24e24 100644 --- a/src/rpc_client/mod.rs +++ b/src/rpc_client/mod.rs @@ -1,7 +1,6 @@ // Copyright 2019-2024 ChainSafe Systems // SPDX-License-Identifier: Apache-2.0, MIT -pub mod beacon_ops; pub mod chain_ops; pub mod common_ops; pub mod eth_ops; @@ -170,8 +169,8 @@ impl RpcRequest { self } - // Discard type information about the response. - pub fn lower(self) -> RpcRequest { + /// Map type information about the response. + pub fn map_ty(self) -> RpcRequest { RpcRequest { method_name: self.method_name, params: self.params, @@ -180,6 +179,10 @@ impl RpcRequest { timeout: self.timeout, } } + /// Discard type information about the response. + pub fn lower(self) -> RpcRequest { + self.map_ty() + } } impl ToRpcParams for RpcRequest { diff --git a/src/tool/subcommands/api_cmd.rs b/src/tool/subcommands/api_cmd.rs index 07050ce2e527..551fbeb554fd 100644 --- a/src/tool/subcommands/api_cmd.rs +++ b/src/tool/subcommands/api_cmd.rs @@ -13,6 +13,7 @@ use crate::lotus_json::{HasLotusJson, LotusJson}; use crate::message::Message as _; use crate::message_pool::{MessagePool, MpoolRpcProvider}; use crate::networks::{parse_bootstrap_peers, ChainConfig, NetworkChain}; +use crate::rpc::beacon_api::BeaconGetEntry; use crate::rpc::eth_api::Address as EthAddress; use crate::rpc::eth_api::*; use crate::rpc::types::{ApiTipsetKey, MessageFilter, MessageLookup}; @@ -212,36 +213,22 @@ struct RpcTest { ignore: Option<&'static str>, } +/// Duplication between `` and `_raw` is a temporary measure, and +/// should be removed when is +/// completed. impl RpcTest { - // Check that an endpoint exist and that both the Lotus and Forest JSON - // response follows the same schema. - fn basic(request: RpcRequest) -> RpcTest + /// Check that an endpoint exists and that both the Lotus and Forest JSON + /// response follows the same schema. + fn basic(request: RpcRequest) -> Self where T: HasLotusJson, { - RpcTest { - request: request.lower(), - check_syntax: Arc::new( - |value| match serde_json::from_value::(value) { - Ok(_) => true, - Err(e) => { - debug!("{e}"); - false - } - }, - ), - check_semantics: Arc::new(|_, _| true), - ignore: None, - } + Self::basic_raw(request.map_ty::()) } - /// This is [`Self::basic`] without the [`HasLotusJson`] bound, for methods - /// where the return type does not need converting via a third type. - /// - /// This is a temporary measure, and should be removed when - /// is completed. + /// See [Self::basic], and note on this `impl` block. fn basic_raw(request: RpcRequest) -> Self { Self { - request: request.lower(), + request: request.map_ty(), check_syntax: Arc::new(|it| match serde_json::from_value::(it) { Ok(_) => true, Err(e) => { @@ -253,37 +240,36 @@ impl RpcTest { ignore: None, } } - - // Check that an endpoint exist, has the same JSON schema, and do custom - // validation over both responses. - fn validate( + /// Check that an endpoint exists, has the same JSON schema, and do custom + /// validation over both responses. + fn validate( request: RpcRequest, validate: impl Fn(T, T) -> bool + Send + Sync + 'static, - ) -> RpcTest - where - T: HasLotusJson, - T::LotusJson: DeserializeOwned, - { - RpcTest { - request: request.lower(), - check_syntax: Arc::new( - |value| match serde_json::from_value::(value) { - Ok(_) => true, - Err(e) => { - debug!("{e}"); - false - } - }, - ), + ) -> Self { + Self::validate_raw(request.map_ty::(), move |l, r| { + validate(T::from_lotus_json(l), T::from_lotus_json(r)) + }) + } + /// See [Self::validate], and note on this `impl` block. + fn validate_raw( + request: RpcRequest, + validate: impl Fn(T, T) -> bool + Send + Sync + 'static, + ) -> Self { + Self { + request: request.map_ty(), + check_syntax: Arc::new(|value| match serde_json::from_value::(value) { + Ok(_) => true, + Err(e) => { + debug!("{e}"); + false + } + }), check_semantics: Arc::new(move |forest_json, lotus_json| { match ( - serde_json::from_value::(forest_json), - serde_json::from_value::(lotus_json), + serde_json::from_value::(forest_json), + serde_json::from_value::(lotus_json), ) { - (Ok(forest), Ok(lotus)) => validate( - HasLotusJson::from_lotus_json(forest), - HasLotusJson::from_lotus_json(lotus), - ), + (Ok(forest), Ok(lotus)) => validate(forest, lotus), (forest, lotus) => { if let Err(e) = forest { debug!("[forest] invalid json: {e}"); @@ -298,20 +284,14 @@ impl RpcTest { ignore: None, } } - - fn ignore(mut self, msg: &'static str) -> Self { - self.ignore = Some(msg); - self + /// Check that an endpoint exists and that Forest returns exactly the same + /// JSON as Lotus. + fn identity(request: RpcRequest) -> RpcTest { + Self::validate(request, |forest, lotus| forest == lotus) } - - // Check that an endpoint exist and that Forest returns exactly the same - // JSON as Lotus. - fn identity(request: RpcRequest) -> RpcTest - where - T: HasLotusJson, - T::LotusJson: DeserializeOwned, - { - RpcTest::validate(request, |forest, lotus| forest == lotus) + /// See [Self::identity], and note on this `impl` block. + fn identity_raw(request: RpcRequest) -> Self { + Self::validate_raw(request, |l, r| l == r) } fn with_timeout(mut self, timeout: Duration) -> Self { @@ -319,6 +299,11 @@ impl RpcTest { self } + fn ignore(mut self, msg: &'static str) -> Self { + self.ignore = Some(msg); + self + } + async fn run( &self, forest_api: &ApiInfo, @@ -382,7 +367,9 @@ fn auth_tests() -> Vec { } fn beacon_tests() -> Vec { - vec![RpcTest::identity(ApiInfo::beacon_get_entry_req(10101))] + vec![RpcTest::identity_raw( + BeaconGetEntry::request((10101,)).unwrap(), + )] } fn chain_tests() -> Vec {