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

FR: Allow custom verifier in ibc::clients::tendermint::types::ClientState #1078

Closed
mina86 opened this issue Feb 6, 2024 · 7 comments · Fixed by #1097
Closed

FR: Allow custom verifier in ibc::clients::tendermint::types::ClientState #1078

mina86 opened this issue Feb 6, 2024 · 7 comments · Fixed by #1097
Milestone

Comments

@mina86
Copy link
Contributor

mina86 commented Feb 6, 2024

Feature Summary

At the moment verifier field in ibc::clients::tendermint::types::ClientState is hard coded to be tendermint’s ProdVerifier. That type implement signature verification using Rust implementation of Ed25519. However, this doesn’t work for Solana where signatures need to be verified using Solana-specific code.

As an aside, the actual implementation of verification on Solana is more complex than just replacing a stateless function (specifically to verify state an extra argument needs to be passed to the verifier) but I could live without addressing that.

Proposal

Make ibc::clients::tendermint::types::ClientState generic on the verifier type.

@Farhad-Shabani
Copy link
Member

Totally makes sense. We've also got the same request in #979. We might prioritize this.

@Farhad-Shabani
Copy link
Member

Farhad-Shabani commented Feb 9, 2024

As an aside, the actual implementation of verification on Solana is more complex than just replacing a stateless function (specifically to verify state an extra argument needs to be passed to the verifier) but I could live without addressing that.

@mina86 Could you give us a pointer or a bit more context on this? What's the argument about? why is it necessary? That would be helpful.

@github-project-automation github-project-automation bot moved this to 📥 To Do in ibc-rs Feb 9, 2024
@mina86
Copy link
Contributor Author

mina86 commented Feb 9, 2024

The way verification works in Solana is that client (who sends the transaction into the block) sends a transaction in which it calls two programs: a system one doing signature verification and the actual smart contract. The smart contract then looks at the transaction it belongs to and checks whether the call to the signature verification program exists in it.

The issue is that to perform that check, the smart contract needs a special object (an Instruction sysvar program account). You can think of it like verification happening in hardware and the verification code needs some handler to access that hardware.

The second issue is that Solana smart contract cannot have global mutable state, so it’s not possible to set that object in one place in the code and than retrieve it in another.

@seanchen1991
Copy link
Contributor

seanchen1991 commented Feb 29, 2024

Hi @mina86, just wanted to bring to your attention that the ClientState type is now generic over the verifier type. This can be done as outlined here: https://github.com/cosmos/ibc-rs/blob/main/ibc-clients/ics07-tendermint/src/context.rs#L64-L78

Let us know if anything about this is unclear 🙂

@mina86
Copy link
Contributor Author

mina86 commented Mar 6, 2024

If I understand correctly, I have to implement ClientStateValidation for my AnyClientState type such that in verify_client_message it calls verify_client_message function from tendermint with a custom verifier. At the same time ClientStateValidation can be derived using ClientState derive but if I do that then I have no way of passing the custom verifier for tendermint. So if I want to use custom verifier I cannot use ClientState derive?

@seanchen1991
Copy link
Contributor

So if I want to use custom verifier I cannot use ClientState derive

Yes, that's correct. We decided to maintain the happy path of opting for the default ProdVerifier as straightforward as possible, but the tradeoff is that injecting a custom verifier is going to take more work. There wasn't really a way to have our cake and eat it too with the current API design.

@seanchen1991
Copy link
Contributor

If it's an important enough use-case, we could look into designing a proc macro that would generate a lot of the client state boilerplate for those looking to introduce a custom verifier.

@Farhad-Shabani Farhad-Shabani added this to the 0.51.0 milestone Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants