-
Notifications
You must be signed in to change notification settings - Fork 586
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
feat(transfer)!: migrate transfer parameters to be self managed #3553
Merged
Merged
Changes from 107 commits
Commits
Show all changes
116 commits
Select commit
Hold shift + click to select a range
20e1c44
added 'UpdateParams' rpc endpoint and its messages to 'tx.proto'
srdtrk 1658827
ran 'make proto-gen'
srdtrk df392d6
implemented 'sdk.Msg' for 'MsgUpdateParams' in 'types/msgs.go'
srdtrk 964f97e
'msgs_test.go' updated - todo test tests + create a 'NewMsgUpdatePara…
srdtrk 369d399
added 'NewMsgUpdateParams' function to 'types/msgs.go'
srdtrk 2b0a739
improved test styling and used helper functions more
srdtrk c8383bb
added boilerplate 'UpdateParams' function to MsgServer - todo impleme…
srdtrk a53233f
removed paramtypes and added authority to Keeper struct - todo Keeper…
srdtrk 829c8a5
updated Keeper functions - todo add ParamsKey to types + update keepe…
srdtrk 5af4e23
deleted unneeded keeper/params.go as this should be managed in keeper.go
srdtrk 5accc5d
moved params test from params_test to keeper_test as this is where th…
srdtrk 0a950ef
added ParamsKey to 'types/keys.go'
srdtrk 3bfcbdb
replaced 'k.GetSendEnabled(ctx)' with 'k.GetParams(ctx).SendEnabled' …
srdtrk 6d5bf05
replaced 'k.GetReceiveEnabled(ctx)' with 'k.GetParams(ctx).ReceiveEna…
srdtrk 464c385
added 'MsgUpdateParams' to InterfaceRegistry
srdtrk 98be444
implemented 'UpdateParams' in msg_server
srdtrk 990e663
added one more test case to msgs_test for ParamsValidation
srdtrk 83c550c
wrote tests for 'TestUpdateParams' rpc handler
srdtrk 8ab5596
removed params subspace - todo: add subspaces back as legacy for migr…
srdtrk e27637a
removed test cases that didn't make sense
srdtrk 859c0bb
fixed the tests here
srdtrk 300600a
fixed typo in function comment
srdtrk 838c495
fixed keeper tests
srdtrk f6df036
merge with main
srdtrk 9dc8bc8
ran 'golangci-lint' on 'transfer/types/...'
srdtrk f07dab2
added validation to SetParams keeper method
srdtrk 637babc
handled the case where 'SetParams' returns an error 'InitGenesis'
srdtrk cc3b07e
fix: forgot to check value of '.SetParams' to ensure it is not an error
srdtrk ffbd6b2
ran 'gofumpt' on 'transfer/keeper/'
srdtrk c244ad2
split 'types/params.go' into a legacy part
srdtrk 63788a4
created 'exported.go' for migration purposes
srdtrk 5673df4
added migration code to 'keeper/migrations.go'
srdtrk e9317e9
increased module.go version and registered param migration
srdtrk b2018cf
passed legacySubspace to transfer for migration
srdtrk 54ef45d
allow tests to compile and pass by passing the subspace to the migrat…
srdtrk 12d93e3
removed unneeded if statement
srdtrk 0395da8
ran gofumpt
srdtrk 027ab9e
added integration tests for successful migration of params
srdtrk ba60fc5
fix: forgot to check if '.SetParams' returns an err
srdtrk 6ee83a7
ran gofumpt
srdtrk c93bf8d
broken: this test is work in progress and will fail
srdtrk ffcbd01
uncommented the two lines in test
srdtrk 5d8fcc8
fixed an important error on params_legacy.go
srdtrk e2c0a65
initialize ParamKeyTable for ibctransfer in app.go now
srdtrk 01637a7
finally a working version of the test
srdtrk 280bdfa
style changes on the test
srdtrk 1acf716
ran 'gofumpt'
srdtrk a52f1d0
moved comment
srdtrk 280b25d
updated GetParams function's comment
srdtrk cd5e83a
updated docs
srdtrk 9cdb411
Merge remote-tracking branch 'origin' into serdar/issue#3502-self-man…
srdtrk 2efb934
e2e: changed x/params query to fallback behavior
srdtrk 8e22f05
e2e: changed x/params query to fallback behavior
srdtrk 222df20
e2e: dropped the fallback behavior
srdtrk 8825b0d
e2e: trying to use the new gov proposal v1 to change params
srdtrk 50c8003
e2e: created 'isSelfManagingParams' function
srdtrk 86dae68
e2e: created fallback behaviour for param update gov proposal
srdtrk b8e6098
chore: e2e code cleanup
srdtrk 1b51575
chore: ran 'gofumpt' in e2e
srdtrk 7300d40
style(transfer/test): moved success case to top
srdtrk 50ea74f
style(transfer/test): replaced request with msg
srdtrk 383cd9d
refactor(transfer): moved MsgUpdateParams' registry to Interface Regi…
srdtrk a6b1859
style(transfer/test): ran gofumpt
srdtrk 0c6fbb7
style(transfer): moved UpdateParams function to the bottom of the file
srdtrk 557f431
imp(transfer): using 'ibcerrors.ErrInvalidAddress' instead of 'govtyp…
srdtrk 18b53f6
refactor(transfer/test): used ibctesting constants instead of definin…
srdtrk 296a140
feat(e2e/transfer): using semver to handle gov prop fallback instead …
srdtrk 0d5bd67
style(e2e/transfer): renamed 'selfParamFeatureReleases' to 'transferS…
srdtrk bd46565
merge: merge remote-tracking branch 'origin' into serdar/issue#3502-s…
srdtrk c81bedb
feat(transfer): keeping paramSpace in keeper and removing exported to…
srdtrk aa7866d
feat(simapp): reduced the modifications to app.go to only one
srdtrk 51cdced
fix(transfer/test): updated migration tests
srdtrk 40f63e2
imp(transfer/test): improved the efficiency and style of the test
srdtrk f560320
style(transfer): ran gofumpt
srdtrk 8cd90eb
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk cf03c5b
style(transfer): ParamsKey is now stored via a string rather than bytes
srdtrk 06cade8
imp(transfer): retired the old validate function, and new one makes m…
srdtrk a33610e
style(transfer): ran gofumpt
srdtrk 9beef82
refactor(transfer): removed the validate function for params as it cu…
srdtrk 485360e
fix(transfer): refactored to not use validate
srdtrk 4dcf47a
imp(transfer): GetParams now panics if params are not set
srdtrk e73f8c1
imp(transfer/test): added a new test case to test when Params are unset
srdtrk 3763e9c
style(transfer/test): updated the test messages
srdtrk 4fef125
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk c8fe227
docs(transfer): added tracking issue for removing params_legacy.go
srdtrk d6a0dfb
imp(transfer): added a new error type for invalid authority
srdtrk 3ba800d
style(e2e/transfer): fixed typo
srdtrk 98b8ba3
style(transfer/test): stopped using field names in test case declaration
srdtrk 764bedb
style(transfer/test): stopped using field names in test case declaration
srdtrk d6496bb
style(transfer/test): renamed test case
srdtrk 6466de8
imp(transfer): added more info to the error message
srdtrk 4d90658
style(e2e/transfer): updated ordering of deps
srdtrk 0d60d6a
imp(transfer/test): reduced the test size
srdtrk 7813b39
style(transfer): changed the err message for 'ErrInvalidAuthority'
srdtrk 29b0f93
imp(transfer): removed ErrInvalidAuthority
srdtrk e5f3960
style(transfer): using 'len(bz) == 0' instead of 'bz == nil'
srdtrk 6de4b5e
style(transfer): removed unneeded comment
srdtrk 948acf8
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk 4c163f0
imp(transfer): switched back to using 'bz == nil'
srdtrk a336e68
revert: '7813b39d7835eec26a3655bcd77587198ae90bea'
srdtrk be96b41
revert: "style(transfer): changed the err message for 'ErrInvalidAuth…
srdtrk bb83f08
revert: e5f396073d3e08fefe3507036b067a2b6af16cd7
srdtrk 33807c6
revert: 4c163f0e4a1c6853a1c7a3e0dc4cfad8c91ef282
srdtrk c672221
style(transfer): added code comment
srdtrk 2b425ce
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk b63e056
docs(migrations): added migration docs for changes in app.go
srdtrk 759c1ae
imp(transfer): improved ValidateBasic err message
srdtrk db60e4f
style(transfer/test): improved the styling of the test
srdtrk 3ad8090
style(transfer/test): improved the styling of the test
srdtrk 9aaf717
docs(transfer): added godoc for UpdateParams method
srdtrk 97c74b5
style(transfer): removed named return arg
srdtrk 18c35c8
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk 73fadd2
merge: remote-tracking branch 'origin' into serdar/issue#3502-self-ma…
srdtrk 305dc88
imp(e2e/transfer): reduced code duplication
srdtrk 7d66ac3
style(transfer/test): updated test case names
srdtrk 04b2591
imp(transfer/test): added whitespace test case
srdtrk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,14 @@ package transfer | |
|
||
import ( | ||
"context" | ||
"strconv" | ||
"testing" | ||
"time" | ||
|
||
"cosmossdk.io/math" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" | ||
paramsproposaltypes "github.com/cosmos/cosmos-sdk/x/params/types/proposal" | ||
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" | ||
|
||
"github.com/strangelove-ventures/interchaintest/v7/ibc" | ||
test "github.com/strangelove-ventures/interchaintest/v7/testutil" | ||
|
@@ -17,7 +18,6 @@ import ( | |
"github.com/cosmos/ibc-go/e2e/semverutil" | ||
"github.com/cosmos/ibc-go/e2e/testsuite" | ||
"github.com/cosmos/ibc-go/e2e/testvalues" | ||
transfertypes "github.com/cosmos/ibc-go/v7/modules/apps/transfer/types" | ||
ibctesting "github.com/cosmos/ibc-go/v7/testing" | ||
) | ||
|
||
|
@@ -29,34 +29,25 @@ type TransferTestSuite struct { | |
testsuite.E2ETestSuite | ||
} | ||
|
||
// transferSelfParamsFeatureReleases represents the releases the transfer module started managing its own params. | ||
var transferSelfParamsFeatureReleases = semverutil.FeatureReleases{ | ||
MajorVersion: "v8", | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. feature release matrices are being grouped together in test values in #3558, just noting this for resolving merge conflicts |
||
|
||
// QueryTransferSendEnabledParam queries the on-chain send enabled param for the transfer module | ||
func (s *TransferTestSuite) QueryTransferSendEnabledParam(ctx context.Context, chain ibc.Chain) bool { | ||
srdtrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
queryClient := s.GetChainGRCPClients(chain).ParamsQueryClient | ||
res, err := queryClient.Params(ctx, ¶msproposaltypes.QueryParamsRequest{ | ||
Subspace: transfertypes.StoreKey, | ||
Key: string(transfertypes.KeySendEnabled), | ||
}) | ||
s.Require().NoError(err) | ||
|
||
enabled, err := strconv.ParseBool(res.Param.Value) | ||
queryClient := s.GetChainGRCPClients(chain).TransferQueryClient | ||
res, err := queryClient.Params(ctx, &transfertypes.QueryParamsRequest{}) | ||
s.Require().NoError(err) | ||
|
||
return enabled | ||
return res.Params.SendEnabled | ||
} | ||
|
||
// QueryTransferReceiveEnabledParam queries the on-chain receive enabled param for the transfer module | ||
func (s *TransferTestSuite) QueryTransferReceiveEnabledParam(ctx context.Context, chain ibc.Chain) bool { | ||
queryClient := s.GetChainGRCPClients(chain).ParamsQueryClient | ||
res, err := queryClient.Params(ctx, ¶msproposaltypes.QueryParamsRequest{ | ||
Subspace: transfertypes.StoreKey, | ||
Key: string(transfertypes.KeyReceiveEnabled), | ||
}) | ||
s.Require().NoError(err) | ||
|
||
enabled, err := strconv.ParseBool(res.Param.Value) | ||
queryClient := s.GetChainGRCPClients(chain).TransferQueryClient | ||
res, err := queryClient.Params(ctx, &transfertypes.QueryParamsRequest{}) | ||
s.Require().NoError(err) | ||
|
||
return enabled | ||
return res.Params.ReceiveEnabled | ||
} | ||
|
||
// TestMsgTransfer_Succeeds_Nonincentivized will test sending successful IBC transfers from chainA to chainB. | ||
|
@@ -258,6 +249,13 @@ func (s *TransferTestSuite) TestSendEnabledParam() { | |
chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) | ||
chainBAddress := chainBWallet.FormattedAddress() | ||
|
||
chainAVersion := chainA.Config().Images[0].Version | ||
isSelfManagingParams := transferSelfParamsFeatureReleases.IsSupported(chainAVersion) | ||
|
||
govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA) | ||
s.Require().NoError(err) | ||
s.Require().NotNil(govModuleAddress) | ||
|
||
s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") | ||
|
||
t.Run("ensure transfer sending is enabled", func(t *testing.T) { | ||
|
@@ -271,12 +269,17 @@ func (s *TransferTestSuite) TestSendEnabledParam() { | |
}) | ||
|
||
t.Run("change send enabled parameter to disabled", func(t *testing.T) { | ||
changes := []paramsproposaltypes.ParamChange{ | ||
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeySendEnabled), "false"), | ||
} | ||
if isSelfManagingParams { | ||
srdtrk marked this conversation as resolved.
Show resolved
Hide resolved
|
||
msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, true)) | ||
s.ExecuteGovProposalV1(ctx, msg, chainA, chainAWallet, 1) | ||
} else { | ||
changes := []paramsproposaltypes.ParamChange{ | ||
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeySendEnabled), "false"), | ||
} | ||
|
||
proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) | ||
s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) | ||
proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) | ||
s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) | ||
} | ||
}) | ||
|
||
t.Run("ensure transfer params are disabled", func(t *testing.T) { | ||
|
@@ -309,6 +312,13 @@ func (s *TransferTestSuite) TestReceiveEnabledParam() { | |
chainBAddress = chainBWallet.FormattedAddress() | ||
) | ||
|
||
chainAVersion := chainA.Config().Images[0].Version | ||
isSelfManagingParams := transferSelfParamsFeatureReleases.IsSupported(chainAVersion) | ||
|
||
govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA) | ||
s.Require().NoError(err) | ||
s.Require().NotNil(govModuleAddress) | ||
|
||
s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks") | ||
|
||
t.Run("ensure transfer receive is enabled", func(t *testing.T) { | ||
|
@@ -350,12 +360,17 @@ func (s *TransferTestSuite) TestReceiveEnabledParam() { | |
}) | ||
|
||
t.Run("change receive enabled parameter to disabled ", func(t *testing.T) { | ||
changes := []paramsproposaltypes.ParamChange{ | ||
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeyReceiveEnabled), "false"), | ||
} | ||
if isSelfManagingParams { | ||
msg := transfertypes.NewMsgUpdateParams(govModuleAddress.String(), transfertypes.NewParams(false, false)) | ||
s.ExecuteGovProposalV1(ctx, msg, chainA, chainAWallet, 1) | ||
} else { | ||
changes := []paramsproposaltypes.ParamChange{ | ||
paramsproposaltypes.NewParamChange(transfertypes.StoreKey, string(transfertypes.KeyReceiveEnabled), "false"), | ||
} | ||
|
||
proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) | ||
s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) | ||
proposal := paramsproposaltypes.NewParameterChangeProposal(ibctesting.Title, ibctesting.Description, changes) | ||
s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal) | ||
} | ||
}) | ||
|
||
t.Run("ensure transfer params are disabled", func(t *testing.T) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add this a
Client
section like we have for ICA? I am also adding one for transfer in this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In another docs related PR?