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

IBC Client event for client UpgradeProposal #1558

Closed
3 tasks
ancazamfir opened this issue Jun 20, 2022 · 2 comments · Fixed by #1570
Closed
3 tasks

IBC Client event for client UpgradeProposal #1558

ancazamfir opened this issue Jun 20, 2022 · 2 comments · Fixed by #1570
Assignees

Comments

@ancazamfir
Copy link

ancazamfir commented Jun 20, 2022

Summary

hermes has a CLI for client UpgradeProposal. Currently, in order to determine if an upgrade proposal was successful, hermes needs to dig out for some non-IBC events (i.e. not defined in events.md).

Problem Definition

Proposal

The proto message for a client upgrade proposal is defined in

message UpgradeProposal {
.

Could we have an IBC event emitted (something like EmitUpgradeProposalEvent) in:

func (k Keeper) HandleUpgradeProposal(ctx sdk.Context, p *types.UpgradeProposal) error {

and documented in events.md


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega
Copy link
Contributor

crodriguezvega commented Jun 21, 2022

Hi @ancazamfir. Would including the plan height and the title in the event enough information for your needs?

I am thinking of emitting the event only in the case where the upgraded client is successfully set in the upgrade store, so to replace this line with this:

err = k.upgradeKeeper.SetUpgradedClient(ctx, p.Plan.Height, bz)

if err != nil {
  ctx.EventManager().EmitEvent(
    sdk.NewEvent(
      types.EventTypeUpgradeClientProposal,
      sdk.NewAttribute(types.AttributeKeyUpgradePlanTitle, p.Title),
      sdk.NewAttribute(types.AttributeKeyUpgradePlanHeight, strconv.FormatInt(p.Plan.Height, 10)),
    ),
  )
}

return err

Would this work for you? Do you need any more information in the event?

@ancazamfir
Copy link
Author

ancazamfir commented Jun 21, 2022

Would including the plan height and the title in the event enough information for your needs?

I think so. Currently not implemented but we are considering starting a relayer "worker" on this event, get the height and wait for the chain to reach the proposal height. At that point it would (attempt to) upgrade its clients on other chains. I assume that if we are able to query the upgraded client and consensus states it means that the plan was approved. In other words we won't have to also query the plan status (passed vs rejected) so we don't need the proposal_id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants