Reorg rpc (no logic changes)

[koushiro]

Description

This is a big PR without any logical changes about rpc.

I think the big file module is hard to maintain and read, so I made the following changes:

• Split EthApi into multiple files
  • Block
  • State
  • Execute
  • Fee
  • Mining
  • Block
  • Transaction
  • Submit
• Move eth_getLogs method to EthFilterApi
• Move EthBlockDataCache and EthTask into eth/cache module.
• Move EthSigner and EthDevSigner into signer module.

Waiting #593 to be merged firstly

[koushiro]

@sorpaas PTAL

[koushiro]

@sorpaas could you give me some feedback?

[sorpaas]

Sorry about my review order, but can you merge master? (Should be a simple one just due to eth_maxPriorityFeeperGas.)

[koushiro]

@sorpaas PTAL

[sorpaas]

I like the improvements of code clarity where we split eth_ implementations into multiple files, but honestly, I don't see much reasons why we need to split the traits as well, which I hope you can explain more. Plus the later will add a lot of API changes to downstream users that they need to adopt.

Alternatively, my recommendation is that we can revert the trait changes, but keep the impl file split structure (you can split a single structs impl into multiple files).

[koushiro]

I don't see much reasons why we need to split the traits as well, which I hope you can explain more. Plus the later will add a lot of API changes to downstream users that they need to adopt.

I think this may be more flexible, such as only accepting some json-rpc methods, filtering out sendTransaction/sendRawTransaction methods, etc.

Maybe we can add a trait EthApiT and struct EthApi to solve the problem of API changes, like the following:

#[rpc(server)]
pub trait EthApiT:
	EthClientApiT
	+ EthBlockApiT
	+ EthTransactionApiT
	+ EthStateApiT
	+ EthExecuteApiT
	+ EthFeeApiT
	+ EthMiningApiT
	+ EthSubmitApiT
{
}

pub struct EthApi<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> {
	block_api: EthBlockApi<B, C, BE>,
	client_api: EthClientApi<B, C, BE, H>,
	execute_api: EthExecuteApi<B, C, BE, A>,
	fee_api: EthFeeApi<B, C, BE>,
	mining_api: EthMiningApi,
	state_api: EthStateApi<B, C, BE, P, A>,
	submit_api: EthSubmitApi<B, C, P, CT, BE, H, A>,
	transaction_api: EthTransactionApi<B, C, BE, A>,
}

impl<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> EthApiT for EthApi<B, C, P, CT, BE, H, A> {}

impl<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> EthBlockApiT for EthApi<B, C, P, CT, BE, H, A> {
    ...
}
impl<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> EthClientApiT for EthApi<B, C, P, CT, BE, H, A> {
    ...
}
impl<B: BlockT, C, P, CT, BE, H: ExHashT, A: ChainApi> EthExecuteApiT for EthApi<B, C, P, CT, BE, H, A> {
    ...
}
...

[koushiro]

@sorpaas what do you think?

[sorpaas]

My main concern is regarding the split trait is honestly how the categorization is done. We'll have more RPC methods in the eth_ namespace in the future, and what I worry is that people cannot (clearly) see where a method should belong then and randomly adding it in a trait. We'll then have a categorization that no one will understand.

If you can explain more about how you did the categorization, it may be helpful. Otherwise, if there are any official EIP categorization, then my recommendation is that we follow that (we of course don't need to strictly follow EIPs if there's something absurd, but given this is not really in our main area of concern, in this case it's useful). If both of the above cannot be satisfied, then my recommendation is still that we tentatively don't split the trait.

[koushiro]

I know what you meaning. Actually, I largely follow the ethereum/execution-apis categorization, but it doesn't include all eth_ prefix json-rpc methods.

[koushiro]

I know what you meaning. Actually, I largely follow the ethereum/execution-apis categorization, but it doesn't include all eth_ prefix json-rpc methods.

@sorpaas what do you think? If it's still not acceptable, I'd split eth_ implementations into multiple files firstly.

[sorpaas]

I'd split eth_ implementations into multiple files firstly.

Yeah I'd prefer that at this moment. Right now I just don't see much benefits to split the traits (it's unlikely that end users will want to use only a subset of eth_ namespace) and it creates a lot of burdens for them when they need to upgrade.

[koushiro]

@sorpaas could you PTAL again?

[sorpaas]

Please pull master.