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

fix: packet keys corrupted #7546

Closed
wants to merge 1 commit into from
Closed

fix: packet keys corrupted #7546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions modules/core/24-host/v2/packet_keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,22 @@ package v2

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
)

// PacketReceiptKey returns the store key of under which a packet
// receipt is stored
func PacketReceiptKey(channelID string, sequence uint64) []byte {
return []byte(fmt.Sprintf("receipts/channels/%s/sequences/%s", channelID, sdk.Uint64ToBigEndian(sequence)))
return []byte(fmt.Sprintf("receipts/channels/%s/sequences/%d", channelID, sequence))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdym by corrupted? String value for key is only intermediate repr before casting to bytes, no? Is there missing context whereby we care about string value for these?

Looking at spec, these will continue using BigEndian encoding for seqs to keep sorted order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what the string ends up looking like: "acks/channels/channel-0/sequences/\x00\x00\x00\x00\x00\x00\x00\x01
and it creates some issues with our relayer. It should technically not be a huge problem, but I can't imagine this being the idea of what is should look like at least :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main reason for doing this was to make it so iteration over channel IDs/ sequences etc happens in an expected order, rather than lexicographically. We are doing this also in the PacketForwardKey fn.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, specifically #7132

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, so we want it to look like this? If so, I can go back and figure out a way to deal with those characters in the operator cli

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea. Can't you just decode the byte slice into the number where needed?

}

// PacketAcknowledgementKey returns the store key of under which a packet acknowledgement is stored.
func PacketAcknowledgementKey(channelID string, sequence uint64) []byte {
return []byte(fmt.Sprintf("acks/channels/%s/sequences/%s", channelID, sdk.Uint64ToBigEndian(sequence)))
return []byte(fmt.Sprintf("acks/channels/%s/sequences/%d", channelID, sequence))
}

// PacketCommitmentKey returns the store key of under which a packet commitment is stored.
func PacketCommitmentKey(channelID string, sequence uint64) []byte {
return []byte(fmt.Sprintf("commitments/channels/%s/sequences/%s", channelID, sdk.Uint64ToBigEndian(sequence)))
return []byte(fmt.Sprintf("commitments/channels/%s/sequences/%d", channelID, sequence))
}

// NextSequenceSendKey returns the store key for the next sequence send of a given channelID.
Expand Down
19 changes: 19 additions & 0 deletions modules/core/24-host/v2/packet_keys_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package v2_test

import (
"testing"

"github.com/stretchr/testify/require"

hostv2 "github.com/cosmos/ibc-go/v9/modules/core/24-host/v2"
)

func TestPacketAcknowledgementKey(t *testing.T) {
var (
channelID = "channel-0"
sequence = uint64(1)
)

key := hostv2.PacketAcknowledgementKey(channelID, sequence)
require.Equal(t, "acks/channels/channel-0/sequences/1", string(key))
}
Loading