Add fee middleware test suite functions (E2E #5) #1710

chatton · 2022-07-15T08:41:06Z

Description

This PR adds some functionality that will be used in the coming fee middleware E2E tests.

The ability to broadcast arbitrary messages for our test suites.
- RegisterCounterPartyPayee
- PayPacketFeeAsync
Initialize GRPC clients for test chains.
Query GRPC endpoints
- QueryCounterPartyPayee
- QueryIncentivizedPacketsForChannel

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.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

…type-(e2e-#3)

…on to E2ETestSuite

crodriguezvega · 2022-07-18T19:03:54Z

e2e/fee_middleware_test.go

@@ -18,13 +24,51 @@ type FeeMiddlewareTestSuite struct {
 	testsuite.E2ETestSuite
 }

+// RegisterCounterPartyPayee broadcasts a MsgRegisterCounterpartyPayee message.
+func (s *FeeMiddlewareTestSuite) RegisterCounterPartyPayee(ctx context.Context, chain *cosmos.CosmosChain, user broadcast.User, portID, channelID, relayerAddr, counterpartyPayeeAddr string) (sdk.TxResponse, error) {


With so many arguments the line becomes quite long, should we split it in multiple lines for better readability?

damiannolan · 2022-07-19T08:20:03Z

e2e/fee_middleware_test.go

+}
+
+// PayPacketFeeAsync broadcasts a MsgPayPacketFeeAsync message.
+func (s *FeeMiddlewareTestSuite) PayPacketFeeAsync(ctx context.Context, chain *cosmos.CosmosChain, user broadcast.User, packetID channeltypes.PacketId, packetFee feetypes.PacketFee) (sdk.TxResponse, error) {


ditto on @crodriguezvega's point

damiannolan · 2022-07-19T08:20:15Z

e2e/fee_middleware_test.go

+}
+
+// QueryIncentivizedPacketsForChannel queries the incentivized packets on the specified channel.
+func (s *FeeMiddlewareTestSuite) QueryIncentivizedPacketsForChannel(ctx context.Context, chain *cosmos.CosmosChain, portId, channelId string) ([]*feetypes.IdentifiedPacketFees, error) {


ditto on @crodriguezvega's point

damiannolan · 2022-07-19T08:20:57Z

e2e/testsuite/testsuite.go

 type E2ETestSuite struct {
 	suite.Suite
+	grpcClientSets map[string]GRPCClientSet


mega nit but should we add a line break after suite.Suite...?

damiannolan · 2022-07-19T08:21:30Z

e2e/testsuite/testsuite.go

 	paths          map[string]path
 	logger         *zap.Logger
 	DockerClient   *dockerclient.Client
 	network        string
 	startRelayerFn func(relayer ibc.Relayer)
 }

+type GRPCClientSet struct {


nit: I think we should add godocs to all exported types

What's the reasoning behind the naming of having the Set suffix? Initially when I read this I thought it was supposed to be a set data structure but I'm slightly confused

The intention is just that is a group of clients, however I can see where the confusion is coming from.

After some searching it looks like the term clientSet isn't that common (used a lot in k8s ecosystem) I can rename to maybe just GRPCClients does that sound good to you?

Yep sounds good to me! :) Thanks 🤝

damiannolan · 2022-07-19T08:25:20Z

e2e/testsuite/testsuite.go

+	})
+
+	if s.grpcClientSets == nil {
+		s.grpcClientSets = map[string]GRPCClientSet{}


Any reason that we should not use make(map[string]GRPCClientSet) ?

nope! I can change it, I think make(...) syntax is more common in the codebase.

…//github.com/cosmos/ibc-go into cian/add-fee-middleware-test-suite-functions

codecov-commenter · 2022-07-19T09:04:27Z

Codecov Report

Merging #1710 (b6a6d01) into main (0fd5656) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1710   +/-   ##
=======================================
  Coverage   80.00%   80.00%           
=======================================
  Files         164      164           
  Lines       12299    12300    +1     
=======================================
+ Hits         9840     9841    +1     
  Misses       1997     1997           
  Partials      462      462

Impacted Files	Coverage Δ
modules/apps/transfer/ibc_module.go	`64.93% <100.00%> (+0.22%)`	⬆️

@colin-axner

* docs: msg types for fee middleware (#1572) * fix broken link * fix: rm AllowUpdateAfter... check (#1118) * update code & test * update proto and adr026 * update CHANGELOG * update cli docs * update broken milestone link * updated docs * update re: comments * nits: adding inline comments Co-authored-by: Sean King <[email protected]> * chore: adding module name to incentivized packet events (#1580) * docs: adding events to fee middleware docs (#1578) * adding fee distribution docs for relayer operators * adding validation information and basic cli examples * removing unnecessary whitespace * updating definitions * adding ics29 fee middleware events docs * cleanup Co-authored-by: Sean King <[email protected]> * docs: adding End Users section to ics29 docs (#1579) * docs: adding End Users section to ics29 docs * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Carlos Rodriguez <[email protected]> * Update docs/middleware/ics29-fee/end-users.md Co-authored-by: Damian Nolan <[email protected]> * chore: add link Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Damian Nolan <[email protected]> * feat: emitting an event when handling a client upgrade proposal (#1570) * feat: emitting an event when handling a client upgrade proposal * refactor: only emit event if err is nil * refactor: idiotmatic go: * docs: nits (#1595) * docs: document that version string can be empty as argument of RegisterInterchainAccount (#1582) Co-authored-by: Carlos Rodriguez <[email protected]> * docs: add upgrade client proposal event (#1596) * Consolidate usage of NewErrorAcknowledgement (#1565) * docs: adding line about module accounts / invariants (#1597) * docs: adding line about module accounts / invariants * Update docs/middleware/ics29-fee/fee-distribution.md Co-authored-by: Damian Nolan <[email protected]> * Update docs/middleware/ics29-fee/fee-distribution.md Co-authored-by: Damian Nolan <[email protected]> Co-authored-by: Damian Nolan <[email protected]> * follow up nits to #1565 (#1598) * delete test files and add error to transfer types * review comments * build(deps): bump github.com/stretchr/testify from 1.7.5 to 1.8.0 (#1616) Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.7.5 to 1.8.0. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](stretchr/testify@v1.7.5...v1.8.0) --- updated-dependencies: - dependency-name: github.com/stretchr/testify 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> * chore: bump go package version to v4 (#1564) * chore: bump go package version to v4 * update go.mod * fix alignment * fix build * review comments * build fixes * deps: bumping go version 1.18 (#1627) * bumping go version 1.18 * updating broken workflow setup * add backport to v4.0.x and remove backports to v1 (EoL) (#1629) * delete unused 04-channel version functions (#1636) * delete unused code and associated tests * Update CHANGELOG.md * build(deps): bump github.com/cosmos/cosmos-sdk from 0.45.5 to 0.45.6 (#1615) * build(deps): bump github.com/cosmos/cosmos-sdk from 0.45.5 to 0.45.6 Bumps [github.com/cosmos/cosmos-sdk](https://github.com/cosmos/cosmos-sdk) from 0.45.5 to 0.45.6. - [Release notes](https://github.com/cosmos/cosmos-sdk/releases) - [Changelog](https://github.com/cosmos/cosmos-sdk/blob/main/CHANGELOG.md) - [Commits](cosmos/cosmos-sdk@v0.45.5...v0.45.6) --- updated-dependencies: - dependency-name: github.com/cosmos/cosmos-sdk dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> * Update CHANGELOG.md * copying part of codeql workflow to try to make it pass Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> * update mergify.yml with new release branches (#1654) * Script to dynamically generate list of e2e tests (E2E #1) (#1644) * Add GitHub actions for e2e tests (E2E #2) (#1646) * Remove crossings hello (#1317) * remove crossing hellos from ChanOpenTry * remove crossing hellos testcase * revert single connectionHops check * add comment in MsgChannelOpenTry tx proto about deprecate field * Update proto/ibc/core/channel/v1/tx.proto Co-authored-by: Aditya <[email protected]> * minor fixes && UPDATE CHANGELOG.md * Update proto/ibc/core/channel/v1/tx.proto Co-authored-by: Carlos Rodriguez <[email protected]> * Update proto/ibc/core/channel/v1/tx.proto Co-authored-by: Damian Nolan <[email protected]> * apply remaining changes for crossing hello removal Deprecate previous channel id with proto tag Remove unnecessary test cases from 04-channel Remove crossing hello check in transfer application and the associated test case * remove previous channel check in WriteChannelOpenTry * regenerate proto files * update documentation * add migration documentation * remove unnecessary doc * remove crossing hello notion from ChanOpenAck * apply review suggestions Co-authored-by: Aditya <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Damian Nolan <[email protected]> Co-authored-by: Jacob Gadikian <[email protected]> Co-authored-by: Colin Axnér <[email protected]> * docs: update roadmap (#1678) * update roadmap * update roadmap * Update roadmap.md Co-authored-by: Carlos Rodriguez <[email protected]> * refactor: remove crossing hellos from 03-connection (#1672) * remove crossing hello check from ConnOpenTry and ConnOpenAck * remove unnecessary test cases and fix build * fix tests and add migration docs * fix connection version check in conn open ack * add changelog entry * Update modules/core/03-connection/keeper/handshake.go Co-authored-by: Aditya <[email protected]> * remove unnecessary testing function * improve migration documentation as per review suggestion Co-authored-by: Aditya <[email protected]> * (core/23-commitment/types) doc: fix typo (#1694) * remove spurious `TestABCICodeDeterminism` tests (#1695) ## Description Thanks @colin-axner for noticing this. closes: #XXXX --- 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. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes * update bug report issue template (#1693) * fix codeowners for 02-client (#1696) ## Description Thanks again @colin-axner for signaling this! closes: #XXXX --- 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. - [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)) - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing) - [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`) - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md` - [x] Re-reviewed `Files changed` in the Github PR explorer - [ ] Review `Codecov Report` in the comment section below once CI passes * build(deps): bump google.golang.org/grpc from 1.47.0 to 1.48.0 (#1699) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.47.0 to 1.48.0. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.47.0...v1.48.0) --- updated-dependencies: - dependency-name: google.golang.org/grpc 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> * docs: update middleware documentation (#1639) * docs: update middleware documentation * remove old text and add func keyword * alignment * fix alignment * Update develop.md * review comments * remove empty line * docs: add links in middleware docs to fee middleware implementation (#1641) * docs: add links in middleware docs to fee middleware implementation * add extra line for better readability * fix typos * Adding github action to run goimports (#1673) * Add E2ETestSuite Type (E2E #3) (#1650) * Separate go mod for e2e (E2E #4) (#1701) * put back module name in event (#1681) * fix typo * Extracting e2e tests into two separate workflows (#1719) * add categories * Add fee middleware test suite functions (E2E #5) (#1710) * Build local image to run tests with make e2e-test (#1722) * Remove leftover crossing hello tests in connection handshake (#1724) * remove leftover crossing hello tests in connection handshake * fix bug in tests * chore: adding environment variable to specify go relayer image (#1727) * Thomas/1584 update docs apps (#1675) * restructure content according to outline, fix image and syntax highlighting, fix titles and prepare for content updates * rewrite bind port section * restructure applications doc into folder structure * add keeper section, make some minor corrections in bind ports, custom packet and implmenent IBC module sections * update ibcmodule interface to encorporate the simpliefied handshake callbacks and version negotiation * fix broken links * fix remaining broken link * fix some nits, correct for removal of crossing hellos and add some more explanation on portIDs * update middleware docs to resolve merge confilicts * update fee mw docs, add formating, fix typos, increase readability (#1665) * update fee mw docs, add formating, fix typos, increase readability * fix broken link * Apply suggestions from code review (De)capitalize headings and references to headings for consistency Co-authored-by: Carlos Rodriguez <[email protected]> * resolving merge conflict * split the CLI commands and small typo correction Co-authored-by: Carlos Rodriguez <[email protected]> * chore: remove @fedekunze from CODEOWNERS (#1733) * Test for AsyncSingleSender (E2E #6) (#1682) * fix: running e2e-fork for dependabot PRs (#1745) * Adding manual triggering of e2e via workflow dispatch (#1749) * chore: denom traces migration handler (#1680) * update code & test * register migrator service * add changelog entry for #1680 * add issue templates for epics and releases (#1702) * add issue templates for epics and releases * remove text Co-authored-by: Carlos Rodriguez <[email protected]> * fix typo * fix typo * feat: allow governance to update the TrustingPeriod of the 07-tendermint light client (#1713) * initial commit * format imports * update docs * update CHANGELOG * update upgrade dev docs * update re: pr comments * E2E Test: TestMsgPayPacketFeeSingleSender (#1756) * move entry to right place * build(deps): bump docker/build-push-action from 3.0.0 to 3.1.0 (#1743) Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 3.0.0 to 3.1.0. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](docker/build-push-action@e551b19...1cb9d22) --- updated-dependencies: - dependency-name: docker/build-push-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> Co-authored-by: Carlos Rodriguez <[email protected]> * chore: Update makefile (#1770) * remove unused tools, remove incorrect import formatting * uncomment command in makefile * add version matrix (#1767) * add pseudocode to handle empty version string in OnChanOpenInit * fix broken link (#1776) * add item to update the version matrix after a release (#1775) * E2E Test: TestMsgPayPacketFeeSingleSenderTimesOut (#1751) * Move scripts from .github directory into cmd (#1787) Co-authored-by: Charly <[email protected]> Co-authored-by: Sean King <[email protected]> Co-authored-by: Sean King <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: Cian Hatton <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: vuong <[email protected]> Co-authored-by: Aditya <[email protected]> Co-authored-by: Jacob Gadikian <[email protected]> Co-authored-by: Colin Axnér <[email protected]> Co-authored-by: rene <[email protected]> Co-authored-by: tmsdkeys <[email protected]> Co-authored-by: Federico Kunze Küllmer <[email protected]>

chatton and others added 30 commits July 4, 2022 10:56

feat(e2e): adding script to dynamically generate list of e2e tests

47be6be

feat(e2e): adding github CI to run E2E tests

ce80411

feat(e2e) adding placeholder test for github action to execute

bfbba03

wip: extracting correct tag for image

4569fa5

wip: extracting correct tag for image

272ad61

wip: corrected workflow syntax

6afe90c

wip: corrected workflow syntax

7497574

removed unreferenced job

7091fbc

wip: adding makefile

6ebcc6d

wip: displaying env vars in placeholder test

05d9a45

wip: adding go script to determine simd tag

308eb0f

fix: corrected outputs name in test.yml

9ebacd7

fix: specifying correct output name

c431850

fix: updating go script to accept a single argument

3c4f62b

chore: greatly simplifying determine_simd_tag.go

b5fff2a

chore: adding docstring to determine_simd_tag.go

abb0eca

chore: extract output variable

f36025b

chore: adding echo to display the tag in the workflow

9039137

feat(e2e): adding E2ETestSuite

30591c9

wip: temporarily remove entrypoint in dockerfile

b43d07e

chore: removing SkipPathCreation

b1bcc12

removed Req and eRep member fields

d740296

merge main

e94d35b

chore: adding docstrings and cleaning up E2ETestSuite

848f888

merge main

99328b1

chore: correcting docstring

35e454f

chore: updating test.yml to pass only the pr

ccd9fd5

chore: upgrading go to 1.18 for e2e test

e154f09

Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…

236eb9f

…type-(e2e-#3)

Merge branch 'main' into cian/issue#1648-add-baseline-e2e-test-suite-…

11227eb

…type-(e2e-#3)

chatton added 5 commits July 14, 2022 11:03

chore: adding newline

58a0fd5

chore: merge main

9b2ffba

chore: handling merge conflicts

1c2e239

chore: uncommenting line from Dockerfile

da2c8b9

chore(e2e): adding fee middleware test functions and broadcast functi…

3b87fd8

…on to E2ETestSuite

chatton changed the title ~~Add fee middleware test suite functions~~ Add fee middleware test suite functions (E2E #5) Jul 15, 2022

chatton added a commit that referenced this pull request Jul 15, 2022

chore: merge with #1710

46c96bb

chatton changed the title ~~Add fee middleware test suite functions (E2E #5)~~ Add fee middleware test suite functions to E2ETestSuite (E2E #5) Jul 15, 2022

Base automatically changed from cian/separate-go-mod-for-e2e to main July 15, 2022 09:32

chore: merge main

b253b8d

chatton marked this pull request as ready for review July 15, 2022 09:38

chatton requested review from colin-axner, AdityaSripal, crodriguezvega, seantking, charleenfei and damiannolan as code owners July 15, 2022 09:38

chatton changed the title ~~Add fee middleware test suite functions to E2ETestSuite (E2E #5)~~ Add fee middleware test suite functions (E2E #5) Jul 15, 2022

chatton added a commit that referenced this pull request Jul 15, 2022

chore: merging #1710

84e5d3f

Merge branch 'main' into cian/add-fee-middleware-test-suite-functions

42e96be

crodriguezvega approved these changes Jul 18, 2022

View reviewed changes

damiannolan reviewed Jul 19, 2022

View reviewed changes

chatton and others added 3 commits July 19, 2022 09:42

chore: addressing PR feedback

196cfc7

Merge branch 'cian/add-fee-middleware-test-suite-functions' of https:…

24abfa1

…//github.com/cosmos/ibc-go into cian/add-fee-middleware-test-suite-functions

Merge branch 'main' into cian/add-fee-middleware-test-suite-functions

b6a6d01

damiannolan approved these changes Jul 19, 2022

View reviewed changes

chatton merged commit 193e414 into main Jul 19, 2022

chatton deleted the cian/add-fee-middleware-test-suite-functions branch July 19, 2022 09:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fee middleware test suite functions (E2E #5) #1710

Add fee middleware test suite functions (E2E #5) #1710

chatton commented Jul 15, 2022 •

edited

Loading

crodriguezvega Jul 18, 2022

damiannolan Jul 19, 2022

damiannolan Jul 19, 2022

damiannolan Jul 19, 2022

damiannolan Jul 19, 2022

damiannolan Jul 19, 2022

chatton Jul 19, 2022

damiannolan Jul 19, 2022

damiannolan Jul 19, 2022

chatton Jul 19, 2022

codecov-commenter commented Jul 19, 2022

Add fee middleware test suite functions (E2E #5) #1710

Add fee middleware test suite functions (E2E #5) #1710

Conversation

chatton commented Jul 15, 2022 • edited Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 19, 2022

Codecov Report

chatton commented Jul 15, 2022 •

edited

Loading