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

Rewrite sov_ledger_rpc::client::RpcClient without jsonrpsee macros #1066

Open
neysofu opened this issue Oct 18, 2023 · 0 comments
Open

Rewrite sov_ledger_rpc::client::RpcClient without jsonrpsee macros #1066

neysofu opened this issue Oct 18, 2023 · 0 comments
Labels
enhancement New feature or request good first issue Good for newcomers Priority-low

Comments

@neysofu
Copy link
Member

neysofu commented Oct 18, 2023

The current ledger JSON-RPC client is auto-generated with jsonrpsee's rpc macro. This is very handy, but has two drawbacks:

  1. Some parameter serialization patterns are not supported by jsonrpsee, so we're forced to resort to hacks on the server side. See feat(rpc): implement ledger_getEvents #1058 for some context.
  2. Somewhat related to n.1, serde details are exposed in the public API even though we'd like not to e.g. HexHash. A custom trait would solve this problem by accepting the right type of arguments ([u8; 32]), and the inner serialization logic can then become an implementation detail.

Once this is done, we also need to:

  • Remove the ledger_getEvents hack on the server-side.
  • Remove the test for said hack.
@neysofu neysofu added enhancement New feature or request good first issue Good for newcomers Priority-low labels Oct 18, 2023
neysofu added a commit that referenced this issue Oct 18, 2023
Related GH issue: <#1066>.

Co-authored-by: Filippo Costa <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>

Signed-off-by: Filippo Costa <[email protected]>
neysofu added a commit that referenced this issue Oct 18, 2023
Related GH issue: <#1066>.

Signed-off-by: Filippo Costa <[email protected]>
Co-authored-by: Filippo Costa <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
github-merge-queue bot pushed a commit that referenced this issue Oct 18, 2023
Related GH issue: <#1066>.

Signed-off-by: Filippo Costa <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Priority-low
Projects
None yet
Development

No branches or pull requests

1 participant