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

Refactor: keyless eth rpc client #4256

Merged
merged 5 commits into from Nov 22, 2023
Merged

Refactor: keyless eth rpc client #4256

merged 5 commits into from Nov 22, 2023

Conversation

msgmaxim
Copy link
Contributor

Pull Request

Closes: PRO-889

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

Split RpcSigningClient into EthRpcSigningClient (capable of signing/broadcasting) and RpcSigningClient (no signing). Similarly, split the traits into EthRpcApi and EthSigningRpcApi. Similary, for I split the "retry" traits into EthersRetrySigningRpcApi and EthersRetryRpcApi. Now EthersRetryRpcClient is generic over the rpc client so that if the inner client implements EthSigningRpcApi, then the retry client implements EthersRetrySigningRpcApi. The result is that EthersRetryRpcClient<EthRpcSigningClient> can do everything that EthersRetryRpcClient<EthRpcClient> plus broadcasting. Finally, I changed some of the interfaces to only require non-signing client (though a signing would work just fine) and the tracker uses a non-signing client.

I recommend reviewing this PR commit by commit.

Copy link

linear bot commented Nov 17, 2023

PRO-889 ETH Client without a signing key

In this PR: https://github.com/chainflip-io/chainflip-backend/pull/4081

we are forced to use a temp key to get around specifying one when we don't need one. We should allow initialising a client without a key, like we do for the SCC.

Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Attention: 212 lines in your changes are missing coverage. Please review.

Comparison is base (aa50279) 72% compared to head (73b9215) 71%.

Files Patch % Lines
engine/src/eth/retry_rpc.rs 6% 103 Missing ⚠️
engine/src/eth/rpc.rs 0% 85 Missing ⚠️
engine/src/eth/rpc/address_checker.rs 0% 18 Missing ⚠️
engine/src/witness/eth/key_manager.rs 0% 3 Missing ⚠️
engine/src/witness/eth.rs 0% 1 Missing ⚠️
engine/src/witness/eth/ethereum_deposits.rs 0% 1 Missing ⚠️
engine/src/witness/start.rs 0% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #4256   +/-   ##
=====================================
- Coverage     72%     71%   -0%     
=====================================
  Files        385     385           
  Lines      63602   63696   +94     
  Branches   63602   63696   +94     
=====================================
- Hits       45489   45488    -1     
- Misses     15774   15863   +89     
- Partials    2339    2345    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

engine/src/eth/rpc.rs Outdated Show resolved Hide resolved
engine/src/eth/retry_rpc.rs Outdated Show resolved Hide resolved
api/bin/chainflip-ingress-egress-tracker/src/main.rs Outdated Show resolved Hide resolved
@kylezs kylezs merged commit 260eb53 into main Nov 22, 2023
40 checks passed
@kylezs kylezs deleted the refactor/eth-client-no-key branch November 22, 2023 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants