Skip to content
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

feat!: reintroduce RPC provider agreement logic #100

Merged
merged 21 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 48 additions & 48 deletions Cargo.lock

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

44 changes: 36 additions & 8 deletions candid/evm_rpc.did
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ type BlockTag = variant {
Pending;
};
type CandidRpcSource = variant {
EthSepolia : opt EthSepoliaService;
EthMainnet : opt EthMainnetService;
EthSepolia : opt vec EthSepoliaService;
EthMainnet : opt vec EthMainnetService;
};
type EthMainnetService = variant { BlockPi; Cloudflare; PublicNode; Ankr };
type EthSepoliaService = variant { BlockPi; PublicNode; Ankr };
Expand Down Expand Up @@ -76,6 +76,30 @@ type LogEntry = record {
removed : bool;
};
type Message = variant { Data : vec nat8; Hash : vec nat8 };
type MultiRpcResult = variant {
Consistent : Result;
Inconsistent : vec record { RpcService; Result };
};
type MultiRpcResult_1 = variant {
Consistent : Result_1;
Inconsistent : vec record { RpcService; Result_1 };
Comment on lines +85 to +87
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit puzzled by the naming MultiRpcResult_1/Result_1, MultiRpcResult_2/Result_2, which is not really indicative of what it represents. I'm guessing that candid doesn't really have a concept for generics and that's why such tricks are necessary but maybe we could have something a bot more user friendly, e.g. with fee history,

type MultiFeeHistoryResult = variant {
  Consistent : FeeHistoryResult;
  Inconsistent : vec record { RpcService; FeeHistoryResult };
};
type FeeHistoryResult = variant { Ok : opt FeeHistory; Err : RpcError };
...
eth_feeHistory : (CandidRpcSource, FeeHistoryArgs) -> (MultiFeeHistoryResult);

Does this make sense?

PS: I would also drop the Rpc in MultiRpcResult to have 2 types of results MultiResult/Result.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense for sure. This Candid file is automatically generated, but we could manually overwrite these names. The missing piece is a way to statically verify that the Candid interface matches the Rust source code. @dfx-json mentioned that a solution is currently in progress for this (as of a month or two ago).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way it was done for ckETH and some other canisters is to have the 2 (candid and Rust code) not auto-generated but then there is a unit test that controls that both are equivalent. Ideally, the candid file should be the source of truth and it should be able to generate the boilerplate Rust code, but I think that's out-of-scope for now. Maybe one easy way to modify this would be to avoid Rust generics in the types involved in the Candid interface and use struct like MultiFeeHistoryResult/FeeHistoryResult so that the generated Candid file is fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This unit test is exactly what I was looking for. Included in #105.

};
type MultiRpcResult_2 = variant {
Consistent : Result_2;
Inconsistent : vec record { RpcService; Result_2 };
};
type MultiRpcResult_3 = variant {
Consistent : Result_3;
Inconsistent : vec record { RpcService; Result_3 };
};
type MultiRpcResult_4 = variant {
Consistent : Result_4;
Inconsistent : vec record { RpcService; Result_4 };
};
type MultiRpcResult_5 = variant {
Consistent : Result_5;
Inconsistent : vec record { RpcService; Result_5 };
};
type ProviderError = variant {
TooFewCycles : record { expected : nat; received : nat };
ProviderNotFound;
Expand Down Expand Up @@ -121,6 +145,10 @@ type RpcError = variant {
ValidationError : ValidationError;
HttpOutcallError : HttpOutcallError;
};
type RpcService = variant {
EthSepolia : EthSepoliaService;
EthMainnet : EthMainnetService;
};
type SendRawTransactionResult = variant {
Ok;
NonceTooLow;
Expand Down Expand Up @@ -172,14 +200,14 @@ type ValidationError = variant {
service : {
authorize : (principal, Auth) -> ();
deauthorize : (principal, Auth) -> ();
eth_feeHistory : (CandidRpcSource, FeeHistoryArgs) -> (Result);
eth_getBlockByNumber : (CandidRpcSource, BlockTag) -> (Result_1);
eth_getLogs : (CandidRpcSource, GetLogsArgs) -> (Result_2);
eth_feeHistory : (CandidRpcSource, FeeHistoryArgs) -> (MultiRpcResult);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: from an interface perspective the name CandidRpcSource seems weird to me because the name of the language specifying the interface (Candid) shouldn't IMO appear anywhere in the interface iteself. Maybe rename it to RpcSource?

Copy link
Collaborator Author

@rvanasa rvanasa Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original intent was to distinguish from what is currently called Source for JSON-RPC methods. I switched these around based on your suggestion (RpcSource for Candid and JsonRpcSource for JSON).

eth_getBlockByNumber : (CandidRpcSource, BlockTag) -> (MultiRpcResult_1);
eth_getLogs : (CandidRpcSource, GetLogsArgs) -> (MultiRpcResult_2);
eth_getTransactionCount : (CandidRpcSource, GetTransactionCountArgs) -> (
Result_3,
MultiRpcResult_3,
);
eth_getTransactionReceipt : (CandidRpcSource, text) -> (Result_4);
eth_sendRawTransaction : (CandidRpcSource, text) -> (Result_5);
eth_getTransactionReceipt : (CandidRpcSource, text) -> (MultiRpcResult_4);
eth_sendRawTransaction : (CandidRpcSource, text) -> (MultiRpcResult_5);
getAccumulatedCycleCount : (nat64) -> (nat) query;
getAuthorized : (Auth) -> (vec text) query;
getNodesInSubnet : () -> (nat32) query;
Expand Down
6 changes: 4 additions & 2 deletions scripts/examples
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#!/usr/bin/env bash
# Run a variety of example RPC calls.

NETWORK=local
IDENTITY=default
CANISTER_ID=evm_rpc
CYCLES=100000000000
WALLET=$(dfx identity get-wallet)
WALLET=$(dfx identity get-wallet --network=$NETWORK --identity=$IDENTITY)
JSON_SOURCE=Chain=1
CANDID_SOURCE=EthMainnet

FLAGS="--with-cycles=$CYCLES --wallet=$WALLET"
FLAGS="--network=$NETWORK --identity=$IDENTITY --with-cycles=$CYCLES --wallet=$WALLET"


dfx canister call $CANISTER_ID request "(variant {$JSON_SOURCE}, "'"{ \"jsonrpc\": \"2.0\", \"method\": \"eth_gasPrice\", \"params\": [], \"id\": 1 }"'", 1000)" $FLAGS || exit 1
Expand Down
Loading