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

[r2r] Port tendermint HTTP RPC client to use webpki roots instead of native. #1546

Merged
merged 1 commit into from
Nov 17, 2022

Conversation

artemii235
Copy link
Member

@artemii235 artemii235 added the P0 label Nov 16, 2022
@artemii235 artemii235 changed the title [wip] Port tendermint HTTP RPC client to use webpki roots instead of native. [r2r] Port tendermint HTTP RPC client to use webpki roots instead of native. Nov 16, 2022
@onur-ozkan
Copy link
Member

What do you think about making this change on fork repository? That way we sync with the updates of the upstream repository easier and of course also mm2 codebase will not grow from this change.

@artemii235
Copy link
Member Author

@ozkanonur I have thought about it, but then I noticed that many duplicated deps were removed from Cargo.lock after porting the implementation. Ideally, we would require to update the deps in our fork too. And in case of upstream update, we might have conflicts and duplicates again.

Eventually, we are required to compile less code at the cost of slightly enlarging our own code base. Maintenance cost of either solution look quite similar to me.

And it is just HTTP client - all structures are still imported from external crates. Btw, maybe we can ask tendermint-rs developers to export Client trait even if http-client feature is not enabled :) Currently, users of the lib can't implement their own client without compiling the sample implementation with all the deps.

@onur-ozkan
Copy link
Member

Btw, maybe we can ask tendermint-rs developers to export Client trait even if http-client feature is not enabled :) Currently, users of the lib can't implement their own client without compiling the sample implementation with all the deps.

This could solve all the problems actually.

Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

🔥

Copy link
Member

@laruh laruh left a comment

Choose a reason for hiding this comment

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

Great job!
Yeah, asking tendermint developers is a good idea.

@artemii235 artemii235 merged commit 7909eb1 into dev Nov 17, 2022
@artemii235 artemii235 deleted the tendermint-http-webpki branch November 17, 2022 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants