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

feat: handler for ChanUpgradeTry #3628

Merged
merged 114 commits into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from 113 commits
Commits
Show all changes
114 commits
Select commit Hold shift + click to select a range
995c1f1
added validateProposedUpgradeFields function and CheckIsOpen helper f…
colin-axner Apr 12, 2023
f8f6ad4
added unit test for CheckIsOpen
chatton Apr 13, 2023
808ec70
adding unit tests for ValidateProposedUpgradeFields
chatton Apr 13, 2023
8dd0c44
fix linter
chatton Apr 13, 2023
64878ae
update to use errorsmod instead of sdk errors
chatton Apr 13, 2023
51bb7a7
added upgrade verification function and unit tests
chatton Apr 13, 2023
f203df9
improved variable naming
chatton Apr 13, 2023
837e3c9
Merge branch '04-channel-upgrades' into cian/issue#3445-add-verifiabl…
chatton Apr 13, 2023
5f9ad67
re-arranged order of test functions
chatton Apr 13, 2023
7bdf33c
Merge branch 'cian/issue#3445-add-verifiable-upgrade-type-and-validat…
chatton Apr 13, 2023
13812eb
refactor chan open init
chatton Apr 13, 2023
2f7d0fc
removed commented out code
chatton Apr 13, 2023
0b60a32
update ChanUpgradeTry and Write functions
charleenfei Apr 14, 2023
17c6379
Merge remote-tracking branch 'origin/cian/issue#3447-modify-upgradein…
charleenfei Apr 14, 2023
28779f5
rename ValidateProposedUpgradeFields to ValidateUpgradeFields
chatton Apr 18, 2023
08af922
merge 04-channel-upgrades
chatton Apr 18, 2023
458b171
merge 04-channel-upgrades branch
chatton Apr 18, 2023
93db50e
refactoring to use new upgrade type
chatton Apr 18, 2023
b83849a
removed unused functions and tests
chatton Apr 18, 2023
f29e2f8
remove unnecessary upgrade prefix
chatton Apr 18, 2023
134b5a2
update msgs and msg server
charleenfei Apr 18, 2023
5caa776
Merge remote-tracking branch 'upstream/cian/issue#3447-modify-upgrade…
charleenfei Apr 18, 2023
9fee155
update to merge
charleenfei Apr 18, 2023
09ddf84
addressing PR feedback
chatton Apr 18, 2023
8a710ff
delete unnecessary code
charleenfei Apr 18, 2023
ef0f3d9
Merge remote-tracking branch 'upstream/cian/issue#3447-modify-upgrade…
charleenfei Apr 18, 2023
8f36c16
fix linter
chatton Apr 18, 2023
0b36176
fixed logging messages
Apr 19, 2023
dfd9be3
wip: pr comments
chatton Apr 19, 2023
f7eb3dd
Merge branch 'cian/issue#3447-modify-upgradeinit-to-use-new-upgrade-t…
chatton Apr 19, 2023
c631e06
pr feedback: use connection id instead of channel id
chatton Apr 19, 2023
b8b5ca4
pr feedback: refactor MsgChannelUpgradeInitResponse to contain Upgrad…
chatton Apr 19, 2023
0e8092b
pr feedback: adjusting error messages and test case names
chatton Apr 19, 2023
c7c7884
add testing
charleenfei Apr 19, 2023
4c23aa5
Merge remote-tracking branch 'upstream/cian/issue#3447-modify-upgrade…
charleenfei Apr 19, 2023
59b86f1
rm spacing
charleenfei Apr 19, 2023
8f0bfe9
lint
charleenfei Apr 19, 2023
72c7169
lint
charleenfei Apr 19, 2023
7be4d3d
adding wip test for chan upgrade try
chatton Apr 19, 2023
2bc2901
update so that last seq send is set internally
charleenfei Apr 19, 2023
6d8ae60
Merge branch 'charly/#3448-modify-upgrade-try' of github.com:cosmos/i…
charleenfei Apr 19, 2023
89f804d
rm merged code
charleenfei Apr 19, 2023
1b813d2
refactored init test to use fields and timeout
chatton Apr 19, 2023
a9a7f44
Merge branch 'charly/#3448-modify-upgrade-try' of https://github.com/…
chatton Apr 19, 2023
3f2e815
correctly use channel on chain b
chatton Apr 19, 2023
11e8ec2
merge 04-channel-upgrades
chatton Apr 19, 2023
5ae5eb4
fix lintert
chatton Apr 19, 2023
b464f7f
update return values
charleenfei Apr 20, 2023
ff64798
update tests
charleenfei Apr 20, 2023
0bb2ec4
Merge branch 'charly/#3448-modify-upgrade-try' of github.com:cosmos/i…
charleenfei Apr 20, 2023
09de100
update response type
charleenfei Apr 20, 2023
885112b
lint
charleenfei Apr 20, 2023
b64f04e
rm deepEqual check for proposed/counterparty proposed fields
charleenfei Apr 20, 2023
99c737e
allow zero proof heights
charleenfei Apr 20, 2023
2330bcf
update version modification in msg_server
charleenfei Apr 20, 2023
5d539c3
rm arguments
charleenfei Apr 20, 2023
8b50c87
update re new spec changes and PR comments
charleenfei Apr 21, 2023
3302d90
channel version
charleenfei Apr 21, 2023
1eb0984
update per pr comments
charleenfei Apr 24, 2023
4f25273
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Apr 24, 2023
af728d6
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 2, 2023
e80d0cd
merge
charleenfei May 3, 2023
7c0f0eb
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 3, 2023
dd0d947
update per spec
charleenfei May 3, 2023
68f0a41
wip
charleenfei May 3, 2023
b160d73
wip
charleenfei May 22, 2023
8a9e799
add channel state to flushingupgrade method
charleenfei May 22, 2023
70630cc
rm msg server code, update upgrade handler to include flushing
charleenfei May 23, 2023
6ef341d
update tests
charleenfei May 23, 2023
c78f6c7
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 23, 2023
07759d9
pr comments
charleenfei May 24, 2023
804f6d0
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 24, 2023
3d99205
pr comments
charleenfei May 24, 2023
40ec564
Merge branch 'charly/#3448-modify-upgrade-try' of github.com:cosmos/i…
charleenfei May 24, 2023
45459d8
remove flushing upgrade handshake logic
charleenfei May 24, 2023
ac5b2d4
update event attribute key
charleenfei May 24, 2023
8eb28c2
update attribute key ordering
charleenfei May 25, 2023
ed6132f
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 26, 2023
05db79b
rm redundant event emitter
charleenfei May 26, 2023
0ecc96f
update tests to rm testing for conditional
charleenfei May 26, 2023
134c459
pr comments
charleenfei May 30, 2023
9b42618
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei May 30, 2023
5d8c43d
update test
charleenfei May 30, 2023
d7fb83f
wip method, missing flushStatus enum
charleenfei May 30, 2023
0e1250d
add enum
charleenfei May 30, 2023
d3749df
Merge branch 'charly/#3631-flush-status' into charly/write-upgrade-try
charleenfei May 30, 2023
f549077
add flushstatus enum to channel
charleenfei May 30, 2023
649415b
update flush status enum comment
charleenfei May 30, 2023
4ba442b
Merge branch 'charly/#3631-flush-status' into charly/write-upgrade-try
charleenfei May 30, 2023
70ae42a
add WriteUpgradeTry method, update event emitter to include flush status
charleenfei May 30, 2023
df5319d
Merge branch '04-channel-upgrades' into charly/write-upgrade-try
charleenfei Jun 1, 2023
7ccc3e2
rm msg server code, update godoc
charleenfei Jun 1, 2023
07c18e2
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 1, 2023
010ab73
Merge branch 'charly/write-upgrade-try' into charly/#3448-modify-upgr…
charleenfei Jun 1, 2023
3dda5e8
rm e2e
charleenfei Jun 1, 2023
0ab93c4
wrap err
charleenfei Jun 1, 2023
eff04ae
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 1, 2023
6d5edfd
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 5, 2023
1ad8f8d
update after merge
charleenfei Jun 5, 2023
54ff9f7
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 5, 2023
e6de4e5
update
charleenfei Jun 5, 2023
98846e0
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 5, 2023
3b2d33b
wip testing
charleenfei Jun 6, 2023
0bca504
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 6, 2023
2573e6b
wip test update
charleenfei Jun 6, 2023
164f03f
update tests
charleenfei Jun 6, 2023
fc0fcca
lint
charleenfei Jun 6, 2023
22b1037
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 6, 2023
2b8cbb3
use QueryChannelUpgradeProofs
charleenfei Jun 6, 2023
2943a8f
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 6, 2023
fba6fac
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
charleenfei Jun 6, 2023
65df60b
add errorsmod import
charleenfei Jun 6, 2023
e7fe924
Merge branch 'charly/#3448-modify-upgrade-try' of github.com:cosmos/i…
charleenfei Jun 6, 2023
e15bb28
Merge branch '04-channel-upgrades' into charly/#3448-modify-upgrade-try
colin-axner Jun 7, 2023
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
50 changes: 0 additions & 50 deletions e2e/tests/core/04-channel/channel_test.go

