From 8e3dc66c8c6d6894cbf2d5d12112772069ea8774 Mon Sep 17 00:00:00 2001 From: zale144 Date: Tue, 26 Mar 2024 14:43:20 +0100 Subject: [PATCH] feat(deonmetadata): enable testing for denom metadata creation with erc20 hook (#343) --- .github/workflows/codeql.yml | 3 - Makefile | 112 +------------ testutil/app/app.go | 4 +- x/denommetadata/keeper/grpc_query_test.go | 4 +- x/denommetadata/keeper/keeper_test.go | 3 +- x/denommetadata/keeper/msg_server.go | 16 +- x/denommetadata/keeper/msg_server_test.go | 191 ++++++++++++++++++---- x/denommetadata/module.go | 13 +- x/denommetadata/testutils/testutils.go | 11 +- x/hub-genesis/genesis_test.go | 3 +- 10 files changed, 194 insertions(+), 166 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 12226a0f..ceff9804 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -34,8 +34,5 @@ jobs: languages: "go" queries: crypto-com/cosmos-sdk-codeql@main,security-and-quality - - name: Build - run: make build - - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v2 diff --git a/Makefile b/Makefile index df11edad..3b20738e 100644 --- a/Makefile +++ b/Makefile @@ -1,107 +1,8 @@ #!/usr/bin/make -f -BRANCH := $(shell git rev-parse --abbrev-ref HEAD) -COMMIT := $(shell git log -1 --format='%H') - -# don't override user values -ifeq (,$(VERSION)) - VERSION := $(shell git describe --tags) - # if VERSION is empty, then populate it with branch's name and raw commit hash - ifeq (,$(VERSION)) - VERSION := $(BRANCH)-$(COMMIT) - endif -endif - -ifndef $(GOPATH) - GOPATH=$(shell go env GOPATH) - export GOPATH -endif - -PACKAGES_SIMTEST=$(shell go list ./... | grep '/simulation') -LEDGER_ENABLED ?= true -SDK_PACK := $(shell go list -m github.com/cosmos/cosmos-sdk | sed 's/ /\@/g') -TM_VERSION := $(shell go list -m github.com/tendermint/tendermint | sed 's:.* ::') -DOCKER := $(shell which docker) -BUILDDIR ?= $(CURDIR)/build - -export GO111MODULE = on - -# process build tags - -build_tags = netgo -ifeq ($(LEDGER_ENABLED),true) - ifeq ($(OS),Windows_NT) - GCCEXE = $(shell where gcc.exe 2> NUL) - ifeq ($(GCCEXE),) - $(error gcc.exe not installed for ledger support, please install or set LEDGER_ENABLED=false) - else - build_tags += ledger - endif - else - UNAME_S = $(shell uname -s) - ifeq ($(UNAME_S),OpenBSD) - $(warning OpenBSD detected, disabling ledger support (https://github.com/cosmos/cosmos-sdk/issues/1988)) - else - GCC = $(shell command -v gcc 2> /dev/null) - ifeq ($(GCC),) - $(error gcc not installed for ledger support, please install or set LEDGER_ENABLED=false) - else - build_tags += ledger - endif - endif - endif -endif - -ifeq (cleveldb,$(findstring cleveldb,$(DYMENSION_BUILD_OPTIONS))) - build_tags += gcc cleveldb -endif -build_tags += $(BUILD_TAGS) -build_tags := $(strip $(build_tags)) - -whitespace := -whitespace := $(whitespace) $(whitespace) -comma := , -build_tags_comma_sep := $(subst $(whitespace),$(comma),$(build_tags)) - -# process linker flags - -ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=dymension-rdk \ - -X github.com/cosmos/cosmos-sdk/version.AppName=rollappd \ - -X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \ - -X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \ - -X "github.com/cosmos/cosmos-sdk/version.BuildTags=$(build_tags_comma_sep)" \ - -X github.com/tendermint/tendermint/version.TMCoreSemVer=$(TM_VERSION) - -ifeq (cleveldb,$(findstring cleveldb,$(DYMENSION_BUILD_OPTIONS))) - ldflags += -X github.com/cosmos/cosmos-sdk/types.DBBackend=cleveldb -endif -ifeq ($(LINK_STATICALLY),true) - ldflags += -linkmode=external -extldflags "-Wl,-z,muldefs -static" -endif -ifeq (,$(findstring nostrip,$(DYMENSION_BUILD_OPTIONS))) - ldflags += -w -s -endif -ldflags += $(LDFLAGS) -ldflags := $(strip $(ldflags)) - -BUILD_FLAGS := -tags "$(build_tags)" -ldflags '$(ldflags)' -# check for nostrip option -ifeq (,$(findstring nostrip,$(DYMENSION_BUILD_OPTIONS))) - BUILD_FLAGS += -trimpath -endif - # ---------------------------------------------------------------------------- # # Make targets # # ---------------------------------------------------------------------------- # -.PHONY: install -install: go.sum ## Installs the rollappd binary - go install -mod=readonly $(BUILD_FLAGS) ./cmd/rollappd - - -.PHONY: build -build: ## Compiles the rollapd binary - go build -o build/rollappd $(BUILD_FLAGS) ./cmd/rollappd - .PHONY: clean clean: ## Clean temporary files @@ -117,17 +18,6 @@ vet: ## Run go vet lint: ## Run linter golangci-lint run - -# ------------------------------------ EVM ----------------------------------- # -.PHONY: install_evm -install_evm: build_evm go.sum ## Install the rollapp_evm binary - mv build/rollapp_evm $(GOPATH)/bin/rollapp_evm - -.PHONY: build_evm -build_evm: ## Compiles the rollapp_evm binary - $(eval BUILD_FLAGS+=-ldflags '-X github.com/cosmos/cosmos-sdk/version.AppName=rollapp_evm') - go build -o build/rollapp_evm $(BUILD_FLAGS) -tags evm ./cmd/evm - # ---------------------------------------------------------------------------- # # testing # # ---------------------------------------------------------------------------- # @@ -151,7 +41,7 @@ containerProtoFmt=cosmos-sdk-proto-fmt-$(containerProtoVer) proto-gen: ## Generates protobuf files @echo "Generating Protobuf files" @if docker ps -a --format '{{.Names}}' | grep -Eq "^${containerProtoGen}$$"; then docker start -a $(containerProtoGen); else docker run --name $(containerProtoGen) -v $(CURDIR):/workspace --workdir /workspace $(containerProtoImage) \ - sh ./proto/protocgen.sh; fi + sh ./scripts/protocgen.sh; fi proto-format: ## Formats protobuf files @echo "Formatting Protobuf files" diff --git a/testutil/app/app.go b/testutil/app/app.go index 7b21c58f..39986fef 100644 --- a/testutil/app/app.go +++ b/testutil/app/app.go @@ -168,6 +168,7 @@ var ( ibctransfer.AppModuleBasic{}, vesting.AppModuleBasic{}, hubgenesis.AppModuleBasic{}, + denommetadata.AppModuleBasic{}, ) // module account permissions @@ -180,6 +181,7 @@ var ( govtypes.ModuleName: {authtypes.Burner}, ibctransfertypes.ModuleName: {authtypes.Minter, authtypes.Burner}, hubgentypes.ModuleName: {authtypes.Burner}, + denommetadatatypes.ModuleName: {authtypes.Minter}, } ) @@ -456,7 +458,7 @@ func NewRollapp( capability.NewAppModule(appCodec, *app.CapabilityKeeper), gov.NewAppModule(appCodec, app.GovKeeper, app.AccountKeeper, app.BankKeeper), mint.NewAppModule(appCodec, app.MintKeeper, app.AccountKeeper, app.BankKeeper), - denommetadata.NewAppModule(appCodec, app.DenommetadataKeeper, app.BankKeeper), + denommetadata.NewAppModule(app.DenommetadataKeeper, app.BankKeeper), distr.NewAppModule(appCodec, app.DistrKeeper, app.AccountKeeper, app.BankKeeper, app.StakingKeeper), staking.NewAppModule(appCodec, app.StakingKeeper, app.AccountKeeper, app.BankKeeper), sequencers.NewAppModule(appCodec, app.SequencersKeeper), diff --git a/x/denommetadata/keeper/grpc_query_test.go b/x/denommetadata/keeper/grpc_query_test.go index d544e3cd..90134dfb 100644 --- a/x/denommetadata/keeper/grpc_query_test.go +++ b/x/denommetadata/keeper/grpc_query_test.go @@ -11,9 +11,9 @@ import ( ) func TestParamsQuery(t *testing.T) { - k, ctx := testutils.NewTestDenommetadataKeeper(t) + app, ctx := testutils.NewTestDenommetadataKeeper(t) - q := keeper.Querier{Keeper: *k} + q := keeper.Querier{Keeper: app.DenommetadataKeeper} wctx := sdk.WrapSDKContext(ctx) diff --git a/x/denommetadata/keeper/keeper_test.go b/x/denommetadata/keeper/keeper_test.go index 5ba02a92..2563903b 100644 --- a/x/denommetadata/keeper/keeper_test.go +++ b/x/denommetadata/keeper/keeper_test.go @@ -10,7 +10,8 @@ import ( func TestParams(t *testing.T) { // Setup the test environment - k, ctx := testutils.NewTestDenommetadataKeeper(t) // Assume you have a similar utility function for denommetadata keeper + app, ctx := testutils.NewTestDenommetadataKeeper(t) // Assume you have a similar utility function for denommetadata keeper + k := app.DenommetadataKeeper // Set some initial parameters initialParams := types.DefaultParams() diff --git a/x/denommetadata/keeper/msg_server.go b/x/denommetadata/keeper/msg_server.go index 339f07f6..862a22b8 100644 --- a/x/denommetadata/keeper/msg_server.go +++ b/x/denommetadata/keeper/msg_server.go @@ -9,10 +9,20 @@ import ( "github.com/dymensionxyz/dymension-rdk/x/denommetadata/types" ) -var _ types.MsgServer = &Keeper{} +type msgServer struct { + Keeper +} + +// NewMsgServerImpl returns an implementation of the MsgServer interface +// for the provided Keeper. +func NewMsgServerImpl(keeper Keeper) types.MsgServer { + return &msgServer{Keeper: keeper} +} + +var _ types.MsgServer = msgServer{} // CreateDenomMetadata create the denom metadata in bank module -func (k Keeper) CreateDenomMetadata( +func (k msgServer) CreateDenomMetadata( goCtx context.Context, msg *types.MsgCreateDenomMetadata, ) (*types.MsgCreateDenomMetadataResponse, error) { @@ -42,7 +52,7 @@ func (k Keeper) CreateDenomMetadata( } // UpdateDenomMetadata update the denom metadata in bank module -func (k Keeper) UpdateDenomMetadata( +func (k msgServer) UpdateDenomMetadata( goCtx context.Context, msg *types.MsgUpdateDenomMetadata, ) (*types.MsgUpdateDenomMetadataResponse, error) { diff --git a/x/denommetadata/keeper/msg_server_test.go b/x/denommetadata/keeper/msg_server_test.go index daf5dd1a..25438b0f 100644 --- a/x/denommetadata/keeper/msg_server_test.go +++ b/x/denommetadata/keeper/msg_server_test.go @@ -1,49 +1,178 @@ package keeper_test import ( + "fmt" + "sync" "testing" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" + "github.com/dymensionxyz/dymension-rdk/testutil/app" + "github.com/dymensionxyz/dymension-rdk/x/denommetadata/keeper" + "github.com/dymensionxyz/dymension-rdk/x/denommetadata/testutils" + "github.com/stretchr/testify/suite" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" - "github.com/dymensionxyz/dymension-rdk/x/denommetadata/testutils" "github.com/dymensionxyz/dymension-rdk/x/denommetadata/types" ) -func TestCreateDenomMetadata(t *testing.T) { - k, ctx := testutils.NewTestDenommetadataKeeper(t) - - // Prepare the test message for creating denom metadata - createMsg := &types.MsgCreateDenomMetadata{ - SenderAddress: "cosmos1s77x8wr2gzdhq8gt8c085vate0s23xu9u80wtx", - TokenMetadata: banktypes.Metadata{ - Name: "Dymension Hub token", - Symbol: "DYM", - Description: "Denom metadata for DYM.", - DenomUnits: []*banktypes.DenomUnit{ - {Denom: "adym", Exponent: uint32(0), Aliases: []string{}}, - {Denom: "DYM", Exponent: uint32(18), Aliases: []string{}}, - }, - Base: "adym", - Display: "DYM", - }, - } +type DenomMetadataMsgServerTestSuite struct { + suite.Suite - // Test permission error - _, err := k.CreateDenomMetadata(sdk.WrapSDKContext(ctx), createMsg) - require.ErrorIs(t, err, types.ErrNoPermission, "should return permission error") + app *app.App + k keeper.Keeper + msgServer types.MsgServer + ctx sdk.Context +} + +func TestDenomMetadataMsgServerTestSuite(t *testing.T) { + suite.Run(t, new(DenomMetadataMsgServerTestSuite)) +} +func (suite *DenomMetadataMsgServerTestSuite) setupTest(hooks types.DenomMetadataHooks) { + suite.app, suite.ctx = testutils.NewTestDenommetadataKeeper(suite.T()) + suite.k = suite.app.DenommetadataKeeper + suite.k.SetHooks(types.NewMultiDenommetadataHooks(hooks)) + suite.msgServer = keeper.NewMsgServerImpl(suite.k) // Set allowed addresses initialParams := types.DefaultParams() - initialParams.AllowedAddresses = []string{"cosmos1s77x8wr2gzdhq8gt8c085vate0s23xu9u80wtx", "cosmos1gusne8eh37myphx09hgdsy85zpl2t0kzdvu3en"} - k.SetParams(ctx, initialParams) + initialParams.AllowedAddresses = []string{senderAddress} + suite.k.SetParams(suite.ctx, initialParams) +} - // Test creating denom metadata successfully - _, err = k.CreateDenomMetadata(sdk.WrapSDKContext(ctx), createMsg) - require.NoError(t, err, "creating denom metadata with allowed address should not error") +const ( + ibcBase = "ibc/7B2A4F6E798182988D77B6B884919AF617A73503FDAC27C916CD7A69A69013CF" + senderAddress = "cosmos1s77x8wr2gzdhq8gt8c085vate0s23xu9u80wtx" +) - // Test creating duplicate denom metadata - _, err = k.CreateDenomMetadata(sdk.WrapSDKContext(ctx), createMsg) - require.ErrorIs(t, err, types.ErrDenomAlreadyExists, "creating duplicate denom metadata should fail") +var denomMetadata = banktypes.Metadata{ + Description: "ATOM IBC", + Base: ibcBase, + // NOTE: Denom units MUST be increasing + DenomUnits: []*banktypes.DenomUnit{ + {Denom: ibcBase, Exponent: 0}, + {Denom: "ATOM", Exponent: 18}, + }, + Name: "ATOM channel-0", + Symbol: "ibcATOM-0", + Display: ibcBase, } + +func (suite *DenomMetadataMsgServerTestSuite) TestCreateDenomMetadata() { + cases := []struct { + name string + msg *types.MsgCreateDenomMetadata + hooks *mockERC20Hook + expectErr string + expectHookCalled bool + malleate func() + }{ + { + name: "success", + msg: &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: denomMetadata, + }, + hooks: &mockERC20Hook{}, + expectHookCalled: true, + }, { + name: "permission error", + msg: &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: denomMetadata, + }, + malleate: func() { + initialParams := types.DefaultParams() + initialParams.AllowedAddresses = []string{} + suite.k.SetParams(suite.ctx, initialParams) + }, + hooks: &mockERC20Hook{}, + expectHookCalled: false, + expectErr: types.ErrNoPermission.Error(), + }, { + name: "denom already exists", + msg: &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: denomMetadata, + }, + malleate: func() { + msg := &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: denomMetadata, + } + _, err := suite.msgServer.CreateDenomMetadata(suite.ctx, msg) + suite.Require().NoError(err, "CreateDenomMetadata() error") + }, + hooks: &mockERC20Hook{}, + expectHookCalled: false, + expectErr: types.ErrDenomAlreadyExists.Error(), + }, { + name: "invalid denom units", + msg: &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: banktypes.Metadata{ + Description: "ATOM IBC", + Base: ibcBase, + DenomUnits: []*banktypes.DenomUnit{ + {Denom: ibcBase, Exponent: 18}, + {Denom: "ATOM", Exponent: 0}, + }, + Name: "ATOM channel-0", + Symbol: "ibcATOM-0", + Display: ibcBase, + }, + }, + hooks: &mockERC20Hook{}, + expectHookCalled: false, + expectErr: fmt.Sprintf("the exponent for base denomination unit %s must be 0", ibcBase), + }, { + name: "failed to create erc20 contract", + msg: &types.MsgCreateDenomMetadata{ + SenderAddress: senderAddress, + TokenMetadata: denomMetadata, + }, + hooks: &mockERC20Hook{ + err: fmt.Errorf("failed to deploy the erc20 contract for the IBC coin"), + }, + expectHookCalled: false, + expectErr: "error in after denom metadata creation hook: failed to deploy the erc20 contract for the IBC coin", + }, + } + + for _, tc := range cases { + suite.Run(tc.name, func() { + // reset the test state + suite.setupTest(tc.hooks) + + if tc.malleate != nil { + tc.malleate() + } + + _, err := suite.msgServer.CreateDenomMetadata(suite.ctx, tc.msg) + if tc.expectErr != "" { + suite.Require().ErrorContains(err, tc.expectErr) + return + } + suite.Require().NoError(err, "CreateDenomMetadata() error") + + // check if the denom metadata was added + _, found := suite.app.BankKeeper.GetDenomMetaData(suite.ctx, ibcBase) + suite.Require().True(found, "denom metadata should exist") + suite.Require().Equal(tc.expectHookCalled, tc.hooks.createCalled, "after denom metadata creation hook should be called") + }) + } +} + +type mockERC20Hook struct { + createCalled bool + err error + sync.Mutex +} + +func (m *mockERC20Hook) AfterDenomMetadataCreation(sdk.Context, banktypes.Metadata) error { + m.Lock() + defer m.Unlock() + m.createCalled = m.err == nil + return m.err +} + +func (m *mockERC20Hook) AfterDenomMetadataUpdate(sdk.Context, banktypes.Metadata) error { return nil } diff --git a/x/denommetadata/module.go b/x/denommetadata/module.go index 8730ccc8..d23f4d0a 100644 --- a/x/denommetadata/module.go +++ b/x/denommetadata/module.go @@ -88,7 +88,6 @@ type AppModule struct { } func NewAppModule( - cdc codec.Codec, keeper keeper.Keeper, bankKeeper types.BankKeeper, ) AppModule { @@ -113,19 +112,19 @@ func (am AppModule) Route() sdk.Route { func (AppModule) QuerierRoute() string { return types.RouterKey } // LegacyQuerierHandler returns the capability module's Querier. -func (am AppModule) LegacyQuerierHandler(legacyQuerierCdc *codec.LegacyAmino) sdk.Querier { +func (am AppModule) LegacyQuerierHandler(*codec.LegacyAmino) sdk.Querier { return nil } // RegisterServices registers a GRPC query service to respond to the // module-specific GRPC queries. func (am AppModule) RegisterServices(cfg module.Configurator) { - types.RegisterQueryServer(cfg.QueryServer(), keeper.Querier{Keeper: am.keeper}) - types.RegisterMsgServer(cfg.MsgServer(), am.keeper) + types.RegisterQueryServer(cfg.QueryServer(), keeper.NewQuerier(am.keeper)) + types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) } // RegisterInvariants registers the capability module's invariants. -func (am AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} +func (am AppModule) RegisterInvariants(sdk.InvariantRegistry) {} // InitGenesis performs the capability module's genesis initialization It returns // no validator updates. @@ -147,11 +146,11 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw func (AppModule) ConsensusVersion() uint64 { return 1 } // BeginBlock executes all ABCI BeginBlock logic respective to the fees module. -func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) { +func (am AppModule) BeginBlock(sdk.Context, abci.RequestBeginBlock) { } // EndBlock executes all ABCI EndBlock logic respective to the fee-share module. It // returns no validator updates. -func (am AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { +func (am AppModule) EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate { return []abci.ValidatorUpdate{} } diff --git a/x/denommetadata/testutils/testutils.go b/x/denommetadata/testutils/testutils.go index cf862eba..eeead69e 100644 --- a/x/denommetadata/testutils/testutils.go +++ b/x/denommetadata/testutils/testutils.go @@ -4,15 +4,14 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" - + "github.com/dymensionxyz/dymension-rdk/testutil/app" testkeepers "github.com/dymensionxyz/dymension-rdk/testutil/keepers" "github.com/dymensionxyz/dymension-rdk/testutil/utils" - "github.com/dymensionxyz/dymension-rdk/x/denommetadata/keeper" ) // NewTestDenommetadataKeeper creates a new denommetadata keeper for testing -func NewTestDenommetadataKeeper(t *testing.T) (*keeper.Keeper, sdk.Context) { - app := utils.Setup(t, false) - k, ctx := testkeepers.NewTestDenommetadataKeeperFromApp(app) - return k, ctx +func NewTestDenommetadataKeeper(t *testing.T) (*app.App, sdk.Context) { + tapp := utils.Setup(t, false) + _, ctx := testkeepers.NewTestDenommetadataKeeperFromApp(tapp) + return tapp, ctx } diff --git a/x/hub-genesis/genesis_test.go b/x/hub-genesis/genesis_test.go index 49711bf3..81c73c27 100644 --- a/x/hub-genesis/genesis_test.go +++ b/x/hub-genesis/genesis_test.go @@ -1,13 +1,14 @@ package hub_genesis_test import ( + "testing" + testkeepers "github.com/dymensionxyz/dymension-rdk/testutil/keepers" "github.com/dymensionxyz/dymension-rdk/testutil/nullify" "github.com/dymensionxyz/dymension-rdk/testutil/utils" hub_genesis "github.com/dymensionxyz/dymension-rdk/x/hub-genesis" "github.com/dymensionxyz/dymension-rdk/x/hub-genesis/types" "github.com/stretchr/testify/require" - "testing" ) func TestGenesis(t *testing.T) {