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

Jump to bottom

feat(callbacks): adr8 implementation (backport #3939) #4432

Merged
merged 9 commits into from
Aug 24, 2023
Merged

feat(callbacks): adr8 implementation (backport #3939) #4432

merged 9 commits into from
Aug 24, 2023

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Aug 23, 2023

This is an automatic backport of pull request #3939 done by Mergify.
Cherry-pick of 2c11494 has failed:

On branch mergify/bp/release/v7.3.x/pr-3939
Your branch is up to date with 'origin/release/v7.3.x'.

You are currently cherry-picking commit 2c11494c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
	modified:   modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
	new file:   modules/apps/callbacks/callbacks_test.go
	new file:   modules/apps/callbacks/export_test.go
	new file:   modules/apps/callbacks/fee_transfer_test.go
	new file:   modules/apps/callbacks/ibc_middleware.go
	new file:   modules/apps/callbacks/ibc_middleware_test.go
	new file:   modules/apps/callbacks/ica_test.go
	new file:   modules/apps/callbacks/transfer_test.go
	new file:   modules/apps/callbacks/types/callbacks.go
	new file:   modules/apps/callbacks/types/callbacks_test.go
	new file:   modules/apps/callbacks/types/errors.go
	new file:   modules/apps/callbacks/types/events.go
	new file:   modules/apps/callbacks/types/events_test.go
	new file:   modules/apps/callbacks/types/expected_keepers.go
	new file:   modules/apps/callbacks/types/export_test.go
	new file:   modules/apps/callbacks/types/keys.go
	new file:   modules/apps/callbacks/types/types_test.go
	modified:   modules/apps/transfer/keeper/keeper_test.go
	new file:   testing/mock/contract_keeper.go
	modified:   testing/mock/mock.go
	modified:   testing/simapp/app.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   .github/CODEOWNERS
	both modified:   modules/apps/29-fee/ibc_middleware_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

@srdtrk @mergify
feat(callbacks): adr8 implementation (#3939)
5d3a08d 
* 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 <25233464+colin-axner@users.noreply.github.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>
(cherry picked from commit 2c11494)

# Conflicts:
#	.github/CODEOWNERS
#	modules/apps/29-fee/ibc_middleware_test.go
@mergify mergify bot added the conflicts label Aug 23, 2023
@srdtrk srdtrk self-assigned this Aug 23, 2023
@srdtrk srdtrk added the callbacks middleware Issues for callbacks middleware label Aug 23, 2023
@srdtrk srdtrk removed the conflicts label Aug 24, 2023
colin-axner
colin-axner reviewed Aug 24, 2023
View reviewed changes
testing/endpoint.go Outdated Show resolved Hide resolved
@srdtrk srdtrk mentioned this pull request Aug 24, 2023
Merged
@srdtrk srdtrk mentioned this pull request Aug 24, 2023
Merged
colin-axner
colin-axner approved these changes Aug 24, 2023
View reviewed changes
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.

LGTM!

srdtrk
srdtrk approved these changes Aug 24, 2023
View reviewed changes
Copy link
Member

@srdtrk srdtrk left a comment

Choose a reason for hiding this comment

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

LGTM

@srdtrk srdtrk merged commit 23b4a24 into release/v7.3.x Aug 24, 2023
@srdtrk srdtrk deleted the mergify/bp/release/v7.3.x/pr-3939 branch August 24, 2023 15:17
@srdtrk srdtrk mentioned this pull request Aug 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srdtrk srdtrk srdtrk approved these changes

@colin-axner colin-axner colin-axner approved these changes

@AdityaSripal AdityaSripal Awaiting requested review from AdityaSripal AdityaSripal is a code owner

@charleenfei charleenfei Awaiting requested review from charleenfei

@damiannolan damiannolan Awaiting requested review from damiannolan damiannolan is a code owner

@crodriguezvega crodriguezvega Awaiting requested review from crodriguezvega

@chatton chatton Awaiting requested review from chatton

Assignees

@srdtrk srdtrk

Labels
callbacks middleware Issues for callbacks middleware
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@colin-axner @srdtrk