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

Confusion regarding next sequence recv #1783

Closed
3 tasks
adizere opened this issue Jul 26, 2022 · 2 comments · Fixed by #3357
Closed
3 tasks

Confusion regarding next sequence recv #1783

adizere opened this issue Jul 26, 2022 · 2 comments · Fixed by #3357
Labels
Milestone

Comments

@adizere
Copy link

adizere commented Jul 26, 2022

Summary

A relayer operator reported this confusion:

what is the expected behaviour of the /ibc/core/channel/v1/channels/{channel_id}/ports/{port_id}/next_sequence endpoint? it seems to always return “next_sequence_receive”: “1" (https://rest-cosmoshub.ecostake.com/ibc/core/channel/v1/channels/channel-141/ports/transfer/next_sequence)

The answer, thanks for @crodriguezvega, was:

I see that this endpoint retrieves the next sequence recv and I think that value is set on RecvPacket only for ordered channels. So I guess it returns 1 because the transfer channel is unordered and the next sequence recv has never been incremented after being initialized to 1 either in ChanOpenInit (here) or ChanOpenTry (here).

Expected Behaviour

No confusion regarding this undocumented feature.

Possible suggestion for a fix:

Just an idea: could it be set to null for unordered channels to avoid the confusion?

Version

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@adizere adizere changed the title Confusion regarding Confusion regarding next sequence recv Jul 26, 2022
@colin-axner
Copy link
Contributor

colin-axner commented Aug 17, 2022

Thanks for opening the issue @adizere! I agree with the suggested fix of using null as a return on unordered channels. In golang, I believe null would be 0. Might be best to just change the return value in the gRPC for now

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Oct 4, 2022

Then the solution for now is to modify NextSequenceReceive by adding some code to retrieve the channel:

channel, found := q.GetChannel(ctx, req.PortId, req.ChannelId)
if !found {
  return nil, status.Error(
    codes.NotFound,
    sdkerrors.Wrapf(types.ErrChannelNotFound, "port-id: %s, channel-id %s", req.PortId, req.ChannelId).Error(),
  )
}

and then we declare and initialize to 0 the sequence and if the channel is ORDERED then we do the same thing that we're already doing to retrieve the sequence:

var sequence uint64 = 0 // return 0 for unordered channels
if channel.Ordering == types.ORDERED {
  sequence, found = q.GetNextSequenceRecv(ctx, req.PortId, req.ChannelId)
  if !found {
    return nil, status.Error(
      codes.NotFound,
      sdkerrors.Wrapf(types.ErrSequenceReceiveNotFound, "port-id: %s, channel-id %s", req.PortId, req.ChannelId).Error(),
    )
  }
}

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

Successfully merging a pull request may close this issue.

3 participants