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

ClientType is never stored on 'create-client' #96

Closed
5 tasks done
CharlyCst opened this issue Jan 11, 2021 · 0 comments
Closed
5 tasks done

ClientType is never stored on 'create-client' #96

CharlyCst opened this issue Jan 11, 2021 · 0 comments
Assignees
Labels
A: bug Admin: something isn't working

Comments

@CharlyCst
Copy link

CharlyCst commented Jan 11, 2021

Crate

modules

Summary of Bug

On client creation the ClientKeeper's store_client_type method is never called (see here), but the ClientType is read during client update (see here) and the update will fail if it can't be found.

The ClientKeeper has three 'store' methods that have to be implemented, two of them (store_client_stateand store_consensus_state) are called on client creation and as an implementer of the trait I expect the same behaviour with store_client_type, however currently the implicit contract is that calling store_client_state is the responsibility of the implementer of ClientKeeper. Failing to set the client type lead to runtime failure during subsequent client-update.

I see two possible solution to prevent this error in future implementations of ClientKeeper:

  • Remove the store_client_type method and make it explicit that it is the responsibility of the trait implementer to store the type.
  • Call the store_client_type on client creation to ensure a consistent behaviour of all 'store' methods.

Version

db2be6f

Steps to Reproduce

A pseudo code implementation of ClientKeeper that will fail on create-client followed update-client:

impl ClientKeeper for Dummy {
    fn store_client_type(t) {
        self.client_type = t; // Never executed
    }

   fn store_client_state(s) {
       self.client_state = s;
   }

   fn store_consensus_state(cs) {
       self.consensus_state = cs;
    }
}

The "fixed" version according to current behaviour:

impl ClientKeeper for Dummy {
    fn store_client_type(t) {
    }

   fn store_client_state(s) {
       self.client_type = s.t; // The client type is set here
       self.client_state = s;
   }

   fn store_consensus_state(cs) {
       self.consensus_state = cs;
    }
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere self-assigned this Jan 12, 2021
adizere referenced this issue in informalsystems/hermes Jan 14, 2021
@adizere adizere added the A: bug Admin: something isn't working label Jan 18, 2021
adizere referenced this issue in informalsystems/hermes Jan 19, 2021
* MsgConnectionOpenAck test cleanup

* Quick fix for #513.

* Better error msgs & frozen client check.

* UPdated changelog
hu55a1n1 referenced this issue in hu55a1n1/hermes Sep 13, 2022
* MsgConnectionOpenAck test cleanup

* Quick fix for #513.

* Better error msgs & frozen client check.

* UPdated changelog
@hu55a1n1 hu55a1n1 transferred this issue from informalsystems/hermes Sep 29, 2022
shuoer86 pushed a commit to shuoer86/ibc-rs that referenced this issue Nov 4, 2023
Remove dependency on `sp-std` and `sp-core`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: bug Admin: something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants