[tracking] RPC refactoring wishlist

[aatifsyed]

The motivating use-case is #4038. While we're in the area we can do a lot of cleanup.

• 4067

• 4081

• 4007

• 4134

• 4172

• [x] Introduce crate::rpc::Client, remove crate::rpc_client::ApiInfo.

  • 4140

  • 4280

• 4231

• 4234

• [x] Migrate handlers to trait RpcMethod.
  • Migration guide for RPC call sites is as follows. Deviations from this SHOULD be checked with me, and please add me as a reviewer to your PR
  • Use the snippets file to scaffold your implementation
    • Make sure the method name matches!
    • You may need to narrow the bounds on Ctx
  • Ensure rpc::types are minimal - any structs used by only one e.g gas API should live in gas.rs, not in types.rs. types.rs is only for types that are shared across APIs
  • Migrate the parameters, dropping LotusJson where possible.
  • Implement JsonSchema where necessary.
  • Add or amend the for_each_method macro in gas.rs.
    • It should be called to expose methods in the prelude
    • It should be called to register methods
  • RpcTest constructors should use RpcTest::basic_raw, and RpcMethodExt::request like this.
  • forest-cli calls should rpc::Client::from(api: ApiInfo) and then RpcMethodExt::call on that. In the limit, ApiInfo can be removed, and replaced by an rpc::Client::from_env method
  • forest-cli attach calls should be removed, and tracked under #4141
  • Progress:

  • 4139

  • 4154

  • 4155

  • 4158

  • 4168

  • 4175

  • 4187

  • 4191

  • 4200

  • 4203

  • 4204

  • 4225

  • 4244

  • 4248

• 4141 / #4358

• [x] Support aliases: https://github.com/ChainSafe/forest/pull/4155#discussion_r1556026830
• [ ] crate::rpc_client::RpcRequest::params should be Vec<Value> or RequestParameters, and should have better error handling.
• [ ] Remove inappropriate impls of HasLotusJson
  • [ ] Tuples should NOT implement HasLotusJson
  • [ ] Most types should NOT implement HasLotusJson
  • https://github.com/ChainSafe/forest/blob/f58b629c79cb26a67bfc48f1ccbc07e2e86e1c38/src/rpc/mpool_api.rs#L136 Note that only the first parameter is wrapped in LotusJson - it has separate CBOR and JSON representations. MessageSendSpec, on the other hand, can just be deserialized normally. A misapplication of the HasLotusJson API has lead to the proliferation of lotus_json_with_self! calls
  • [ ] LotusJson<Vec<T>> should NOT recurse: users should use LotusJson<Vec<LotusJson<T>>
  • [ ] LotusJson<ApiTipsetKey> is wrong

• 4277

• [ ] Reconsider const generic on trait RpcMethod
• [x] Make type names in OpenRPC document nice.
• [ ] Unconsolidate crate::rpc::types
• [x] Autogenerate CLI, maybe JS bindings

  • 4295

• [ ] Autogenerate implementation report as discussed in #4129

[aatifsyed]

Code snippets file for VSCode:

// This should live somewhere like `${workspaceDir}/.vscode/rpc.code-snippets`
{
    "rpc method": {
        "prefix": "impl RpcMethod",
        "body": [
            "pub enum ${1:MethodName} {}",
            "impl RpcMethod<${2:ARITY}> for $1 {",
            "const NAME: &'static str = \"Filecoin.$1\";",
            "const PARAM_NAMES: [&'static str; $2] = [$3];",
            "const API_VERSION: ApiVersion = ApiVersion::${7:V0};",
            "",
            "type Params = ($4);",
            "type Ok = $6;",
            "",
            "async fn handle(ctx: Ctx<impl Blockstore $8>, ($5): Self::Params) -> Result<Self::Ok, ServerError> {",
            "$0",
            "}", // fn handle
            "}", // impl RpcMethod
        ]
    },
    "imports": {
        "prefix": "use",
        "body": [
            "use crate::rpc::{reflect::SelfDescribingRpcModule, ApiVersion, Ctx, RPCState, RpcMethod, RpcMethodExt as _, ServerError};",
            "use schemars::JsonSchema;"
        ]
    },
}