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

Make trait ChainHandle object-safe & clonable #447

Closed
4 of 5 tasks
adizere opened this issue Dec 10, 2020 · 0 comments · Fixed by #448
Closed
4 of 5 tasks

Make trait ChainHandle object-safe & clonable #447

adizere opened this issue Dec 10, 2020 · 0 comments · Fixed by #448
Assignees
Labels
I: logic Internal: related to the relaying logic O: new-feature Objective: cause to add a new feature or support
Milestone

Comments

@adizere
Copy link
Member

adizere commented Dec 10, 2020

Crate

relayer

Summary & problem definition

The ChainHandle trait is currently clonable thanks to its bound on Clone, which implies Sized, which in turn makes the ChainHandle non-object-safe.

https://github.com/informalsystems/ibc-rs/blob/65e4f268efc176d861d30949dc7bde049e0b3bb5/relayer/src/chain/handle.rs#L177

This means that we cannot make objects out of the ChainHandle trait. So the compiler will forbid something of the kind:

pub struct ForeignClient {
    /// The configuration of this client
    config: ForeignClientConfig,
    /// A handle to the chain hosting this client
    host_chain: dyn ChainHandle,
}

The precise error is as follows:

error[E0038]: the trait `ChainHandle` cannot be made into an object
   --> relayer/src/foreign_client.rs:59:17
    |
59  |     host_chain: Box<dyn ChainHandle>,
    |                 ^^^^^^^^^^^^^^^^^^^^ the trait `ChainHandle` cannot be made into an object
    |
   ::: relayer/src/chain/handle.rs:177:24
    |
177 | pub trait ChainHandle: Clone + Send + Sync {
    |           -----------  ----- ...because it requires `Self: Sized`
    |           |
    |           this trait cannot be made into an object...
error: aborting due to previous error
For more information about this error, try `rustc --explain E0038`.
error: could not compile `relayer`

See more details on object safety here:
https://rust-lang.github.io/rfcs/0255-object-safety.html#detailed-design

Proposal

At @romac's suggestion, we can replace the Clone bound with DynClone instead, and obtain object safety. https://github.com/kardeiz/objekt-clonable may also be an option. Then we can do this:

pub struct ForeignClient {
    /// The configuration of this client
    config: ForeignClientConfig,
    /// A handle to the chain hosting this client
    host_chain: Box<dyn ChainHandle>,
}

Alternative solutions include splitting the handle trait into the handle proper + a constructor, or adding specific where clauses cf. this. But the DynClone method seems the most robust and simple.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@adizere adizere added O: new-feature Objective: cause to add a new feature or support I: logic Internal: related to the relaying logic labels Dec 10, 2020
@adizere adizere added this to the v0.0.6 milestone Dec 10, 2020
@adizere adizere self-assigned this Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: logic Internal: related to the relaying logic O: new-feature Objective: cause to add a new feature or support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant