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

e2e: schedule IBC software upgrade #4585

Merged
merged 21 commits into from
Sep 11, 2023

Conversation

charleenfei
Copy link
Contributor

Description

closes: #XXXX

Commit Message / Changelog Entry

type: commit message

see the guidelines for commit messages. (view raw markdown for examples)


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md).
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

charleenfei and others added 2 commits September 5, 2023 17:28
* build(deps): Bump bufbuild/buf-setup-action from 1.25.0 to 1.25.1 (#4286)

Bumps [bufbuild/buf-setup-action](https://github.com/bufbuild/buf-setup-action) from 1.25.0 to 1.25.1.
- [Release notes](https://github.com/bufbuild/buf-setup-action/releases)
- [Commits](bufbuild/buf-setup-action@v1.25.0...v1.25.1)

---
updated-dependencies:
- dependency-name: bufbuild/buf-setup-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* deps: bump golangci-lint to v1.53.3 (#4276)

Co-authored-by: Damian Nolan <[email protected]>

* testing: fix usage on TimeoutPacket to use counterparty portID/channelID in nextSeqRecv query (#4319)

* build(deps): Bump bufbuild/buf-setup-action from 1.25.1 to 1.26.0 (#4321)

Bumps [bufbuild/buf-setup-action](https://github.com/bufbuild/buf-setup-action) from 1.25.1 to 1.26.0.
- [Release notes](https://github.com/bufbuild/buf-setup-action/releases)
- [Commits](bufbuild/buf-setup-action@v1.25.1...v1.26.0)

---
updated-dependencies:
- dependency-name: bufbuild/buf-setup-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* imp: use `types.MetadataFromVersion` helper function for callback handlers (#4290)

* feat(callbacks): adr8 implementation (#3939)

* imp(callbacks.test): added 'TestUnmarshalPacketData'

* docs(callbacks): simapp comment for callback stacks updated

* imp(callbacks.test): added 'TestOnChanCloseInit'

* fix(simapp): passed feeKeeper as channel keeper to callbacks middleware

* imp(callbacks.test): added 'TestSendPacket'

* imp(callbacks.test): added TestWriteAcknowledgement

* imp(callbacks.test): added 'TestOnChanCloseConfirm'

* imp(callbacks.test): added 'TestOnAcknowledgementPacketError'

* imp(callbacks.test): added 'TestOnTimeoutPacketError'

* imp(callbacks.test): added export_test.go

* imp(callbacks.test): added 'TestProcessCallbackDataGetterError'

* imp(callbacks.test): added events tests

* imp(callbacks): using PacketDataUnmarshaler to unmarshal instead of the full app

* imp(callbacks): updated the api of getCallbackData functions

* imp(callbacks): added TestGetSourceCallbackDataTransfer and TestGetDestCallbackDataTransfer

* imp(callbacks.test): added export_test for type tests

* imp(callbacks.test): added 'TestGetCallbackDataErrors'

* imp(fee_test): added 'TestUnmarshalPacketDataError'

* style(adr8): renamed the contract api functions

* feat(callbacks.test): added incentivized transfer tests

* imp(callbacks_test): added timeout test case to fee test

* style(ica.adr8): updated godocs

* style(ica.adr8): updated godocs

* feat(adr8): replaced PacketDataUnmarshaller with PacketInfoProvider

* imp(callbacks): added sender and receiver addresses to ContractKeeper interface

* docs(ica): godocs updated

* style(adr8): renamed PacketDataUnmarshaler to PacketInfoProvider

* style(callbacks): renamed channel to ics4Wrapper

* docs(callbacks): updated godocs

* imp(adr8_test): tested new GetPacketSender and GetPacketReceiver interface functions

* feat(adr8): added IBCSendPacketCallback to ContractKeeper

* imp(testing/mock): added callback counter helpers

* imp(ica, transfer): added WithICS4Wrapper api function

* feat(callbacks_test): SendPacket tests are now passing

* imp(fee_test): added more tests to TestPacketInfoProviderInterfaceError

* style(callbacks): renamed PacketUnmarshalerIBCModule to PacketInfoProviderIBCModule

* feat(callbacks): added maxGas param to middleware

* fix(callbacks): fixed SendPacket

* feat(callbacks): implemented WriteAcknowledgement callbacks

* style(mock): updated the name of callback counter

* fix(callbacks): fixed using channelID instead of portID

* feat(callbacks): all acknowledgements implemented

* style(ica.adr8): used more consistent formating in ica and transfer

* docs(ica, transfer): updated 'WithICS4Wrapper's godocs

* imp(callbacks_test): improved WriteAcknowledgement tests

* tests(mock, callbacks): moved mock PacketUnmarshaller logic to mock module

* tests(callbacks): added mock async ack test

* ci: CODEOWNERS updated to include callbacks

* docs(callbacks_test): updated godocs

* imp(callbacks): only handling oog panic in recovery now

* style(callbacks): fix variable name

* imp(mock): mock panic is now a real oog panic

* tests(adr8): added state reversal test

* style(callbacks): renamed hasEnoughGas to remainingGasIsAtGasLimit

* tests(adr8): moved panic and error treshold to 400k and 500k gas respectively

* fix(callbacks): fixed panic handling

* imp(callbacks_test): added a low relayer gas test

* fix(transfer_test.adr8): fixed a typo in a test case

* docs(callbacks): improved godocs

* imp(callbacks): added CommitGasLimit to CallbackData for events

* imp(callbacks): AttributeKeyCallbackCommitGasLimit added to events

* fix(callbacks): fixed major gas panic issue

* imp(callbacks_test): improved the oog panic test

* style(callbacks): used '.GetData()' instead of '.Data'

* docs(adr8): updated some godocs

* style(ica/host_test): fixed test case memo styling

* style(ica/controller): docs and style fixes

* imp(ica/host): adr8 removed from icahost

* docs(adr8): updated godocs for withics4wrapper

* docs(adr8): updated godocs for gasLimit specs

* style(adr8_test): fixed test case naming

* docs(adr8): updated godocs for some interface functions

* style(fee.adr8): renamed unmarshaler to provider in some cases

* imp(adr8_test): improved mock unmarshaler

* docs(transfer.adr8): updated godocs

* docs(adr8): updated godocs

* docs(adr8): updated godocs

* docs(callbacks): updated godocs

* style(callbacks): moved SendPacket func to top

* docs(callbacks): updated godocs

* imp(callbacks): logging to debug instead of info

* style(callbacks): renamed remainingGasIsAtGasLimit -> commitTxIfOutOfGas

* docs(callbacks): updated godocs

* docs(callbacks): updated event docs

* fix(callbacks): added CallbackTypeSendPacket to events

* imp(callbacks): events emit port and channel id based on src vs dest

* docs(callbacks): updated godocs

* imp(callbacks): changed some event to a log

* imp(callbacks): improved log

* docs(callbacks): updated godocs

* imp(callbacks): unsuccessful ack now bypasses callback in 'OnRecvPacket'

* imp(callbacks_test): added mock logger

* imp(mock): created mock logger

* style: ran 'golangci-lint run --fix'

* style(callbacks): made code more concise

* style(callbacks): renamed PacketInfoProviderIBCModule to CallbacksCompatibleModule

* style(callbacks): improved they style of getCallbackData and negated the bool for better readability

* style(callbacks): used constants for 'success' and 'failure' attributes

* docs(adr8): updated godocs

* style(ica/controller): added more explicit prefix check

* imp(adr8): moved 'GetPacketSender' and 'GetPacketReceiver' to 'CallbackPacketData' interface

* style(adr8): renamed PacketInfoProvider to PacketDataUnmarshaler

* imp(callbacks_test): switched hostStack for controllerStack

* imp(callbacks_test): added missing test case

* imp(callbacks): callbacks can now reject SendPacket

* imp(callbacks_test): added TestSendPacketReject

* style(callbacks_test): using TestCoin instead

* imp(callbacks_test): added TestWriteAcknowledgementOogError and TestOnTimeoutPacketOogError

* imp(callbacks_test): added TestOnAcknowledgementPacketOogError

* imp(adr8): removed packetReceiver concept

* imp(adr8): removed srcChannelID from GetPacketSender interface

* imp(callbacks): oogError is now simply oogPanic

* imp(callbacks): added more mw initialization notnil checks

* docs(callbacks): updated godocs

* feat(adr8): moved adr8 logic to callbacks middleware

* style(callbacks): replaced AuthAddr -> SenderAddr

* imp(callbacks_test): increased codecov

* docs(adr8): improved some godocs around AdditionalPacketDataProvider interface

* revert(docs): reverted changes to adr8 specs, this needs a seperate PR

* imp(callbacks_test): improved tests slightly

* docs(callbacks): improved godocs for keys.go

* docs(mock.adr8): updated godocs for mock logger

* imp(adr8): changed GetAdditionalData function signature

* imp(callbacks): split AdditionalPacketDataProvider into two interfaces

* style(mock): used interface impl convention

* tests: removed ErrorMock

* style(callbacks_test): improved test style

* style(callbacks_test): improved test name

* style(core, apps): renamed PacketSenderRetriever to PacketData

* style(adr8): conforming to revive linter

* fix(transfer_test, ica/controller_test): fixed failing tests

* style: conforming to revive linter

* nit(ica): removed uneeded diffs

* style(callbacks): some style updates

* docs(callbacks): updated godocs

* imp(callbacks): added 'WithICS4Wrapper'

* style(callbacks): rename GasLimit -> ExecutionGasLimit

* imp(callbacks): allowRetry was removed

* style(callbacks): moved callbackAddr code block above gas logic

* style(callbacks): renamed ContractAddr,SenderAddr ->
ContractAddress,SenderAddress

* style(callbacks): updated godocs and var names for keys and errors

* docs(callbacks): updated godocs for contract keeper

* test: remove unnecessary code

* test: apply review code suggestions

* test: move SendPacket test up in layout

* refactor: panic with errors in NewIBCMiddleware

* test: refactor TestWithICS4Wrapper to simplify and reduce  LOC

* chore: rm mock logger and usage in tests

* nit: explicitly return 0 sequence when error is not nil

* lint: make use of unused test var maxCallbackGas

* imp(callbacks): reduced case logic for eventTrigger

* style(callbacks): improved event keys

* refactor(callbacks): refactored gas logic

* imp(callbacks): empty address returns an error now

* style(callbacks): styled function arguments

* refactor: simplify testing setup for callbacks

* rename: mock's keeper.go to contract_keeper.go

* refactor: remove mock keeper, use only mock contract keeper

* rename: StateCounter -> StateEntryCounter

* nit: simapp in-code docs

* refactor: simplify mock contract keeper process callback

* test: remove unnecessary test cases in transfer/fee integration tests

* imp(callbacks): moved 'callbackDataGetter' logic up a level

* refactor(callbacks): moved emit event logic up a level

* style(callbacks): styled function arguments

* docs(callbacks): improved godocs of contract keeper

* style: renamed CallbackTypeAcknowledgement -> CallbackTypeAcknowledgementPacket

* docs(callbacks): fixed events godocs

* style(callbacks_test): fixed typo

* style(callbacks): rename timeout -> timeout_packet

* style(callbacks): rename ContractAddress -> CallbackAddress

* imp(callbacks): don't handle panics for SendPacket

* style: renamed CallbackTypeWriteAcknowledgement -> CallbackTypeReceivePacket

* style: renamed CallbackType -> CallbackTrigger

* style(callbacks_test): fixed typo in test case

* docs(mock.adr8): updated godocs of contract keeper

* imp(callbacks): moved logging after possible retry

* style(callbacks): renamed function argument callbackType -> callbackTrigger

* imp(callbacks): fixed logger name

* imp(callbacks): 'LogDebugWithPacket' added

* refactor(ibc_middleware_test): turn SendPacket into table test

* test: add test cases for SendPacket table test

* refactor(ibc_middleware_test): turn OnAcknowledgementPacket tests into table tests

* test(OnAcknowledgement): add counter and state entry checks

* test(ica_test): remove duplicate tests

Remove tests which relied on older assertion in mock contract keeper
Tests for contract execution failure can be added with issue #4390

* testing: fix usage on TimeoutPacket to use counterparty portID/channelID in nextSeqRecv query

* test(ica_test): simplify timeout logic

* test(types/callback_test.go): remove unused testing bool

* style: ran golangci-lint

* style: renamed CallbackTrigger -> CallbackType

* refactor(ibc_middleware_test): turn OnTimeout into a table test

* refactor(ibc_middleware_test): turn OnRecvPacket into a table test

* style(callbacks_test): added nolint comments

* style(callbacks_test): fixed some typos

* imp(callbacks_test): added table tests for WriteAcknowledgement'

* imp(callbacks_test): removed dest_callback test cases for ica as it is not supported

* fix(callbacks): fixed send_packet panic handling

* fix(callbacks_test): fix failing tests due to SendPacket panic

* imp(callbacks): added 'AllowRetry' function

* imp(callbacks): processCallback panic recovery logic is simplified

* style(callbacks): updated style of the comment

* fix(callbacks_test): removed potential premature return

* docs(callbacks_test): updated inline comment

* imp(callbacks_test): 'TestProcessCallback' added

* imp(callbacks): upgraded the panic on timeout logic

* docs(callbacks): added inline comments

* imp(callbacks): removed 'LogDebugWithPacket'

* docs(callbacks): updated godocs and inline comments

* imp(callbacks): prevent maxCallbackGas from being 0

* imp(callbacks): removed logger

* imp(callbacks): added 'ErrCannotUnmarshalPacketData'

* imp(callbacks_test): created ''TestGetCallbackData

* imp(callbacks_test): improved 'TestNewIBCMiddleware'

* imp(callbacks): issue#4323 - add strings.Trimspace

* docs(callbacks): issue#4325 - inline comment added for explaining why nil is returned on error

* style(callbacks): passing nil instead of err to events in SendPacket

* docs(callbacks): issue#4325 - added inline comments explaining why some error are only used for event emissions

* imp(callbacks_test): added test cases for '0' user defined gas limit

* imp(simapp): removed unneeded comment

* imp(callbacks_test): using testAccAddress for transfer tests now

---------

Co-authored-by: colin axner <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>

* Duplicate SimApp into callbacks directory (#4337)

* Remove callbacks wiring in ibc go simapp (#4340)

* Give callbacks its own go.mod (#4341)

* imp(callbacks): remove reconstructed packet logic from 'SendPacket' (#4343)

* style: ran golangci-lint

* imp(callbacks): removed unused packet param from processCallback

* imp(callbacks): removed packet from event functions

* imp(callbacks): removed packet from callbackDataGetter functions

* style(callbacks): reorder func arguments for more consistency

* imp(callbacks_test): using ibcmock.PortID for testing instead of empty string

* style(callbacks): renamed packetData to data in 'GetCallbackData' functions

* fix(callbacks): reordered EmitCallbackEvents parameters during usage

* fix(proto): fix nested msg signer annoation (#4336)

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>

* switched back to cosmos/relayer (#4345)

* build(deps): Bump golangci/golangci-lint-action from 3.6.0 to 3.7.0 (#4349)

Bumps [golangci/golangci-lint-action](https://github.com/golangci/golangci-lint-action) from 3.6.0 to 3.7.0.
- [Release notes](https://github.com/golangci/golangci-lint-action/releases)
- [Commits](golangci/golangci-lint-action@v3.6.0...v3.7.0)

---
updated-dependencies:
- dependency-name: golangci/golangci-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* build(deps): Bump bufbuild/buf-setup-action from 1.26.0 to 1.26.1 (#4350)

Bumps [bufbuild/buf-setup-action](https://github.com/bufbuild/buf-setup-action) from 1.26.0 to 1.26.1.
- [Release notes](https://github.com/bufbuild/buf-setup-action/releases)
- [Commits](bufbuild/buf-setup-action@v1.26.0...v1.26.1)

---
updated-dependencies:
- dependency-name: bufbuild/buf-setup-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Carlos Rodriguez <[email protected]>

* chore: use IsOpen method in missed places (#4354)

* end of life for v4.1, v4.2, v4.3 (#4335)

* Add linting for callbacks submodule. (#4355)

* docs(simapp): fixed inline middleware wiring comments (#4361)

* docs(callbacks/simapp): fixed inline stack documentation

* docs(testing/simapp): fixed inline stack documentation

* docs(testing/simapp): fixed icacontroller inline stack documentation

* docs(simapp): improved icaController stack's documentation

* docs(callbacks/simapp): review fixes

* bump golangci-lint to latest version

* tidy

* update cosmos-sdk

* update cosmos-sdk's submodules

* updadte to sdk with updated modules

* update capability module

* update to the latest versions of modules in sdk 50

* fix

* revert change to golangci-lint timeout

* Update go.work.example (#4397)

* refactor(ica): packet data unmarshaling logic refactored (#4232)

* refactor(ica): refactored packet data's  unmarshal logic

* fix(ica_test): made tests pass

* imp(ica): changed to UnmarshalJSON api

* docs(ica): added godocs to iapd's 'UnmarshalJSON' method

* test(callbacks): checking that processCallback consumes gas (#4381)

* imp(callbacks_test): checking that processCallback consumes gas from callback execution

* imp(callbacks_test): simplified success test case a bit

* build(deps): Bump cosmossdk.io/math from 1.0.1 to 1.1.2 (#4407)

* build(deps): Bump cosmossdk.io/math from 1.0.1 to 1.1.1

Bumps [cosmossdk.io/math](https://github.com/cosmos/cosmos-sdk) from 1.0.1 to 1.1.1.
- [Release notes](https://github.com/cosmos/cosmos-sdk/releases)
- [Changelog](https://github.com/cosmos/cosmos-sdk/blob/main/CHANGELOG.md)
- [Commits](cosmos/cosmos-sdk@math/v1.0.1...math/v1.1.1)

---
updated-dependencies:
- dependency-name: cosmossdk.io/math
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Bump in e2e, callbacks, tidy.

* bump cosmossdk.io/math to v1.1.2 across all go modules

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: DimitrisJim <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>

* deps: update all modules to go 1.21 (#4398)

* refactor: add unparam linter (#4333)

* add unparam linter

* remove unused-param from revive, add comments.

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

* Update testing/simapp/export.go

Co-authored-by: Carlos Rodriguez <[email protected]>

* eliminate redundant call

* re-add status command

* Add callbacks to dependabot. (#4410)

Co-authored-by: Carlos Rodriguez <[email protected]>

* correct applications -> apps (#4414)

Co-authored-by: Damian Nolan <[email protected]>

* deps: proto image builder and golangci-lint (#4413)

* bump the proto image builder and golangci-lint

* address implicit memory aliasing

---------

Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

* fix build issues

* Bump golangci for callbacks, fix complaints from unparam. (#4417)

* docs(callbacks): added godocs for defensive send packet validation (#4358)

* docs(callbacks): added godocs for send packet validation

* docs(callbacks): improved godocs

* docs(callbacks): used colin and damian suggested doc string

* docs(callbacks): fixed typo

* docs(callbacks): improved a minor typo

* deps: update sdk to latest sdk v0.50 release candidate in mod/capability (#4399)

* update all modules to go 1.2.1

* update golang in the Dockerfile

* update cosmos-sdk in capability module

---------

Co-authored-by: colin axnér <[email protected]>

* fix: sdk update for callbacks

* fix: update ante handler in callbacks simapp

* fix: recopy ibc-go simapp

* fix: various sdk update issues

* test(callbacks): simplified mock contract keeper's processCallback logic (#4375)

* imp(callbacks/mock/contract_keeeper): improved contract keeper logic

* imp(callbacks_test): fixed transfer_test.go

* imp(callbacks_test): fixed ica_test.go

* imp(callbacks_test): fixed fee_transfer_test.go

* style(callbacks_test): removed unneeded gas_limit

* style: ran golangci-lint

* imp(callbacks_test): implemented simplified logic

* imp(callbacks_test): removed unneeded variable

* imp(callbacks): implemented some review feedback

* docs(callbacks/simapp): updated godocs of mock contract keeper functions

* Support e2e tests for callback module SimApp (#4406)

* deps: update interchaintest to latest upstream code (#4396)

* update interchaintest to latest upstream code

* update all modules to go 1.2.1

* update golang in the Dockerfile

* use sdk math in the right places

* math to int64 where needed

* additional patches

* attempt (again)

* test the fix

* correct earlier errors

* update incorrect updates

* update incorrect iavl

* update incorrect iavl version

* re-introdue "the bug"

* use int instead of math.Int in GetAndFundTestUsers

* fix TestInterchainAccountsGroupsTest

* fix TestAuthz_InvalidTransferAuthorizations

* fix the rest of the tests ?

* convert to int64

* convert actualBalance to int64

* Revert "convert actualBalance to int64"

This reverts commit e4df8f3.

* convert balance to int64 in ica localhost test

* convert balance in interchain accounts base_test.go line 222

* convert actualBalance in line 109 of transfer's base_test.go to int64

* can we use zeroint like that?

* preassign zero

* use sdkmath.NewInt

* properly call sdkmath.ZeroInt()

---------

Co-authored-by: catShaark <[email protected]>
Co-authored-by: khanh-notional <[email protected]>
Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>

* docs: add docs for PacketData, PacketDataProvider, and PacketDataUnmarshaler interfaces (#4435)

* docs: add docs for PacketData and PacketDataProvider

* docs: add docs for PacketDataUnmarshaler

* Apply suggestions from code review

Co-authored-by: srdtrk <[email protected]>

---------

Co-authored-by: srdtrk <[email protected]>

* e2e: optimise running of params tests/set default allowed clients conditionally on version (#4428)

* wip: fixing failing params test

* fine tuned host enabled params test

* Use slices.DeleteFunc, fix LocalhostClientFeatureReleases docstring.

---------

Co-authored-by: DimitrisJim <[email protected]>

* fix: update callbacks tests to v0.50 sdk

* lint

* fix: copy over root.go

* go mod tidy

* copy over main.go from simd

* fix merge conflict

* fix(callback/types): update test file to sdk v0.50

* fix: remove toolchain directive

workflow for splitting files cannot read toolchain directive

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Sishir Giri <[email protected]>
Co-authored-by: srdtrk <[email protected]>
Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
Co-authored-by: sontrinh16 <[email protected]>
Co-authored-by: catShaark <[email protected]>
Co-authored-by: khanh-notional <[email protected]>
@charleenfei charleenfei changed the base branch from main to feat/govv1 September 6, 2023 07:33
upgradedClientState,
)
s.Require().NoError(err)
s.ExecuteGovProposalV1(ctx, scheduleUpgradeMsg, chainA, chainAWallet, 1)
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 wanted to query if the plan has been stored/upgraded client state was stored, but realised after digging around that the UpgradeKeeper doesn't seem to have a query client. what do you guys think? our msg_server tests do perform this check, is that sufficient?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can try to upgrade the client on the counterparty. Hermes might do it automatically. Maybe try starting the relayer and querying the client's status afterward

Copy link
Member

@damiannolan damiannolan Sep 7, 2023

Choose a reason for hiding this comment

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

IIRC correctly hermes previously had an upgrade client cmd which actually submitted the proposal on your behalf. See here - and also here

At a glance I think this is supposed to be doing the upgrade client flow, but I'm unsure how hermes handles this. Does it block and wait(?) until upgrade height, then query the upgradedClient at upgradePath and then upgrade the counterparty.

I think their cmd would need to change for gov v1, maybe it still uses the legacy v1beta1 appraoch

edit: added additional link - looks like they have cmd to send proposal and separate to then carry out the upgrade, my thinking would be that the latter blocks and waits for upgrade height

Copy link
Member

Choose a reason for hiding this comment

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

You can try to upgrade the client on the counterparty. Hermes might do it automatically. Maybe try starting the relayer and querying the client's status afterward

Might need to check if interchaintest has built in the support for client updates. Hermes can do it based on the links in my first comment. But I'm unsure if rly also has the same support, and interchaintest would need to expose an API for it in their relayer interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

We added event emission when the consensus state is written allowing the counterparty client to be upgraded, unsure if hermes started listening for that event

Copy link
Contributor

Choose a reason for hiding this comment

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

I regress on my suggestion, happy to finish the full e2e of upgrading the client in a followup pr as this isn't essential for v8 testing

Copy link
Contributor Author

@charleenfei charleenfei Sep 7, 2023

Choose a reason for hiding this comment

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

after discussion with @colin-axner , will just merge this e2e testing the scope of the ScheduleIBCSoftwareUpgrade for now.

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 their cmd would need to change for gov v1, maybe it still uses the legacy v1beta1 appraoch

You're right, @damiannolan: hermes is using gov v1beta proposal. Let's ask them.

e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
upgradedClientState,
)
s.Require().NoError(err)
s.ExecuteGovProposalV1(ctx, scheduleUpgradeMsg, chainA, chainAWallet, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can try to upgrade the client on the counterparty. Hermes might do it automatically. Maybe try starting the relayer and querying the client's status afterward

e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
@charleenfei charleenfei marked this pull request as draft September 7, 2023 14:58
@charleenfei charleenfei marked this pull request as ready for review September 7, 2023 16:07
Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

Thank you @charleenfei!!

e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
@colin-axner colin-axner mentioned this pull request Sep 11, 2023
9 tasks
Copy link
Member

@damiannolan damiannolan left a comment

Choose a reason for hiding this comment

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

LGTM pending chain ID using some value

e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@chatton chatton left a comment

Choose a reason for hiding this comment

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

LGTM! Just a few small suggestions 👍

Also - seems the linter is complaining 😢

e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
e2e/tests/core/02-client/client_test.go Outdated Show resolved Hide resolved
@colin-axner colin-axner merged commit 7585e3f into feat/govv1 Sep 11, 2023
@colin-axner colin-axner deleted the charly/e2e-schedule-ibc-upgrad branch September 11, 2023 13:25
colin-axner added a commit that referenced this pull request Sep 13, 2023
…and `MsgIBCSoftwareUpgrade`. The legacy proposal types `ClientUpdateProposal` and `UpgradeClientProposal` have been removed. (#4620)

* Add proto message, implement sdk.Msg for Recover Client. (#4494)

* Add proto message for Recover client, implement sdk.Message interface.

* Update modules/core/02-client/types/msgs.go

Co-authored-by: Damian Nolan <[email protected]>

* Apply suggestions from code review

Co-authored-by: Charly <[email protected]>

* Remove gogoproto false for cmp, lint, move ibctesting address inline.

---------

Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Charly <[email protected]>

* add protos, msgs, keeper handler to upgrade clients using v1 governance proposals (#4436)

* add protos and keeper function placeholder

* add keeper functions/tests, msgs and tests

* update to 0.47.5 release branch

* Move signer as last proto field. (#4510)

* msg server function and tests for MsgScheduleIBCClientUpgrade (#4442)

* Add 02-client implementation for Recover client. (#4499)

* Add 02-client implementation for Recover client.

* Partially address feedback.

* Docu RecoverClient, add label, re-use error.

* Add implementation for recover client on message server. (#4503)

* Add message server handler for recovering a client

* Don't assign to deprecated attrs, clean up unused fields.

* Further clean-up, remove declaration of unmutated vars.

* Add cmd for submitting a recover client prop. (#4522)

* Add cmd for submitting a recover client prop.

* Bump cosmossdk in e2e.

* Use govtypes.ModuleName, rename old govtypes to govv1beta1

* Update modules/core/02-client/client/cli/tx.go

Co-authored-by: Damian Nolan <[email protected]>

* Add auth flag.

---------

Co-authored-by: Damian Nolan <[email protected]>

* rename command

* docs: fixed broken links (#4571)

* Add e2e test for recovering a client. (#4543)

Co-authored-by: Colin Axnér <[email protected]>

* feat: add unpacket inerfaces message assertion (#4588)

* add cli for MsgIBCSoftwareUpgrade (#4558)

* docs: use MsgRecoverClient in docs (#4580)

* docs: recover client update

* Update docs/ibc/proposals.md

* Apply suggestions from code review

Co-authored-by: Carlos Rodriguez <[email protected]>

---------

Co-authored-by: Carlos Rodriguez <[email protected]>

* refactor: remove legacy client update proposal (#4581)

* refactor: remove legacy client update proposal

* e2e: swap from ClientUpdateProposal e2e to RecoverClient

* refactor: remove unused events

* feat: add proposal simulator interface function and tests (#4466)

Co-authored-by: colin axner <[email protected]>

* refactor!: remove UpgradeProposal type (#4602)

* create separate event emission for ibc software upgrades vs an upgraded client (#4594)

* add new event type

* update event name

* fix build

* remove: legacy event emissions

* remove: unnecessary assignments, apply suggestion from code review

* e2e: schedule IBC software upgrade (#4585)

* wip e2e test

* query proposal

* update upgrade height in plan

* rm unnecessary wait/authority

* rm test artifact from merge

* add checks for scheduled plan

* hook up upgrade query client

* plan height

* pr fixes

* update test

* import space

* update newchainID value

* update clientID upgrade

* linter

* gci

* rm unnecessary event

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: colin axnér <[email protected]>
Co-authored-by: Sishir Giri <[email protected]>
Co-authored-by: srdtrk <[email protected]>
Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
Co-authored-by: sontrinh16 <[email protected]>
Co-authored-by: catShaark <[email protected]>
Co-authored-by: khanh-notional <[email protected]>

* chore: docs for MsgIBCSoftwareUpgrade (#4601)

* chore: update docs for UpgradeProposal -> MsgIBCSoftwareUpgrade

* chore: anticipate link change

* fix event docs

* refactor: s.Assert -> s.Require

* Apply suggestions from code review

Co-authored-by: Damian Nolan <[email protected]>

* make proto-all

* chore: update compiler assertion

* refactor: ordering, order as follows: create, update, upgrade, misbheaviour, recover, ibcsoftwareupgrade, update params

* refactor: simplify ibc software upgrade emitted event

* lint lint lint

* Apply suggestions from code review

Co-authored-by: Charly <[email protected]>

* review of feat/govv1

* pr nits

* fix tests for error wrapping

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Jim Fasarakis-Hilliard <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>
Co-authored-by: Charly <[email protected]>
Co-authored-by: Charly <[email protected]>
Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: srdtrk <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sishir Giri <[email protected]>
Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Julien Robert <[email protected]>
Co-authored-by: sontrinh16 <[email protected]>
Co-authored-by: catShaark <[email protected]>
Co-authored-by: khanh-notional <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants