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 Upgrade Part 2 - Human Readable IBC Denom #4649

Merged
merged 44 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
618947d
chore: adding scaffolding function and workflow for v8 upgrade
chatton Sep 5, 2023
c1e3359
chore: adding basic scaffolding for upgrade test
chatton Sep 6, 2023
b41931c
chore: duplicating sdk function temporarily
chatton Sep 6, 2023
635bc3c
chore: happy path test passing
chatton Sep 6, 2023
46be834
chore: adding proposal to modify allowed clients list
chatton Sep 6, 2023
4455333
chore: remove proposal id
chatton Sep 6, 2023
7a50bd6
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
charleenfei Sep 8, 2023
94eb0d5
linter
charleenfei Sep 8, 2023
26841f9
update params for ExecuteGovProposalV1
charleenfei Sep 8, 2023
9c13c40
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
chatton Sep 11, 2023
eb1fe98
chore: use actualBalance Int64 values
chatton Sep 11, 2023
283f9dd
chore: add conditional check for tmjson/json libraries
chatton Sep 11, 2023
860fb84
chore: hard code image to PR
chatton Sep 11, 2023
307f28f
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
chatton Sep 12, 2023
f7482c0
chore: bumping go.mod versions to version with migration fix
chatton Sep 12, 2023
df0d044
chore: merge conflicts
chatton Sep 12, 2023
a4f47ac
chore: run upgrade tests when upgrade_test.go changes
chatton Sep 12, 2023
9a830a1
chore: adding better comment to the custom MustProtoMarshalJSON
chatton Sep 12, 2023
1299b98
chore: removing dupliated function and use the sdk fn directly
chatton Sep 12, 2023
b883409
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
chatton Sep 12, 2023
f469dc3
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
chatton Sep 13, 2023
6c7d4cf
Merge branch 'cian/issue#4216-add-v7-to-v8-e2e-upgrade-test' of https…
chatton Sep 13, 2023
ecd9a33
chore: bumping sdk version to latest commit
chatton Sep 13, 2023
882b657
chore: merge main
chatton Sep 13, 2023
4b4fa7b
chore: merge main
chatton Sep 13, 2023
591203c
chore: fixed usage of s.ExecuteGovV1Proposal
chatton Sep 13, 2023
4a8b708
chore: update E2E to assert human readable denom
chatton Sep 13, 2023
f863354
Merge branch 'main' into cian/issue#4216-add-v7-to-v8-e2e-upgrade-test
chatton Sep 13, 2023
1b80c4f
chore: correct logic around proposal id
chatton Sep 14, 2023
20c810f
chore: addressing PR feedback
chatton Sep 14, 2023
c0faf67
chore: fix proposal id logic
chatton Sep 14, 2023
ed168a3
chore: merge main
chatton Sep 14, 2023
b76b9c9
Merge branch 'cian/issue#4216-add-v7-to-v8-e2e-upgrade-test' into cia…
chatton Sep 14, 2023
d37c0a0
chore: corrected HumanReadableDenomFeatureReleases
chatton Sep 14, 2023
46fd3ba
chore: correcting upgrade chain
chatton Sep 14, 2023
ef727b6
chore: addressed PR feedback
chatton Sep 14, 2023
5785dc6
Merge branch 'main' into cian/issue#4645-v7-to-v8-upgrade-test-part-2
chatton Sep 14, 2023
aace0a1
Merge branch 'main' into cian/issue#4645-v7-to-v8-upgrade-test-part-2
chatton Sep 14, 2023
d0f3b34
Merge branch 'main' into cian/issue#4645-v7-to-v8-upgrade-test-part-2
crodriguezvega Sep 15, 2023
240c89c
Merge branch 'main' into cian/issue#4645-v7-to-v8-upgrade-test-part-2
crodriguezvega Sep 15, 2023
33dc779
merged with other code for testing denom metadata
crodriguezvega Sep 15, 2023
76b007f
Merge branch 'main' into cian/issue#4645-v7-to-v8-upgrade-test-part-2
chatton Sep 18, 2023
043ca76
chore: merge main
chatton Sep 18, 2023
a5087a6
chore: merge main
chatton Sep 18, 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
2 changes: 1 addition & 1 deletion .github/workflows/e2e-upgrade.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
chain-binary: simd
chain-a-tag: v7.0.0
chain-b-tag: v7.0.0
chain-upgrade-tag: pr-4591 # TODO: update this to a real tag once v8 is released
chain-upgrade-tag: main # TODO: update this to a real tag once v8 is released
Copy link
Contributor

Choose a reason for hiding this comment

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

I opened #4674.

upgrade-plan-name: "v8"
test-entry-point: "TestUpgradeTestSuite"
test: "TestV7ToV8ChainUpgrade"
Expand Down
8 changes: 2 additions & 6 deletions e2e/tests/transfer/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,8 @@ func (s *TransferTestSuite) TestMsgTransfer_Succeeds_Nonincentivized() {
})

if testvalues.TokenMetadataFeatureReleases.IsSupported(chainBVersion) {
t.Run("metadata for token exists on chainB", func(t *testing.T) {
balances, err := s.QueryAllBalances(ctx, chainB, chainBAddress, true)
s.Require().NoError(err)

// balance for IBC token returns a human-readable denomination
s.Require().Equal(chainBIBCToken.GetFullDenomPath(), balances[1].Denom)
t.Run("metadata for IBC denomination exists on chainB", func(t *testing.T) {
s.AssertHumanReadableDenom(ctx, chainB, chainADenom, channelA)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I put together both assertions (check metadata is stored, check that balance returns the human readable denom). I hope that's ok, @chatton.

})
}

Expand Down
14 changes: 9 additions & 5 deletions e2e/tests/upgrades/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -721,21 +721,21 @@ func (s *UpgradeTestSuite) TestV7ToV8ChainUpgrade() {
s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA), "failed to wait for blocks")

t.Run("upgrade chain", func(t *testing.T) {
govProposalWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
s.UpgradeChain(ctx, chainA, govProposalWallet, testCfg.UpgradeConfig.PlanName, testCfg.ChainConfigs[0].Tag, testCfg.UpgradeConfig.Tag)
govProposalWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
s.UpgradeChain(ctx, chainB, govProposalWallet, testCfg.UpgradeConfig.PlanName, testCfg.ChainConfigs[0].Tag, testCfg.UpgradeConfig.Tag)
})

t.Run("update params", func(t *testing.T) {
authority, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA)
authority, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainB)
s.Require().NoError(err)
s.Require().NotNil(authority)

msg := clienttypes.NewMsgUpdateParams(authority.String(), clienttypes.NewParams(exported.Tendermint, "some-client"))
s.ExecuteGovV1Proposal(ctx, msg, chainA, chainAWallet)
s.ExecuteGovV1Proposal(ctx, msg, chainB, chainBWallet)
})

t.Run("query params", func(t *testing.T) {
clientParams, err := s.GetChainGRCPClients(chainA).ClientQueryClient.ClientParams(ctx, &clienttypes.QueryClientParamsRequest{})
clientParams, err := s.GetChainGRCPClients(chainB).ClientQueryClient.ClientParams(ctx, &clienttypes.QueryClientParamsRequest{})
s.Require().NoError(err)

allowedClients := clientParams.Params.AllowedClients
Expand All @@ -744,6 +744,10 @@ func (s *UpgradeTestSuite) TestV7ToV8ChainUpgrade() {
s.Require().Contains(allowedClients, exported.Tendermint)
s.Require().Contains(allowedClients, "some-client")
})

t.Run("query human readable ibc denom", func(t *testing.T) {
s.AssertHumanReadableDenom(ctx, chainB, chainADenom, channelA)
})
}

// RegisterInterchainAccount will attempt to register an interchain account on the counterparty chain.
Expand Down
41 changes: 27 additions & 14 deletions e2e/testsuite/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,20 +279,6 @@ func (s *E2ETestSuite) QueryCounterPartyPayee(ctx context.Context, chain ibc.Cha
return res.CounterpartyPayee, nil
}

// QueryBalances returns all the balances on the given chain for the provided address.
func (s *E2ETestSuite) QueryAllBalances(ctx context.Context, chain ibc.Chain, address string, resolveDenom bool) (sdk.Coins, error) {
queryClient := s.GetChainGRCPClients(chain).BankQueryClient
res, err := queryClient.AllBalances(ctx, &banktypes.QueryAllBalancesRequest{
Address: address,
ResolveDenom: resolveDenom,
})
if err != nil {
return sdk.Coins{}, err
}

return res.Balances, nil
}

// QueryProposalV1Beta1 queries the governance proposal on the given chain with the given proposal ID.
func (s *E2ETestSuite) QueryProposalV1Beta1(ctx context.Context, chain ibc.Chain, proposalID uint64) (govtypesv1beta1.Proposal, error) {
queryClient := s.GetChainGRCPClients(chain).GovQueryClient
Expand Down Expand Up @@ -394,3 +380,30 @@ func (s *E2ETestSuite) QueryGranterGrants(ctx context.Context, chain *cosmos.Cos

return grants.Grants, nil
}

// QueryBalances returns all the balances on the given chain for the provided address.
func (s *E2ETestSuite) QueryAllBalances(ctx context.Context, chain ibc.Chain, address string, resolveDenom bool) (sdk.Coins, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I moved this down here to keep both usages of the bank query client together.

queryClient := s.GetChainGRCPClients(chain).BankQueryClient
res, err := queryClient.AllBalances(ctx, &banktypes.QueryAllBalancesRequest{
Address: address,
ResolveDenom: resolveDenom,
})
if err != nil {
return sdk.Coins{}, err
}

return res.Balances, nil
}

// QueryDenomMetadata queries the metadata for the given denom.
func (s *E2ETestSuite) QueryDenomMetadata(ctx context.Context, chain *cosmos.CosmosChain, denom string) (banktypes.Metadata, error) {
bankClient := s.GetChainGRCPClients(chain).BankQueryClient
queryRequest := &banktypes.QueryDenomMetadataRequest{
Denom: denom,
}
res, err := bankClient.DenomMetadata(ctx, queryRequest)
if err != nil {
return banktypes.Metadata{}, err
}
return res.Metadata, nil
}
15 changes: 15 additions & 0 deletions e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,21 @@ func (s *E2ETestSuite) AssertPacketRelayed(ctx context.Context, chain *cosmos.Co
s.Require().Empty(commitment)
}

// AssertHumanReadableDenom asserts that a human readable denom is present for a given chain.
func (s *E2ETestSuite) AssertHumanReadableDenom(ctx context.Context, chain *cosmos.CosmosChain, counterpartyNativeDenom string, counterpartyChannel ibc.ChannelOutput) {
chainIBCDenom := GetIBCToken(counterpartyNativeDenom, counterpartyChannel.Counterparty.PortID, counterpartyChannel.Counterparty.ChannelID)

denomMetadata, err := s.QueryDenomMetadata(ctx, chain, chainIBCDenom.IBCDenom())
s.Require().NoError(err)

s.Require().Equal(chainIBCDenom.IBCDenom(), denomMetadata.Base, "denom metadata base does not match expected %s: got %s", chainIBCDenom.IBCDenom(), denomMetadata.Base)
expectedName := fmt.Sprintf("%s/%s/%s IBC token", counterpartyChannel.Counterparty.PortID, counterpartyChannel.Counterparty.ChannelID, counterpartyNativeDenom)
s.Require().Equal(expectedName, denomMetadata.Name, "denom metadata name does not match expected %s: got %s", expectedName, denomMetadata.Name)
expectedDisplay := fmt.Sprintf("%s/%s/%s", counterpartyChannel.Counterparty.PortID, counterpartyChannel.Counterparty.ChannelID, counterpartyNativeDenom)
s.Require().Equal(expectedDisplay, denomMetadata.Display, "denom metadata display does not match expected %s: got %s", expectedDisplay, denomMetadata.Display)
s.Require().Equal(strings.ToUpper(counterpartyNativeDenom), denomMetadata.Symbol, "denom metadata symbol does not match expected %s: got %s", strings.ToUpper(counterpartyNativeDenom), denomMetadata.Symbol)
}

// createCosmosChains creates two separate chains in docker containers.
// test and can be retrieved with GetChains.
func (s *E2ETestSuite) createCosmosChains(chainOptions ChainOptions) (*cosmos.CosmosChain, *cosmos.CosmosChain) {
Expand Down
1 change: 0 additions & 1 deletion modules/apps/transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ func NewKeeper(
if addr := authKeeper.GetModuleAddress(types.ModuleName); addr == nil {
panic("the IBC transfer module account has not been set")
}

// set KeyTable if it has not already been set
if !legacySubspace.HasKeyTable() {
legacySubspace = legacySubspace.WithKeyTable(types.ParamKeyTable())
Expand Down
Loading