From b75d2c713453c3176c435402b5d55f8af7feb196 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Tue, 18 Apr 2023 11:00:10 +0200 Subject: [PATCH 01/15] Add warnings when provider unbonding is shorter than consumer unbonding --- x/ccv/provider/client/proposal_handler.go | 47 +++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 8be7f0fb56..0aa659fd68 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -1,10 +1,12 @@ package client import ( + "context" "encoding/json" "fmt" "io/ioutil" "net/http" + "os" "path/filepath" "time" @@ -74,6 +76,16 @@ Where proposal.json contains: return err } + err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) + if err != nil { + // do not fail for errors regarding the unbonding period, but just log a warning + fmt.Fprintf( + os.Stderr, + "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", + err.Error(), + ) + } + content := types.NewConsumerAdditionProposal( proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime, @@ -367,6 +379,16 @@ func postConsumerAdditionProposalHandlerFn(clientCtx client.Context) http.Handle return } + err := CheckPropUnbondingPeriod(clientCtx, req.UnbondingPeriod) + if err != nil { + // do not fail for errors regarding the unbonding period, but just log a warning + fmt.Fprintf( + os.Stderr, + "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", + err.Error(), + ) + } + content := types.NewConsumerAdditionProposal( req.Title, req.Description, req.ChainId, req.InitialHeight, req.GenesisHash, req.BinaryHash, req.SpawnTime, @@ -441,3 +463,28 @@ func postEquivocationProposalHandlerFn(clientCtx client.Context) http.HandlerFun tx.WriteGeneratedTxResponse(clientCtx, w, req.BaseReq, msg) } } + +func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time.Duration) error { + client, err := clientCtx.GetNode() + + if err != nil { + return err + } + + consensusParams, err := client.ConsensusParams(context.Background(), &clientCtx.Height) + + if err != nil { + return err + } + + providerUnbondingTime := consensusParams.ConsensusParams.Evidence.MaxAgeDuration + + if providerUnbondingTime <= propUnbondingPeriod { + return fmt.Errorf( + "consumer unbonding period should be smaller than provider unbonding period, but is longer: \n consumer unbonding: %s \n provider unbonding: %s", + propUnbondingPeriod, + providerUnbondingTime) + } + + return nil +} From 248a9d1b62e5243f972fbe65a59f4b897530c9cc Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Tue, 18 Apr 2023 11:07:18 +0200 Subject: [PATCH 02/15] Add a link to the onboarding documentation to the unbonding period warning --- x/ccv/provider/client/proposal_handler.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 0aa659fd68..4f9b8fd782 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -481,7 +481,10 @@ func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time if providerUnbondingTime <= propUnbondingPeriod { return fmt.Errorf( - "consumer unbonding period should be smaller than provider unbonding period, but is longer: \n consumer unbonding: %s \n provider unbonding: %s", + "consumer unbonding period should be smaller than provider unbonding period, but is longer: \n"+ + "consumer unbonding: %s \n provider unbonding: %s \n"+ + "See the Consumer onboarding documentation for more information:\n"+ + "https://cosmos.github.io/interchain-security/consumer-development/onboarding#3-submit-a-governance-proposal", propUnbondingPeriod, providerUnbondingTime) } From 9d8d5c76615c91536467b6f02edc21219da34265 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Tue, 18 Apr 2023 11:25:05 +0200 Subject: [PATCH 03/15] Gofumpt proposal handler --- x/ccv/provider/client/proposal_handler.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 4f9b8fd782..7cc86eab60 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -466,13 +466,11 @@ func postEquivocationProposalHandlerFn(clientCtx client.Context) http.HandlerFun func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time.Duration) error { client, err := clientCtx.GetNode() - if err != nil { return err } consensusParams, err := client.ConsensusParams(context.Background(), &clientCtx.Height) - if err != nil { return err } From b95ce62b1d5589c47a0efb0d07b775afe6275777 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 20 Apr 2023 14:10:57 +0200 Subject: [PATCH 04/15] Add testing with mocked out rpc client --- go.mod | 1 + testutil/mock_rpc_client.go | 248 ++++++++++++++++++ x/ccv/provider/client/proposal_handler.go | 64 ++--- .../provider/client/proposal_handler_test.go | 74 ++++++ 4 files changed, 357 insertions(+), 30 deletions(-) create mode 100644 testutil/mock_rpc_client.go create mode 100644 x/ccv/provider/client/proposal_handler_test.go diff --git a/go.mod b/go.mod index c75f56a6b8..e73b1e981a 100644 --- a/go.mod +++ b/go.mod @@ -133,6 +133,7 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.14.0 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.4.1 // indirect github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 // indirect github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c // indirect diff --git a/testutil/mock_rpc_client.go b/testutil/mock_rpc_client.go new file mode 100644 index 0000000000..f5900582d4 --- /dev/null +++ b/testutil/mock_rpc_client.go @@ -0,0 +1,248 @@ +package testutil + +import ( + "context" + + "github.com/stretchr/testify/mock" + "github.com/tendermint/tendermint/libs/bytes" + "github.com/tendermint/tendermint/libs/log" + rpcClient "github.com/tendermint/tendermint/rpc/client" + ctypes "github.com/tendermint/tendermint/rpc/core/types" + "github.com/tendermint/tendermint/types" +) + +type MockClient struct { + mock.Mock +} + +// IsRunning implements client.Client +func (m MockClient) IsRunning() bool { + args := m.Called() + return args.Bool(0) +} + +// OnReset implements client.Client +func (m MockClient) OnReset() error { + args := m.Called() + return args.Error(0) +} + +// OnStart implements client.Client +func (m MockClient) OnStart() error { + args := m.Called() + return args.Error(0) +} + +// OnStop implements client.Client +func (m MockClient) OnStop() { + m.Called() +} + +// Quit implements client.Client +func (m MockClient) Quit() <-chan struct{} { + args := m.Called() + return args.Get(0).(<-chan struct{}) +} + +// Reset implements client.Client +func (m MockClient) Reset() error { + args := m.Called() + return args.Error(0) +} + +// SetLogger implements client.Client +func (m MockClient) SetLogger(logger log.Logger) { + m.Called(logger) +} + +// Start implements client.Client +func (m MockClient) Start() error { + args := m.Called() + return args.Error(0) +} + +// Stop implements client.Client +func (m MockClient) Stop() error { + args := m.Called() + return args.Error(0) +} + +// ABCIInfo implements client.Client +func (m MockClient) ABCIInfo(ctx context.Context) (*ctypes.ResultABCIInfo, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultABCIInfo), args.Error(1) +} + +// ABCIQuery implements client.Client +func (m MockClient) ABCIQuery(ctx context.Context, path string, data bytes.HexBytes) (*ctypes.ResultABCIQuery, error) { + args := m.Called(ctx, path, data) + return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) +} + +// ABCIQueryWithOptions implements client.Client +func (m MockClient) ABCIQueryWithOptions(ctx context.Context, path string, data bytes.HexBytes, opts rpcClient.ABCIQueryOptions) (*ctypes.ResultABCIQuery, error) { + args := m.Called(ctx, path, data, opts) + return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) +} + +// BroadcastTxAsync implements client.Client +func (m MockClient) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { + args := m.Called(ctx, tx) + return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) +} + +// BroadcastTxCommit implements client.Client +func (m MockClient) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) { + args := m.Called(ctx, tx) + return args.Get(0).(*ctypes.ResultBroadcastTxCommit), args.Error(1) +} + +// BroadcastTxSync implements client.Client +func (m MockClient) BroadcastTxSync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { + args := m.Called(ctx, tx) + return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) +} + +// Subscribe implements client.Client +func (m MockClient) Subscribe(ctx context.Context, subscriber string, query string, outCapacity ...int) (<-chan ctypes.ResultEvent, error) { + args := m.Called(ctx, subscriber, query, outCapacity) + return args.Get(0).(<-chan ctypes.ResultEvent), args.Error(1) +} + +// Unsubscribe implements client.Client +func (m *MockClient) Unsubscribe(ctx context.Context, subscriber string, query string) error { + args := m.Called(ctx, subscriber, query) + return args.Error(0) +} + +// UnsubscribeAll implements client.Client +func (m *MockClient) UnsubscribeAll(ctx context.Context, subscriber string) error { + args := m.Called(ctx, subscriber) + return args.Error(0) +} + +// BlockchainInfo implements client.Client +func (m *MockClient) BlockchainInfo(ctx context.Context, minHeight int64, maxHeight int64) (*ctypes.ResultBlockchainInfo, error) { + args := m.Called(ctx, minHeight, maxHeight) + return args.Get(0).(*ctypes.ResultBlockchainInfo), args.Error(1) +} + +// Genesis implements client.Client +func (m *MockClient) Genesis(ctx context.Context) (*ctypes.ResultGenesis, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultGenesis), args.Error(1) +} + +// GenesisChunked implements client.Client +func (m *MockClient) GenesisChunked(ctx context.Context, chunkSize uint) (*ctypes.ResultGenesisChunk, error) { + args := m.Called(ctx, chunkSize) + return args.Get(0).(*ctypes.ResultGenesisChunk), args.Error(1) +} + +// ConsensusParams implements client.Client +func (m *MockClient) ConsensusParams(ctx context.Context, height *int64) (*ctypes.ResultConsensusParams, error) { + args := m.Called(ctx, height) + return args.Get(0).(*ctypes.ResultConsensusParams), args.Error(1) +} + +// ConsensusState implements client.Client +func (m MockClient) ConsensusState(ctx context.Context) (*ctypes.ResultConsensusState, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultConsensusState), args.Error(1) +} + +// DumpConsensusState implements client.Client +func (m MockClient) DumpConsensusState(ctx context.Context) (*ctypes.ResultDumpConsensusState, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultDumpConsensusState), args.Error(1) +} + +// Health implements client.Client +func (m MockClient) Health(ctx context.Context) (*ctypes.ResultHealth, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultHealth), args.Error(1) +} + +// NetInfo implements client.Client +func (m MockClient) NetInfo(ctx context.Context) (*ctypes.ResultNetInfo, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultNetInfo), args.Error(1) +} + +// Block implements client.Client +func (m MockClient) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock, error) { + args := m.Called(ctx, height) + return args.Get(0).(*ctypes.ResultBlock), args.Error(1) +} + +// BlockByHash implements client.Client +func (m MockClient) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) { + args := m.Called(ctx, hash) + return args.Get(0).(*ctypes.ResultBlock), args.Error(1) +} + +// BlockResults implements client.Client +func (m MockClient) BlockResults(ctx context.Context, height *int64) (*ctypes.ResultBlockResults, error) { + args := m.Called(ctx, height) + return args.Get(0).(*ctypes.ResultBlockResults), args.Error(1) +} + +// BlockSearch implements client.Client +func (m MockClient) BlockSearch(ctx context.Context, query string, page *int, perPage *int, orderBy string) (*ctypes.ResultBlockSearch, error) { + args := m.Called(ctx, query, page, perPage, orderBy) + return args.Get(0).(*ctypes.ResultBlockSearch), args.Error(1) +} + +// Commit implements client.Client +func (m MockClient) Commit(ctx context.Context, height *int64) (*ctypes.ResultCommit, error) { + args := m.Called(ctx, height) + return args.Get(0).(*ctypes.ResultCommit), args.Error(1) +} + +// Tx implements client.Client +func (m MockClient) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) { + args := m.Called(ctx, hash, prove) + return args.Get(0).(*ctypes.ResultTx), args.Error(1) +} + +// TxSearch implements client.Client +func (m MockClient) TxSearch(ctx context.Context, query string, prove bool, page *int, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) { + args := m.Called(ctx, query, prove, page, perPage, orderBy) + return args.Get(0).(*ctypes.ResultTxSearch), args.Error(1) +} + +// Validators implements client.Client +func (m MockClient) Validators(ctx context.Context, height *int64, page *int, perPage *int) (*ctypes.ResultValidators, error) { + args := m.Called(ctx, height, page, perPage) + return args.Get(0).(*ctypes.ResultValidators), args.Error(1) +} + +// Status implements client.Client +func (m *MockClient) Status(ctx context.Context) (*ctypes.ResultStatus, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultStatus), args.Error(1) +} + +// BroadcastEvidence implements client.Client +func (m *MockClient) BroadcastEvidence(ctx context.Context, ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) { + args := m.Called(ctx, ev) + return args.Get(0).(*ctypes.ResultBroadcastEvidence), args.Error(1) +} + +// CheckTx implements client.Client +func (m *MockClient) CheckTx(ctx context.Context, tx types.Tx) (*ctypes.ResultCheckTx, error) { + args := m.Called(ctx, tx) + return args.Get(0).(*ctypes.ResultCheckTx), args.Error(1) +} + +// NumUnconfirmedTxs implements client.Client +func (m *MockClient) NumUnconfirmedTxs(ctx context.Context) (*ctypes.ResultUnconfirmedTxs, error) { + args := m.Called(ctx) + return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) +} + +// UnconfirmedTxs implements client.Client +func (m *MockClient) UnconfirmedTxs(ctx context.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) { + args := m.Called(ctx, limit) + return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) +} diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 7cc86eab60..d58e484098 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -71,42 +71,46 @@ Where proposal.json contains: return err } - proposal, err := ParseConsumerAdditionProposalJSON(args[0]) - if err != nil { - return err - } + return ExecConsumerAdditionCmdWithClientCtx(cmd, args, clientCtx) + }, + } +} - err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) - if err != nil { - // do not fail for errors regarding the unbonding period, but just log a warning - fmt.Fprintf( - os.Stderr, - "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", - err.Error(), - ) - } +func ExecConsumerAdditionCmdWithClientCtx(cmd *cobra.Command, args []string, clientCtx client.Context) error { + proposal, err := ParseConsumerAdditionProposalJSON(args[0]) + if err != nil { + return err + } - content := types.NewConsumerAdditionProposal( - proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, - proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime, - proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission, proposal.HistoricalEntries, - proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod) + // do not fail for errors regarding the unbonding period, but just log a warning + err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) + if err != nil { + fmt.Fprintf( + os.Stderr, + "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", + err.Error(), + ) + } - from := clientCtx.GetFromAddress() + content := types.NewConsumerAdditionProposal( + proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, + proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime, + proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission, proposal.HistoricalEntries, + proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod) - deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit) - if err != nil { - return err - } + from := clientCtx.GetFromAddress() - msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) - if err != nil { - return err - } + deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit) + if err != nil { + return err + } - return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) - }, + msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) + if err != nil { + return err } + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) } // SubmitConsumerRemovalPropTxCmd returns a CLI command handler for submitting @@ -470,7 +474,7 @@ func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time return err } - consensusParams, err := client.ConsensusParams(context.Background(), &clientCtx.Height) + consensusParams, err := client.ConsensusParams(context.Background(), nil) if err != nil { return err } diff --git a/x/ccv/provider/client/proposal_handler_test.go b/x/ccv/provider/client/proposal_handler_test.go new file mode 100644 index 0000000000..f8f50f27b1 --- /dev/null +++ b/x/ccv/provider/client/proposal_handler_test.go @@ -0,0 +1,74 @@ +package client_test + +import ( + "context" + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/client" + "github.com/cosmos/interchain-security/testutil" + providerClient "github.com/cosmos/interchain-security/x/ccv/provider/client" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + cmtproto "github.com/tendermint/tendermint/proto/tendermint/types" + coretypes "github.com/tendermint/tendermint/rpc/core/types" +) + +func ConsumerAdditionProposalCLIDriver(providerUnbondingTime, consumerUnbondingTime time.Duration) error { + mockClient := new(testutil.MockClient) + + resConsensParams := coretypes.ResultConsensusParams{ + BlockHeight: 5, + ConsensusParams: cmtproto.ConsensusParams{ + Evidence: cmtproto.EvidenceParams{ + MaxAgeDuration: providerUnbondingTime, + }, + }, + } + mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) + clientCtx := client.Context{}.WithClient(mockClient) + res, _ := mockClient.ConsensusParams(context.Background(), nil) + + if res != &resConsensParams { + panic("What") + } + + return providerClient.CheckPropUnbondingPeriod(clientCtx, consumerUnbondingTime) +} + +func TestConsumerAdditionProposalSuite(t *testing.T) { + testCases := []struct { + name string + providerUnbonding time.Duration + consumerUnbonding time.Duration + errorExpected bool + }{ + { + name: "provider unbonding longer", + providerUnbonding: time.Hour * 5, + consumerUnbonding: time.Hour * 4, + errorExpected: false, + }, + { + name: "consumer unbonding longer", + providerUnbonding: time.Hour * 5, + consumerUnbonding: time.Hour * 6, + errorExpected: true, + }, + { + name: "unbondings equal", + providerUnbonding: time.Hour * 5, + consumerUnbonding: time.Hour * 5, + errorExpected: true, + }, + } + + for _, tc := range testCases { + res := ConsumerAdditionProposalCLIDriver(tc.providerUnbonding, tc.consumerUnbonding) + if tc.errorExpected { + require.Error(t, res) + } else { + require.NoError(t, res) + } + } +} From 2ae197f6a694eb902393ba3a9f02620a9d80710f Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 20 Apr 2023 14:26:57 +0200 Subject: [PATCH 05/15] Remove debug output --- x/ccv/provider/client/proposal_handler_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler_test.go b/x/ccv/provider/client/proposal_handler_test.go index f8f50f27b1..3b93de4a39 100644 --- a/x/ccv/provider/client/proposal_handler_test.go +++ b/x/ccv/provider/client/proposal_handler_test.go @@ -1,7 +1,6 @@ package client_test import ( - "context" "testing" "time" @@ -27,11 +26,6 @@ func ConsumerAdditionProposalCLIDriver(providerUnbondingTime, consumerUnbondingT } mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) clientCtx := client.Context{}.WithClient(mockClient) - res, _ := mockClient.ConsensusParams(context.Background(), nil) - - if res != &resConsensParams { - panic("What") - } return providerClient.CheckPropUnbondingPeriod(clientCtx, consumerUnbondingTime) } From 543f1dee5818564423d167ebd5ceb32e2b7a2a50 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 20 Apr 2023 14:28:42 +0200 Subject: [PATCH 06/15] Add docstring --- testutil/mock_rpc_client.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testutil/mock_rpc_client.go b/testutil/mock_rpc_client.go index f5900582d4..abafafca43 100644 --- a/testutil/mock_rpc_client.go +++ b/testutil/mock_rpc_client.go @@ -11,6 +11,14 @@ import ( "github.com/tendermint/tendermint/types" ) +// A mock client to be used in tests that want to mock out calls to the CometBFT CLI. +// The client implements the interface "github.com/tendermint/tendermint/rpc/client".Client +// Example Usage: +// +// mockClient := new(testutil.MockClient) +// clientCtx := client.Context{}.WithClient(mockClient) +// mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) +// {code using clientCtx} type MockClient struct { mock.Mock } From 36d566e5dc77b90fb1f5e6ef62dd7e6cda34eb51 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 20 Apr 2023 17:14:00 +0200 Subject: [PATCH 07/15] Remove warning from REST handler --- x/ccv/provider/client/proposal_handler.go | 72 +++++++++-------------- 1 file changed, 29 insertions(+), 43 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index d58e484098..7db692229a 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -71,46 +71,42 @@ Where proposal.json contains: return err } - return ExecConsumerAdditionCmdWithClientCtx(cmd, args, clientCtx) - }, - } -} + proposal, err := ParseConsumerAdditionProposalJSON(args[0]) + if err != nil { + return err + } -func ExecConsumerAdditionCmdWithClientCtx(cmd *cobra.Command, args []string, clientCtx client.Context) error { - proposal, err := ParseConsumerAdditionProposalJSON(args[0]) - if err != nil { - return err - } + // do not fail for errors regarding the unbonding period, but just log a warning + err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) + if err != nil { + fmt.Fprintf( + os.Stderr, + "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", + err.Error(), + ) + } - // do not fail for errors regarding the unbonding period, but just log a warning - err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) - if err != nil { - fmt.Fprintf( - os.Stderr, - "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", - err.Error(), - ) - } + content := types.NewConsumerAdditionProposal( + proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, + proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime, + proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission, proposal.HistoricalEntries, + proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod) - content := types.NewConsumerAdditionProposal( - proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, - proposal.GenesisHash, proposal.BinaryHash, proposal.SpawnTime, - proposal.ConsumerRedistributionFraction, proposal.BlocksPerDistributionTransmission, proposal.HistoricalEntries, - proposal.CcvTimeoutPeriod, proposal.TransferTimeoutPeriod, proposal.UnbondingPeriod) + from := clientCtx.GetFromAddress() - from := clientCtx.GetFromAddress() + deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit) + if err != nil { + return err + } - deposit, err := sdk.ParseCoinsNormalized(proposal.Deposit) - if err != nil { - return err - } + msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) + if err != nil { + return err + } - msg, err := govtypes.NewMsgSubmitProposal(content, deposit, from) - if err != nil { - return err + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) + }, } - - return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg) } // SubmitConsumerRemovalPropTxCmd returns a CLI command handler for submitting @@ -383,16 +379,6 @@ func postConsumerAdditionProposalHandlerFn(clientCtx client.Context) http.Handle return } - err := CheckPropUnbondingPeriod(clientCtx, req.UnbondingPeriod) - if err != nil { - // do not fail for errors regarding the unbonding period, but just log a warning - fmt.Fprintf( - os.Stderr, - "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", - err.Error(), - ) - } - content := types.NewConsumerAdditionProposal( req.Title, req.Description, req.ChainId, req.InitialHeight, req.GenesisHash, req.BinaryHash, req.SpawnTime, From 6a75fb50d76e916fdeadae58ea712f5d1fe167bd Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Thu, 20 Apr 2023 18:31:28 +0200 Subject: [PATCH 08/15] Rename MockClient for clarity --- testutil/mock_rpc_client.go | 82 +++++++++---------- .../provider/client/proposal_handler_test.go | 2 +- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/testutil/mock_rpc_client.go b/testutil/mock_rpc_client.go index abafafca43..bb324f1cf1 100644 --- a/testutil/mock_rpc_client.go +++ b/testutil/mock_rpc_client.go @@ -15,242 +15,242 @@ import ( // The client implements the interface "github.com/tendermint/tendermint/rpc/client".Client // Example Usage: // -// mockClient := new(testutil.MockClient) +// mockClient := new(testutil.MockCometBFTRPCClient) // clientCtx := client.Context{}.WithClient(mockClient) // mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) // {code using clientCtx} -type MockClient struct { +type MockCometBFTRPCClient struct { mock.Mock } // IsRunning implements client.Client -func (m MockClient) IsRunning() bool { +func (m MockCometBFTRPCClient) IsRunning() bool { args := m.Called() return args.Bool(0) } // OnReset implements client.Client -func (m MockClient) OnReset() error { +func (m MockCometBFTRPCClient) OnReset() error { args := m.Called() return args.Error(0) } // OnStart implements client.Client -func (m MockClient) OnStart() error { +func (m MockCometBFTRPCClient) OnStart() error { args := m.Called() return args.Error(0) } // OnStop implements client.Client -func (m MockClient) OnStop() { +func (m MockCometBFTRPCClient) OnStop() { m.Called() } // Quit implements client.Client -func (m MockClient) Quit() <-chan struct{} { +func (m MockCometBFTRPCClient) Quit() <-chan struct{} { args := m.Called() return args.Get(0).(<-chan struct{}) } // Reset implements client.Client -func (m MockClient) Reset() error { +func (m MockCometBFTRPCClient) Reset() error { args := m.Called() return args.Error(0) } // SetLogger implements client.Client -func (m MockClient) SetLogger(logger log.Logger) { +func (m MockCometBFTRPCClient) SetLogger(logger log.Logger) { m.Called(logger) } // Start implements client.Client -func (m MockClient) Start() error { +func (m MockCometBFTRPCClient) Start() error { args := m.Called() return args.Error(0) } // Stop implements client.Client -func (m MockClient) Stop() error { +func (m MockCometBFTRPCClient) Stop() error { args := m.Called() return args.Error(0) } // ABCIInfo implements client.Client -func (m MockClient) ABCIInfo(ctx context.Context) (*ctypes.ResultABCIInfo, error) { +func (m MockCometBFTRPCClient) ABCIInfo(ctx context.Context) (*ctypes.ResultABCIInfo, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultABCIInfo), args.Error(1) } // ABCIQuery implements client.Client -func (m MockClient) ABCIQuery(ctx context.Context, path string, data bytes.HexBytes) (*ctypes.ResultABCIQuery, error) { +func (m MockCometBFTRPCClient) ABCIQuery(ctx context.Context, path string, data bytes.HexBytes) (*ctypes.ResultABCIQuery, error) { args := m.Called(ctx, path, data) return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) } // ABCIQueryWithOptions implements client.Client -func (m MockClient) ABCIQueryWithOptions(ctx context.Context, path string, data bytes.HexBytes, opts rpcClient.ABCIQueryOptions) (*ctypes.ResultABCIQuery, error) { +func (m MockCometBFTRPCClient) ABCIQueryWithOptions(ctx context.Context, path string, data bytes.HexBytes, opts rpcClient.ABCIQueryOptions) (*ctypes.ResultABCIQuery, error) { args := m.Called(ctx, path, data, opts) return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) } // BroadcastTxAsync implements client.Client -func (m MockClient) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { +func (m MockCometBFTRPCClient) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { args := m.Called(ctx, tx) return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) } // BroadcastTxCommit implements client.Client -func (m MockClient) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) { +func (m MockCometBFTRPCClient) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) { args := m.Called(ctx, tx) return args.Get(0).(*ctypes.ResultBroadcastTxCommit), args.Error(1) } // BroadcastTxSync implements client.Client -func (m MockClient) BroadcastTxSync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { +func (m MockCometBFTRPCClient) BroadcastTxSync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { args := m.Called(ctx, tx) return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) } // Subscribe implements client.Client -func (m MockClient) Subscribe(ctx context.Context, subscriber string, query string, outCapacity ...int) (<-chan ctypes.ResultEvent, error) { +func (m MockCometBFTRPCClient) Subscribe(ctx context.Context, subscriber string, query string, outCapacity ...int) (<-chan ctypes.ResultEvent, error) { args := m.Called(ctx, subscriber, query, outCapacity) return args.Get(0).(<-chan ctypes.ResultEvent), args.Error(1) } // Unsubscribe implements client.Client -func (m *MockClient) Unsubscribe(ctx context.Context, subscriber string, query string) error { +func (m *MockCometBFTRPCClient) Unsubscribe(ctx context.Context, subscriber string, query string) error { args := m.Called(ctx, subscriber, query) return args.Error(0) } // UnsubscribeAll implements client.Client -func (m *MockClient) UnsubscribeAll(ctx context.Context, subscriber string) error { +func (m *MockCometBFTRPCClient) UnsubscribeAll(ctx context.Context, subscriber string) error { args := m.Called(ctx, subscriber) return args.Error(0) } // BlockchainInfo implements client.Client -func (m *MockClient) BlockchainInfo(ctx context.Context, minHeight int64, maxHeight int64) (*ctypes.ResultBlockchainInfo, error) { +func (m *MockCometBFTRPCClient) BlockchainInfo(ctx context.Context, minHeight int64, maxHeight int64) (*ctypes.ResultBlockchainInfo, error) { args := m.Called(ctx, minHeight, maxHeight) return args.Get(0).(*ctypes.ResultBlockchainInfo), args.Error(1) } // Genesis implements client.Client -func (m *MockClient) Genesis(ctx context.Context) (*ctypes.ResultGenesis, error) { +func (m *MockCometBFTRPCClient) Genesis(ctx context.Context) (*ctypes.ResultGenesis, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultGenesis), args.Error(1) } // GenesisChunked implements client.Client -func (m *MockClient) GenesisChunked(ctx context.Context, chunkSize uint) (*ctypes.ResultGenesisChunk, error) { +func (m *MockCometBFTRPCClient) GenesisChunked(ctx context.Context, chunkSize uint) (*ctypes.ResultGenesisChunk, error) { args := m.Called(ctx, chunkSize) return args.Get(0).(*ctypes.ResultGenesisChunk), args.Error(1) } // ConsensusParams implements client.Client -func (m *MockClient) ConsensusParams(ctx context.Context, height *int64) (*ctypes.ResultConsensusParams, error) { +func (m *MockCometBFTRPCClient) ConsensusParams(ctx context.Context, height *int64) (*ctypes.ResultConsensusParams, error) { args := m.Called(ctx, height) return args.Get(0).(*ctypes.ResultConsensusParams), args.Error(1) } // ConsensusState implements client.Client -func (m MockClient) ConsensusState(ctx context.Context) (*ctypes.ResultConsensusState, error) { +func (m MockCometBFTRPCClient) ConsensusState(ctx context.Context) (*ctypes.ResultConsensusState, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultConsensusState), args.Error(1) } // DumpConsensusState implements client.Client -func (m MockClient) DumpConsensusState(ctx context.Context) (*ctypes.ResultDumpConsensusState, error) { +func (m MockCometBFTRPCClient) DumpConsensusState(ctx context.Context) (*ctypes.ResultDumpConsensusState, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultDumpConsensusState), args.Error(1) } // Health implements client.Client -func (m MockClient) Health(ctx context.Context) (*ctypes.ResultHealth, error) { +func (m MockCometBFTRPCClient) Health(ctx context.Context) (*ctypes.ResultHealth, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultHealth), args.Error(1) } // NetInfo implements client.Client -func (m MockClient) NetInfo(ctx context.Context) (*ctypes.ResultNetInfo, error) { +func (m MockCometBFTRPCClient) NetInfo(ctx context.Context) (*ctypes.ResultNetInfo, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultNetInfo), args.Error(1) } // Block implements client.Client -func (m MockClient) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock, error) { +func (m MockCometBFTRPCClient) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock, error) { args := m.Called(ctx, height) return args.Get(0).(*ctypes.ResultBlock), args.Error(1) } // BlockByHash implements client.Client -func (m MockClient) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) { +func (m MockCometBFTRPCClient) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) { args := m.Called(ctx, hash) return args.Get(0).(*ctypes.ResultBlock), args.Error(1) } // BlockResults implements client.Client -func (m MockClient) BlockResults(ctx context.Context, height *int64) (*ctypes.ResultBlockResults, error) { +func (m MockCometBFTRPCClient) BlockResults(ctx context.Context, height *int64) (*ctypes.ResultBlockResults, error) { args := m.Called(ctx, height) return args.Get(0).(*ctypes.ResultBlockResults), args.Error(1) } // BlockSearch implements client.Client -func (m MockClient) BlockSearch(ctx context.Context, query string, page *int, perPage *int, orderBy string) (*ctypes.ResultBlockSearch, error) { +func (m MockCometBFTRPCClient) BlockSearch(ctx context.Context, query string, page *int, perPage *int, orderBy string) (*ctypes.ResultBlockSearch, error) { args := m.Called(ctx, query, page, perPage, orderBy) return args.Get(0).(*ctypes.ResultBlockSearch), args.Error(1) } // Commit implements client.Client -func (m MockClient) Commit(ctx context.Context, height *int64) (*ctypes.ResultCommit, error) { +func (m MockCometBFTRPCClient) Commit(ctx context.Context, height *int64) (*ctypes.ResultCommit, error) { args := m.Called(ctx, height) return args.Get(0).(*ctypes.ResultCommit), args.Error(1) } // Tx implements client.Client -func (m MockClient) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) { +func (m MockCometBFTRPCClient) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) { args := m.Called(ctx, hash, prove) return args.Get(0).(*ctypes.ResultTx), args.Error(1) } // TxSearch implements client.Client -func (m MockClient) TxSearch(ctx context.Context, query string, prove bool, page *int, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) { +func (m MockCometBFTRPCClient) TxSearch(ctx context.Context, query string, prove bool, page *int, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) { args := m.Called(ctx, query, prove, page, perPage, orderBy) return args.Get(0).(*ctypes.ResultTxSearch), args.Error(1) } // Validators implements client.Client -func (m MockClient) Validators(ctx context.Context, height *int64, page *int, perPage *int) (*ctypes.ResultValidators, error) { +func (m MockCometBFTRPCClient) Validators(ctx context.Context, height *int64, page *int, perPage *int) (*ctypes.ResultValidators, error) { args := m.Called(ctx, height, page, perPage) return args.Get(0).(*ctypes.ResultValidators), args.Error(1) } // Status implements client.Client -func (m *MockClient) Status(ctx context.Context) (*ctypes.ResultStatus, error) { +func (m *MockCometBFTRPCClient) Status(ctx context.Context) (*ctypes.ResultStatus, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultStatus), args.Error(1) } // BroadcastEvidence implements client.Client -func (m *MockClient) BroadcastEvidence(ctx context.Context, ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) { +func (m *MockCometBFTRPCClient) BroadcastEvidence(ctx context.Context, ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) { args := m.Called(ctx, ev) return args.Get(0).(*ctypes.ResultBroadcastEvidence), args.Error(1) } // CheckTx implements client.Client -func (m *MockClient) CheckTx(ctx context.Context, tx types.Tx) (*ctypes.ResultCheckTx, error) { +func (m *MockCometBFTRPCClient) CheckTx(ctx context.Context, tx types.Tx) (*ctypes.ResultCheckTx, error) { args := m.Called(ctx, tx) return args.Get(0).(*ctypes.ResultCheckTx), args.Error(1) } // NumUnconfirmedTxs implements client.Client -func (m *MockClient) NumUnconfirmedTxs(ctx context.Context) (*ctypes.ResultUnconfirmedTxs, error) { +func (m *MockCometBFTRPCClient) NumUnconfirmedTxs(ctx context.Context) (*ctypes.ResultUnconfirmedTxs, error) { args := m.Called(ctx) return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) } // UnconfirmedTxs implements client.Client -func (m *MockClient) UnconfirmedTxs(ctx context.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) { +func (m *MockCometBFTRPCClient) UnconfirmedTxs(ctx context.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) { args := m.Called(ctx, limit) return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) } diff --git a/x/ccv/provider/client/proposal_handler_test.go b/x/ccv/provider/client/proposal_handler_test.go index 3b93de4a39..1e45652253 100644 --- a/x/ccv/provider/client/proposal_handler_test.go +++ b/x/ccv/provider/client/proposal_handler_test.go @@ -14,7 +14,7 @@ import ( ) func ConsumerAdditionProposalCLIDriver(providerUnbondingTime, consumerUnbondingTime time.Duration) error { - mockClient := new(testutil.MockClient) + mockClient := new(testutil.MockCometBFTRPCClient) resConsensParams := coretypes.ResultConsensusParams{ BlockHeight: 5, From 7d10819899210d1cae4295c26a3cfa353b801710 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 08:36:37 +0200 Subject: [PATCH 09/15] Use staking.UnbondingTime instead of Evidence Age --- x/ccv/provider/client/proposal_handler.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 12a1e22c68..7714c7eabd 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -16,6 +16,7 @@ import ( govclient "github.com/cosmos/cosmos-sdk/x/gov/client" govrest "github.com/cosmos/cosmos-sdk/x/gov/client/rest" govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" + stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" clienttypes "github.com/cosmos/ibc-go/v4/modules/core/02-client/types" "github.com/cosmos/interchain-security/x/ccv/provider/types" "github.com/spf13/cobra" @@ -454,17 +455,14 @@ func postEquivocationProposalHandlerFn(clientCtx client.Context) http.HandlerFun } func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time.Duration) error { - client, err := clientCtx.GetNode() - if err != nil { - return err - } + queryClient := stakingtypes.NewQueryClient(clientCtx) - consensusParams, err := client.ConsensusParams(context.Background(), nil) + res, err := queryClient.Params(context.Background(), &stakingtypes.QueryParamsRequest{}) if err != nil { return err } - providerUnbondingTime := consensusParams.ConsensusParams.Evidence.MaxAgeDuration + providerUnbondingTime := res.Params.UnbondingTime if providerUnbondingTime <= propUnbondingPeriod { return fmt.Errorf( From 5050b73457b19c26df4aa384aeb116048818a656 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 08:45:58 +0200 Subject: [PATCH 10/15] Refactor error message --- x/ccv/provider/client/proposal_handler.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 7714c7eabd..ed3ffb11b1 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -466,10 +466,9 @@ func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time if providerUnbondingTime <= propUnbondingPeriod { return fmt.Errorf( - "consumer unbonding period should be smaller than provider unbonding period, but is longer: \n"+ - "consumer unbonding: %s \n provider unbonding: %s \n"+ - "See the Consumer onboarding documentation for more information:\n"+ - "https://cosmos.github.io/interchain-security/consumer-development/onboarding#3-submit-a-governance-proposal", + "consumer unbonding period is advised to be smaller than provider unbonding period, but is longer.: \n"+ + "This is not a security risk, but will effectively lengthen the unbonding period on the provider.\n"+ + "consumer unbonding: %s \n provider unbonding: %s", propUnbondingPeriod, providerUnbondingTime) } From 08a2feaafb89df02b798c7f1e426a399de4456d3 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 09:16:40 +0200 Subject: [PATCH 11/15] Remove deprecated test --- testutil/mock_rpc_client.go | 256 ------------------ .../provider/client/proposal_handler_test.go | 68 ----- 2 files changed, 324 deletions(-) delete mode 100644 testutil/mock_rpc_client.go delete mode 100644 x/ccv/provider/client/proposal_handler_test.go diff --git a/testutil/mock_rpc_client.go b/testutil/mock_rpc_client.go deleted file mode 100644 index bb324f1cf1..0000000000 --- a/testutil/mock_rpc_client.go +++ /dev/null @@ -1,256 +0,0 @@ -package testutil - -import ( - "context" - - "github.com/stretchr/testify/mock" - "github.com/tendermint/tendermint/libs/bytes" - "github.com/tendermint/tendermint/libs/log" - rpcClient "github.com/tendermint/tendermint/rpc/client" - ctypes "github.com/tendermint/tendermint/rpc/core/types" - "github.com/tendermint/tendermint/types" -) - -// A mock client to be used in tests that want to mock out calls to the CometBFT CLI. -// The client implements the interface "github.com/tendermint/tendermint/rpc/client".Client -// Example Usage: -// -// mockClient := new(testutil.MockCometBFTRPCClient) -// clientCtx := client.Context{}.WithClient(mockClient) -// mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) -// {code using clientCtx} -type MockCometBFTRPCClient struct { - mock.Mock -} - -// IsRunning implements client.Client -func (m MockCometBFTRPCClient) IsRunning() bool { - args := m.Called() - return args.Bool(0) -} - -// OnReset implements client.Client -func (m MockCometBFTRPCClient) OnReset() error { - args := m.Called() - return args.Error(0) -} - -// OnStart implements client.Client -func (m MockCometBFTRPCClient) OnStart() error { - args := m.Called() - return args.Error(0) -} - -// OnStop implements client.Client -func (m MockCometBFTRPCClient) OnStop() { - m.Called() -} - -// Quit implements client.Client -func (m MockCometBFTRPCClient) Quit() <-chan struct{} { - args := m.Called() - return args.Get(0).(<-chan struct{}) -} - -// Reset implements client.Client -func (m MockCometBFTRPCClient) Reset() error { - args := m.Called() - return args.Error(0) -} - -// SetLogger implements client.Client -func (m MockCometBFTRPCClient) SetLogger(logger log.Logger) { - m.Called(logger) -} - -// Start implements client.Client -func (m MockCometBFTRPCClient) Start() error { - args := m.Called() - return args.Error(0) -} - -// Stop implements client.Client -func (m MockCometBFTRPCClient) Stop() error { - args := m.Called() - return args.Error(0) -} - -// ABCIInfo implements client.Client -func (m MockCometBFTRPCClient) ABCIInfo(ctx context.Context) (*ctypes.ResultABCIInfo, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultABCIInfo), args.Error(1) -} - -// ABCIQuery implements client.Client -func (m MockCometBFTRPCClient) ABCIQuery(ctx context.Context, path string, data bytes.HexBytes) (*ctypes.ResultABCIQuery, error) { - args := m.Called(ctx, path, data) - return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) -} - -// ABCIQueryWithOptions implements client.Client -func (m MockCometBFTRPCClient) ABCIQueryWithOptions(ctx context.Context, path string, data bytes.HexBytes, opts rpcClient.ABCIQueryOptions) (*ctypes.ResultABCIQuery, error) { - args := m.Called(ctx, path, data, opts) - return args.Get(0).(*ctypes.ResultABCIQuery), args.Error(1) -} - -// BroadcastTxAsync implements client.Client -func (m MockCometBFTRPCClient) BroadcastTxAsync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { - args := m.Called(ctx, tx) - return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) -} - -// BroadcastTxCommit implements client.Client -func (m MockCometBFTRPCClient) BroadcastTxCommit(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTxCommit, error) { - args := m.Called(ctx, tx) - return args.Get(0).(*ctypes.ResultBroadcastTxCommit), args.Error(1) -} - -// BroadcastTxSync implements client.Client -func (m MockCometBFTRPCClient) BroadcastTxSync(ctx context.Context, tx types.Tx) (*ctypes.ResultBroadcastTx, error) { - args := m.Called(ctx, tx) - return args.Get(0).(*ctypes.ResultBroadcastTx), args.Error(1) -} - -// Subscribe implements client.Client -func (m MockCometBFTRPCClient) Subscribe(ctx context.Context, subscriber string, query string, outCapacity ...int) (<-chan ctypes.ResultEvent, error) { - args := m.Called(ctx, subscriber, query, outCapacity) - return args.Get(0).(<-chan ctypes.ResultEvent), args.Error(1) -} - -// Unsubscribe implements client.Client -func (m *MockCometBFTRPCClient) Unsubscribe(ctx context.Context, subscriber string, query string) error { - args := m.Called(ctx, subscriber, query) - return args.Error(0) -} - -// UnsubscribeAll implements client.Client -func (m *MockCometBFTRPCClient) UnsubscribeAll(ctx context.Context, subscriber string) error { - args := m.Called(ctx, subscriber) - return args.Error(0) -} - -// BlockchainInfo implements client.Client -func (m *MockCometBFTRPCClient) BlockchainInfo(ctx context.Context, minHeight int64, maxHeight int64) (*ctypes.ResultBlockchainInfo, error) { - args := m.Called(ctx, minHeight, maxHeight) - return args.Get(0).(*ctypes.ResultBlockchainInfo), args.Error(1) -} - -// Genesis implements client.Client -func (m *MockCometBFTRPCClient) Genesis(ctx context.Context) (*ctypes.ResultGenesis, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultGenesis), args.Error(1) -} - -// GenesisChunked implements client.Client -func (m *MockCometBFTRPCClient) GenesisChunked(ctx context.Context, chunkSize uint) (*ctypes.ResultGenesisChunk, error) { - args := m.Called(ctx, chunkSize) - return args.Get(0).(*ctypes.ResultGenesisChunk), args.Error(1) -} - -// ConsensusParams implements client.Client -func (m *MockCometBFTRPCClient) ConsensusParams(ctx context.Context, height *int64) (*ctypes.ResultConsensusParams, error) { - args := m.Called(ctx, height) - return args.Get(0).(*ctypes.ResultConsensusParams), args.Error(1) -} - -// ConsensusState implements client.Client -func (m MockCometBFTRPCClient) ConsensusState(ctx context.Context) (*ctypes.ResultConsensusState, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultConsensusState), args.Error(1) -} - -// DumpConsensusState implements client.Client -func (m MockCometBFTRPCClient) DumpConsensusState(ctx context.Context) (*ctypes.ResultDumpConsensusState, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultDumpConsensusState), args.Error(1) -} - -// Health implements client.Client -func (m MockCometBFTRPCClient) Health(ctx context.Context) (*ctypes.ResultHealth, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultHealth), args.Error(1) -} - -// NetInfo implements client.Client -func (m MockCometBFTRPCClient) NetInfo(ctx context.Context) (*ctypes.ResultNetInfo, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultNetInfo), args.Error(1) -} - -// Block implements client.Client -func (m MockCometBFTRPCClient) Block(ctx context.Context, height *int64) (*ctypes.ResultBlock, error) { - args := m.Called(ctx, height) - return args.Get(0).(*ctypes.ResultBlock), args.Error(1) -} - -// BlockByHash implements client.Client -func (m MockCometBFTRPCClient) BlockByHash(ctx context.Context, hash []byte) (*ctypes.ResultBlock, error) { - args := m.Called(ctx, hash) - return args.Get(0).(*ctypes.ResultBlock), args.Error(1) -} - -// BlockResults implements client.Client -func (m MockCometBFTRPCClient) BlockResults(ctx context.Context, height *int64) (*ctypes.ResultBlockResults, error) { - args := m.Called(ctx, height) - return args.Get(0).(*ctypes.ResultBlockResults), args.Error(1) -} - -// BlockSearch implements client.Client -func (m MockCometBFTRPCClient) BlockSearch(ctx context.Context, query string, page *int, perPage *int, orderBy string) (*ctypes.ResultBlockSearch, error) { - args := m.Called(ctx, query, page, perPage, orderBy) - return args.Get(0).(*ctypes.ResultBlockSearch), args.Error(1) -} - -// Commit implements client.Client -func (m MockCometBFTRPCClient) Commit(ctx context.Context, height *int64) (*ctypes.ResultCommit, error) { - args := m.Called(ctx, height) - return args.Get(0).(*ctypes.ResultCommit), args.Error(1) -} - -// Tx implements client.Client -func (m MockCometBFTRPCClient) Tx(ctx context.Context, hash []byte, prove bool) (*ctypes.ResultTx, error) { - args := m.Called(ctx, hash, prove) - return args.Get(0).(*ctypes.ResultTx), args.Error(1) -} - -// TxSearch implements client.Client -func (m MockCometBFTRPCClient) TxSearch(ctx context.Context, query string, prove bool, page *int, perPage *int, orderBy string) (*ctypes.ResultTxSearch, error) { - args := m.Called(ctx, query, prove, page, perPage, orderBy) - return args.Get(0).(*ctypes.ResultTxSearch), args.Error(1) -} - -// Validators implements client.Client -func (m MockCometBFTRPCClient) Validators(ctx context.Context, height *int64, page *int, perPage *int) (*ctypes.ResultValidators, error) { - args := m.Called(ctx, height, page, perPage) - return args.Get(0).(*ctypes.ResultValidators), args.Error(1) -} - -// Status implements client.Client -func (m *MockCometBFTRPCClient) Status(ctx context.Context) (*ctypes.ResultStatus, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultStatus), args.Error(1) -} - -// BroadcastEvidence implements client.Client -func (m *MockCometBFTRPCClient) BroadcastEvidence(ctx context.Context, ev types.Evidence) (*ctypes.ResultBroadcastEvidence, error) { - args := m.Called(ctx, ev) - return args.Get(0).(*ctypes.ResultBroadcastEvidence), args.Error(1) -} - -// CheckTx implements client.Client -func (m *MockCometBFTRPCClient) CheckTx(ctx context.Context, tx types.Tx) (*ctypes.ResultCheckTx, error) { - args := m.Called(ctx, tx) - return args.Get(0).(*ctypes.ResultCheckTx), args.Error(1) -} - -// NumUnconfirmedTxs implements client.Client -func (m *MockCometBFTRPCClient) NumUnconfirmedTxs(ctx context.Context) (*ctypes.ResultUnconfirmedTxs, error) { - args := m.Called(ctx) - return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) -} - -// UnconfirmedTxs implements client.Client -func (m *MockCometBFTRPCClient) UnconfirmedTxs(ctx context.Context, limit *int) (*ctypes.ResultUnconfirmedTxs, error) { - args := m.Called(ctx, limit) - return args.Get(0).(*ctypes.ResultUnconfirmedTxs), args.Error(1) -} diff --git a/x/ccv/provider/client/proposal_handler_test.go b/x/ccv/provider/client/proposal_handler_test.go deleted file mode 100644 index 1e45652253..0000000000 --- a/x/ccv/provider/client/proposal_handler_test.go +++ /dev/null @@ -1,68 +0,0 @@ -package client_test - -import ( - "testing" - "time" - - "github.com/cosmos/cosmos-sdk/client" - "github.com/cosmos/interchain-security/testutil" - providerClient "github.com/cosmos/interchain-security/x/ccv/provider/client" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" - cmtproto "github.com/tendermint/tendermint/proto/tendermint/types" - coretypes "github.com/tendermint/tendermint/rpc/core/types" -) - -func ConsumerAdditionProposalCLIDriver(providerUnbondingTime, consumerUnbondingTime time.Duration) error { - mockClient := new(testutil.MockCometBFTRPCClient) - - resConsensParams := coretypes.ResultConsensusParams{ - BlockHeight: 5, - ConsensusParams: cmtproto.ConsensusParams{ - Evidence: cmtproto.EvidenceParams{ - MaxAgeDuration: providerUnbondingTime, - }, - }, - } - mockClient.On("ConsensusParams", mock.Anything, mock.Anything).Return(&resConsensParams, nil) - clientCtx := client.Context{}.WithClient(mockClient) - - return providerClient.CheckPropUnbondingPeriod(clientCtx, consumerUnbondingTime) -} - -func TestConsumerAdditionProposalSuite(t *testing.T) { - testCases := []struct { - name string - providerUnbonding time.Duration - consumerUnbonding time.Duration - errorExpected bool - }{ - { - name: "provider unbonding longer", - providerUnbonding: time.Hour * 5, - consumerUnbonding: time.Hour * 4, - errorExpected: false, - }, - { - name: "consumer unbonding longer", - providerUnbonding: time.Hour * 5, - consumerUnbonding: time.Hour * 6, - errorExpected: true, - }, - { - name: "unbondings equal", - providerUnbonding: time.Hour * 5, - consumerUnbonding: time.Hour * 5, - errorExpected: true, - }, - } - - for _, tc := range testCases { - res := ConsumerAdditionProposalCLIDriver(tc.providerUnbonding, tc.consumerUnbonding) - if tc.errorExpected { - require.Error(t, res) - } else { - require.NoError(t, res) - } - } -} From cb05a76be97143207b369df8b29bd1ab4830ca53 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 09:28:16 +0200 Subject: [PATCH 12/15] Remove unused dependency --- go.mod | 1 - 1 file changed, 1 deletion(-) diff --git a/go.mod b/go.mod index dbf01c81f7..072bcb8122 100644 --- a/go.mod +++ b/go.mod @@ -133,7 +133,6 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/spf13/viper v1.14.0 // indirect - github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.4.1 // indirect github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 // indirect github.com/tecbot/gorocksdb v0.0.0-20191217155057-f0fad39f321c // indirect From b68bcc129d2d82a3452651cb22d3c9a107a4f376 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 13:24:06 +0200 Subject: [PATCH 13/15] Adjust warning text --- x/ccv/provider/client/proposal_handler.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index ed3ffb11b1..e5b8d31592 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -79,9 +79,8 @@ Where proposal.json contains: // do not fail for errors regarding the unbonding period, but just log a warning err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) if err != nil { - fmt.Fprintf( + fmt.Fprint( os.Stderr, - "Warning: Could not assure that Proposal Unbonding Period is shorter than Provider Unbonding Period. Error message: %s", err.Error(), ) } @@ -464,11 +463,13 @@ func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time providerUnbondingTime := res.Params.UnbondingTime - if providerUnbondingTime <= propUnbondingPeriod { + if providerUnbondingTime < propUnbondingPeriod { return fmt.Errorf( - "consumer unbonding period is advised to be smaller than provider unbonding period, but is longer.: \n"+ - "This is not a security risk, but will effectively lengthen the unbonding period on the provider.\n"+ - "consumer unbonding: %s \n provider unbonding: %s", + `consumer unbonding period is advised to be smaller than provider unbonding period, but is longer. +This is not a security risk, but will effectively lengthen the unbonding period on the provider. +consumer unbonding: %s +provider unbonding: %s +`, propUnbondingPeriod, providerUnbondingTime) } From 246dafa4b17ad28159fcbc62f65513e3535e9185 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 21 Apr 2023 14:47:07 +0200 Subject: [PATCH 14/15] Remove newline to pass linting --- x/ccv/provider/client/proposal_handler.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index e5b8d31592..93879a7ee3 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -468,8 +468,7 @@ func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time `consumer unbonding period is advised to be smaller than provider unbonding period, but is longer. This is not a security risk, but will effectively lengthen the unbonding period on the provider. consumer unbonding: %s -provider unbonding: %s -`, +provider unbonding: %s`, propUnbondingPeriod, providerUnbondingTime) } From f44bafecb8b1c37025d0411643cc5cf835f5bc52 Mon Sep 17 00:00:00 2001 From: Philip Offtermatt Date: Fri, 28 Apr 2023 09:37:13 +0200 Subject: [PATCH 15/15] Print error directly instead of returning --- x/ccv/provider/client/proposal_handler.go | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/x/ccv/provider/client/proposal_handler.go b/x/ccv/provider/client/proposal_handler.go index 93879a7ee3..1bfb21ea40 100644 --- a/x/ccv/provider/client/proposal_handler.go +++ b/x/ccv/provider/client/proposal_handler.go @@ -77,13 +77,7 @@ Where proposal.json contains: } // do not fail for errors regarding the unbonding period, but just log a warning - err = CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) - if err != nil { - fmt.Fprint( - os.Stderr, - err.Error(), - ) - } + CheckPropUnbondingPeriod(clientCtx, proposal.UnbondingPeriod) content := types.NewConsumerAdditionProposal( proposal.Title, proposal.Description, proposal.ChainId, proposal.InitialHeight, @@ -453,18 +447,19 @@ func postEquivocationProposalHandlerFn(clientCtx client.Context) http.HandlerFun } } -func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time.Duration) error { +func CheckPropUnbondingPeriod(clientCtx client.Context, propUnbondingPeriod time.Duration) { queryClient := stakingtypes.NewQueryClient(clientCtx) res, err := queryClient.Params(context.Background(), &stakingtypes.QueryParamsRequest{}) if err != nil { - return err + fmt.Println(err.Error()) + return } providerUnbondingTime := res.Params.UnbondingTime if providerUnbondingTime < propUnbondingPeriod { - return fmt.Errorf( + fmt.Printf( `consumer unbonding period is advised to be smaller than provider unbonding period, but is longer. This is not a security risk, but will effectively lengthen the unbonding period on the provider. consumer unbonding: %s @@ -472,6 +467,4 @@ provider unbonding: %s`, propUnbondingPeriod, providerUnbondingTime) } - - return nil }