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.

Sign up for GitHub

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

Jump to bottom

Move more default implementations to ChainEndpoint #2806

Closed
5 tasks
kevinji opened this issue Nov 3, 2022 · 1 comment · Fixed by #2807
Closed
5 tasks

Move more default implementations to ChainEndpoint #2806

kevinji opened this issue Nov 3, 2022 · 1 comment · Fixed by #2807
Assignees
@seanchen1991
Labels
A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews I: logic Internal: related to the relaying logic
Milestone
v1.2

Comments

@kevinji
Copy link
Contributor

kevinji commented Nov 3, 2022

Summary

Move default implementations of init_event_monitor, id, get_key, and add_key from CosmosSdkChain to ChainEndpoint.

Problem Definition

The implementations in CosmosSdkChain are generalizable, so moving them to ChainEndpoint reduces the amount of boilerplate for other chains.

Proposal

See the summary.

Acceptance Criteria

The functions are moved over to ChainEndpoint.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@kevinji kevinji mentioned this issue Nov 3, 2022
Merged
7 tasks
@adizere
Copy link
Member

adizere commented Nov 3, 2022

Thanks for documenting this issue and opening a PR for it! The requested change seems straightforward. One of us will look into it in more detail and review your PR.

@adizere adizere added this to the v1.2 milestone Nov 3, 2022
@adizere adizere added I: logic Internal: related to the relaying logic A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews labels Nov 3, 2022
@seanchen1991 seanchen1991 self-assigned this Nov 4, 2022
romac added a commit that referenced this issue Nov 15, 2022
@kevinji @seanchen1991
@adizere @romac
ChainEndpoint: Use default implementations from CosmosSdkChain (#2807)
027578f 
* ChainEndpoint: Use default implementations from CosmosSdkChain

Fixes #2806.

This requires changing config() to return a &ChainConfig instead of a
ChainConfig, so the one call site using config() is also updated.

* Fix changelog link

Co-authored-by: Adi Seredinschi <adizere@gmail.com>
Signed-off-by: Kevin Ji <1146876+kevinji@users.noreply.github.com>

* Undo `init_event_monitor` change

Signed-off-by: Kevin Ji <1146876+kevinji@users.noreply.github.com>
Co-authored-by: Sean Chen <seanchen11235@gmail.com>
Co-authored-by: Adi Seredinschi <adizere@gmail.com>
Co-authored-by: Romain Ruetschi <romain@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@seanchen1991 seanchen1991

Labels
A: low-priority Admin: low priority / non urgent issue, expect longer wait time for PR reviews I: logic Internal: related to the relaying logic
Projects
None yet
Milestone
v1.2
Development

Successfully merging a pull request may close this issue.

ChainEndpoint: Use default implementations from CosmosSdkChain
3 participants
@kevinji @adizere @seanchen1991