This file was deleted.

62 changes: 56 additions & 6 deletions modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cosmos/cosmos-sdk/telemetry"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v7/internal/collections"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
Expand Down Expand Up @@ -67,21 +68,69 @@ func (k Keeper) WriteUpgradeInitChannel(ctx sdk.Context, portID, channelID strin
emitChannelUpgradeInitEvent(ctx, portID, channelID, currentChannel, upgrade)
}

// upgradeTry
// ChanUpgradeTry is called by a module to accept the first step of a channel upgrade handshake initiated by
// a module on another chain. If this function is successful, the proposed upgrade will be returned. If the upgrade fails, the upgrade sequence will still be incremented but an error will be returned.
func (k Keeper) ChanUpgradeTry(
ctx sdk.Context,
portID,
channelID string,
proposedConnectionHops []string,
Copy link
Contributor

Choose a reason for hiding this comment

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

@AdityaSripal is it intentional that the TRY step must use the same version as the counterparty? I don't see a mention in the spec for this reasoning. There's no check in startFlushUpgradeHandshake for this (potentially because we automatically use the counterparty upgrade version, but there is a check for the ordering being the same (despite the fact we also use the counterparty upgrade ordering), so there's an inconsistency in construction

upgradeTimeout types.Timeout,
proposedUpgradeTimeout types.Timeout,
counterpartyProposedUpgrade types.Upgrade,
counterpartyUpgradeSequence uint64,
proofCounterpartyChannel,
Copy link
Contributor

Choose a reason for hiding this comment

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

for the actual proofs I find the name counterpartyChannelProof much better to read more naturally and to avoid confusion with proofHeight .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i spoke to @colin-axner about this, i think the issue is that this is the convention we currently use all over the rest of the codebase. i'm happy to open up an issue though to rename these params!

Copy link
Contributor

Choose a reason for hiding this comment

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

#3726

yea, happy to decide better naming, but nice to do it in one sweep through the codebase

proofCounterpartyUpgrade []byte,
proofHeight clienttypes.Height,
) (types.Upgrade, error) {
// TODO
return types.Upgrade{}, nil
channel, found := k.GetChannel(ctx, portID, channelID)
if !found {
return types.Upgrade{}, errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
}

// the channel state must be in OPEN or INITUPGRADE if we are in a crossing hellos situation
if !collections.Contains(channel.State, []types.State{types.OPEN, types.INITUPGRADE}) {
return types.Upgrade{}, errorsmod.Wrapf(types.ErrInvalidChannelState, "expected one of [%s, %s], got %s", types.OPEN, types.INITUPGRADE, channel.State)
}

connectionEnd, err := k.GetConnection(ctx, channel.ConnectionHops[0])
if err != nil {
return types.Upgrade{}, errorsmod.Wrap(err, "failed to retrieve connection using the channel connection hops")
}

// make sure connection is OPEN
Copy link
Member

Choose a reason for hiding this comment

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

nit: I feel like this comment doesn't really add much value, I prefer to avoid inline comments which are self explanatory, this part of the code is self documenting and pretty obvious whats happening

if connectionEnd.GetState() != int32(connectiontypes.OPEN) {
return types.Upgrade{}, errorsmod.Wrapf(
connectiontypes.ErrInvalidConnectionState,
"connection state is not OPEN (got %s)", connectiontypes.State(connectionEnd.GetState()).String(),
)
}

// check if packet is timed out on the receiving chain
Copy link
Member

Choose a reason for hiding this comment

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

ditto re. this comment - this is also incorrect as it mentions packet but I understand this piece of timeout code will change later with the new Timeout API

if hasPassed, err := counterpartyProposedUpgrade.Timeout.HasPassed(ctx); hasPassed {
// abort here and let counterparty timeout the upgrade
return types.Upgrade{}, errorsmod.Wrap(err, "upgrade timeout has passed")
}

// assert that the proposed connection hops are compatible with the counterparty connection hops
// the proposed connections hops must have a counterparty which matches the counterparty connection hops
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

// construct counterpartyChannel from existing information and provided counterpartyUpgradeSequence

// create upgrade fields from counterparty proposed upgrade and own verified connection hops

// if OPEN, then initialize handshake with upgradeFields
// this should validate the upgrade fields, set the upgrade path and set the final correct sequence.

// otherwise, if the channel state is already in INITUPGRADE (crossing hellos case),
// assert that the upgrade fields are the same as the upgrade already in progress

// } else if channel.State == types.INITUPGRADE {

// if the counterparty sequence is not equal to our own at this point, either the counterparty chain is out-of-sync or the message is out-of-sync
// we write an error receipt with our own sequence so that the counterparty can update their sequence as well.
// We must then increment our sequence so both sides start the next upgrade with a fresh sequence.
var proposedUpgrade types.Upgrade
return proposedUpgrade, nil
}

// WriteUpgradeTryChannel writes a channel which has successfully passed the UpgradeTry handshake step.
Expand Down Expand Up @@ -255,13 +304,14 @@ func extractUpgradeFields(channel types.Channel) types.UpgradeFields {

// constructProposedUpgrade returns the proposed upgrade from the provided arguments.
func (k Keeper) constructProposedUpgrade(ctx sdk.Context, portID, channelID string, fields types.UpgradeFields, upgradeTimeout types.Timeout) (types.Upgrade, error) {
seq, found := k.GetNextSequenceSend(ctx, portID, channelID)
nextSequenceSend, found := k.GetNextSequenceSend(ctx, portID, channelID)
if !found {
return types.Upgrade{}, types.ErrSequenceSendNotFound
}

return types.Upgrade{
Fields: fields,
Timeout: upgradeTimeout,
LatestSequenceSend: seq - 1,
LatestSequenceSend: nextSequenceSend - 1,
}, nil
}
Loading