From 0e3e05e1483808de8e70d80a22506d669e8ecad7 Mon Sep 17 00:00:00 2001 From: Simon Noetzlin Date: Mon, 8 Jul 2024 16:09:29 +0200 Subject: [PATCH] address comments --- CHANGELOG.md | 82 ++++++++++--------- app/consumer-democracy/app.go | 2 +- .../proposals_whitelisting.go | 2 +- app/consumer/app.go | 2 +- app/provider/app.go | 2 +- app/sovereign/app.go | 2 +- ...StateMarshalling-20240314151749-32478.fail | 10 --- ...eadAndWriteTrace-20240314151749-32478.fail | 18 ---- x/ccv/provider/keeper/genesis.go | 2 +- x/ccv/provider/keeper/key_assignment.go | 2 +- x/ccv/provider/keeper/msg_server.go | 4 +- x/ccv/provider/keeper/relay.go | 6 +- x/ccv/provider/types/msg.go | 15 +++- 13 files changed, 65 insertions(+), 84 deletions(-) delete mode 100644 tests/e2e/testdata/rapid/TestChainStateMarshalling/TestChainStateMarshalling-20240314151749-32478.fail delete mode 100644 tests/e2e/testdata/rapid/TestReadAndWriteTrace/TestReadAndWriteTrace-20240314151749-32478.fail diff --git a/CHANGELOG.md b/CHANGELOG.md index 78f0892c0b..f8489c31a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,44 +1,4 @@ # CHANGELOG -## v5.0.0 - -❗ *The provider module should not be used with this version.* - -*May 9, 2024* - -### DEPENDENCIES - -- Bump [ibc-go](https://github.com/cosmos/ibc-go) to - [v8.1.x](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0). - ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) -- Bump [CometBFT](https://github.com/cometbft/cometbft) to - [v0.38.4\5](https://github.com/cometbft/cometbft/releases/tag/v0.38.5). - ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) -- Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to -[v0.50.x](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.4) -([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) - -### FEATURES - -- [Consumer](x/ccv/consumer) - - Add consumer `MsgUpdateParams` from [cosmos-sdk](https://github.com/cosmos/cosmos-sdk). - ([\#1814](https://github.com/cosmos/interchain-security/pull/1814)). -- [Provider](x/ccv/provider) - - Add provider `MsgUpdateParams` from [cosmos-sdk](https://github.com/cosmos/cosmos-sdk). - ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)). - -### STATE BREAKING - -- Bump [ibc-go](https://github.com/cosmos/ibc-go) to - [v8.1.x](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0). - ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) -- Bump [CometBFT](https://github.com/cometbft/cometbft) to - [v0.38.4\5](https://github.com/cometbft/cometbft/releases/tag/v0.38.5). - ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) -- Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to -[v0.50.x](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.4) -([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) -- Revert `PutUnbondingOnHold` behavior to ICS@v1 -([\#1819](https://github.com/cosmos/interchain-security/pull/1819)) ## v4.3.0 @@ -91,6 +51,48 @@ - Allow consumer chains to change their PSS parameters. ([\#1932](https://github.com/cosmos/interchain-security/pull/1932)) +## v5.0.0 + +❗ *The provider module should not be used with this version.* + +*May 9, 2024* + +### DEPENDENCIES + +- Bump [ibc-go](https://github.com/cosmos/ibc-go) to + [v8.1.x](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0). + ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) +- Bump [CometBFT](https://github.com/cometbft/cometbft) to + [v0.38.4\5](https://github.com/cometbft/cometbft/releases/tag/v0.38.5). + ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) +- Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to +[v0.50.x](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.4) +([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) + +### FEATURES + +- [Consumer](x/ccv/consumer) + - Add consumer `MsgUpdateParams` from [cosmos-sdk](https://github.com/cosmos/cosmos-sdk). + ([\#1814](https://github.com/cosmos/interchain-security/pull/1814)). +- [Provider](x/ccv/provider) + - Add provider `MsgUpdateParams` from [cosmos-sdk](https://github.com/cosmos/cosmos-sdk). + ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)). + +### STATE BREAKING + +- Bump [ibc-go](https://github.com/cosmos/ibc-go) to + [v8.1.x](https://github.com/cosmos/ibc-go/releases/tag/v8.1.0). + ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) +- Bump [CometBFT](https://github.com/cometbft/cometbft) to + [v0.38.4\5](https://github.com/cometbft/cometbft/releases/tag/v0.38.5). + ([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) +- Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to +[v0.50.x](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.4) +([\#1698](https://github.com/cosmos/interchain-security/pull/1698)) +- Revert `PutUnbondingOnHold` behavior to ICS@v1 +([\#1819](https://github.com/cosmos/interchain-security/pull/1819)) + + ## v4.2.0 May 17, 2024 diff --git a/app/consumer-democracy/app.go b/app/consumer-democracy/app.go index 50c2c1da29..664aff84ed 100644 --- a/app/consumer-democracy/app.go +++ b/app/consumer-democracy/app.go @@ -754,7 +754,7 @@ func New( }, ) if err != nil { - panic(fmt.Errorf("failed to create AnteHandler: %s", err)) + panic(fmt.Errorf("failed to create AnteHandler: %w", err)) } app.SetAnteHandler(anteHandler) diff --git a/app/consumer-democracy/proposals_whitelisting.go b/app/consumer-democracy/proposals_whitelisting.go index fb333dc014..eeee5befa2 100644 --- a/app/consumer-democracy/proposals_whitelisting.go +++ b/app/consumer-democracy/proposals_whitelisting.go @@ -29,7 +29,7 @@ type legacyParamChangeKey struct { Subspace, Key string } -// these parameters don't exist in the consumer app -- keeping them as an +// these parameters don't exist in the consumer app -- keeping them as an example var LegacyWhitelistedParams = map[legacyParamChangeKey]struct{}{ // add whitlisted legacy parameters here [cosmos-sdk <= 0.47] // commented parameters are just an example - most params have been moved to their respecitve modules diff --git a/app/consumer/app.go b/app/consumer/app.go index ab86ec8b42..8eb64bf5c6 100644 --- a/app/consumer/app.go +++ b/app/consumer/app.go @@ -576,7 +576,7 @@ func New( }, ) if err != nil { - panic(fmt.Errorf("failed to create AnteHandler: %s", err)) + panic(fmt.Errorf("failed to create AnteHandler: %w", err)) } app.SetAnteHandler(anteHandler) diff --git a/app/provider/app.go b/app/provider/app.go index 8ba64f7928..ffa071855a 100644 --- a/app/provider/app.go +++ b/app/provider/app.go @@ -741,7 +741,7 @@ func New( }, ) if err != nil { - panic(fmt.Errorf("failed to create AnteHandler: %s", err)) + panic(fmt.Errorf("failed to create AnteHandler: %w", err)) } app.SetInitChainer(app.InitChainer) diff --git a/app/sovereign/app.go b/app/sovereign/app.go index 67386cc70a..328fb5e4b9 100644 --- a/app/sovereign/app.go +++ b/app/sovereign/app.go @@ -597,7 +597,7 @@ func New( }, ) if err != nil { - panic(fmt.Errorf("failed to create AnteHandler: %s", err)) + panic(fmt.Errorf("failed to create AnteHandler: %w", err)) } app.SetAnteHandler(anteHandler) diff --git a/tests/e2e/testdata/rapid/TestChainStateMarshalling/TestChainStateMarshalling-20240314151749-32478.fail b/tests/e2e/testdata/rapid/TestChainStateMarshalling/TestChainStateMarshalling-20240314151749-32478.fail deleted file mode 100644 index 14da620099..0000000000 --- a/tests/e2e/testdata/rapid/TestChainStateMarshalling/TestChainStateMarshalling-20240314151749-32478.fail +++ /dev/null @@ -1,10 +0,0 @@ -# -v0.4.8#14881217339431652995 -0x0 -0x5555555555555 -0x0 -0x0 -0x38e38e38e38e4 -0x3 -0x0 -0x0 \ No newline at end of file diff --git a/tests/e2e/testdata/rapid/TestReadAndWriteTrace/TestReadAndWriteTrace-20240314151749-32478.fail b/tests/e2e/testdata/rapid/TestReadAndWriteTrace/TestReadAndWriteTrace-20240314151749-32478.fail deleted file mode 100644 index 12af62a4bf..0000000000 --- a/tests/e2e/testdata/rapid/TestReadAndWriteTrace/TestReadAndWriteTrace-20240314151749-32478.fail +++ /dev/null @@ -1,18 +0,0 @@ -# -v0.4.8#2933927387728423654 -0x5555555555555 -0x0 -0x0 -0x0 -0x0 -0x0 -0x5555555555555 -0x0 -0x0 -0x5555555555555 -0x0 -0x0 -0x38e38e38e38e4 -0x3 -0x0 -0x0 \ No newline at end of file diff --git a/x/ccv/provider/keeper/genesis.go b/x/ccv/provider/keeper/genesis.go index b8d6d179fc..b965e3ed57 100644 --- a/x/ccv/provider/keeper/genesis.go +++ b/x/ccv/provider/keeper/genesis.go @@ -21,7 +21,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { err := k.BindPort(ctx, ccv.ProviderPortID) if err != nil { // If the binding fails, the chain MUST NOT start - panic(fmt.Errorf("could not claim port capability: %v", err)) + panic(fmt.Errorf("could not claim port capability: %w", err)) } } diff --git a/x/ccv/provider/keeper/key_assignment.go b/x/ccv/provider/keeper/key_assignment.go index b7d1f2de0b..596c3be606 100644 --- a/x/ccv/provider/keeper/key_assignment.go +++ b/x/ccv/provider/keeper/key_assignment.go @@ -506,7 +506,7 @@ func (k Keeper) ValidatorConsensusKeyInUse(ctx sdk.Context, valAddr sdk.ValAddre val, err := k.stakingKeeper.GetValidator(ctx, valAddr) if err != nil { // Abort TX, do NOT allow validator to be created - panic(fmt.Errorf("error finding newly created validator in staking module: %s", err)) + panic(fmt.Errorf("error finding newly created validator in staking module: %w", err)) } // Get the consensus address of the validator being added diff --git a/x/ccv/provider/keeper/msg_server.go b/x/ccv/provider/keeper/msg_server.go index 4239a7cf41..a9aa6d4379 100644 --- a/x/ccv/provider/keeper/msg_server.go +++ b/x/ccv/provider/keeper/msg_server.go @@ -83,7 +83,7 @@ func (k msgServer) AssignConsumerKey(goCtx context.Context, msg *types.MsgAssign return &types.MsgAssignConsumerKeyResponse{}, nil } -// ConsumerAddition defines a rpc handler method for MsgConsumerAddition +// ConsumerAddition defines an RPC handler method for MsgConsumerAddition func (k msgServer) ConsumerAddition(goCtx context.Context, msg *types.MsgConsumerAddition) (*types.MsgConsumerAdditionResponse, error) { if k.GetAuthority() != msg.Authority { return nil, errorsmod.Wrapf(types.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Authority) @@ -97,7 +97,7 @@ func (k msgServer) ConsumerAddition(goCtx context.Context, msg *types.MsgConsume return &types.MsgConsumerAdditionResponse{}, nil } -// ConsumerRemoval defines a rpc handler method for MsgConsumerRemoval +// ConsumerRemoval defines an RPC handler method for MsgConsumerRemoval func (k msgServer) ConsumerRemoval( goCtx context.Context, msg *types.MsgConsumerRemoval) (*types.MsgConsumerRemovalResponse, error) { diff --git a/x/ccv/provider/keeper/relay.go b/x/ccv/provider/keeper/relay.go index 6373e6a7ce..4d8f862c1b 100644 --- a/x/ccv/provider/keeper/relay.go +++ b/x/ccv/provider/keeper/relay.go @@ -215,7 +215,7 @@ func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, chainID, channelID string } // QueueVSCPackets queues latest validator updates for every registered consumer chain -// failing to GetLastValidators will cause a panic in EndBlock +// failing to GetLastBondedValidators will cause a panic in EndBlock // TODO: decide if this func shouldn't return an error to be propagated to BeginBlocker func (k Keeper) QueueVSCPackets(ctx sdk.Context) { @@ -427,8 +427,8 @@ func (k Keeper) HandleSlashPacket(ctx sdk.Context, chainID string, data ccv.Slas // Obtain validator from staking keeper validator, err := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerConsAddr.ToSdkConsAddr()) - if err != nil && errors.Is(err, stakingtypes.ErrNoValidatorFound) { - k.Logger(ctx).Error("validator not found", "validator", providerConsAddr.String()) + if err != nil { + k.Logger(ctx).Error("validator not found", "validator", providerConsAddr.String(), "error", err) return } diff --git a/x/ccv/provider/types/msg.go b/x/ccv/provider/types/msg.go index 058ee1d2e5..954d5e7cd1 100644 --- a/x/ccv/provider/types/msg.go +++ b/x/ccv/provider/types/msg.go @@ -32,6 +32,7 @@ var ( _ sdk.Msg = (*MsgAssignConsumerKey)(nil) _ sdk.Msg = (*MsgConsumerAddition)(nil) _ sdk.Msg = (*MsgConsumerRemoval)(nil) + _ sdk.Msg = (*MsgConsumerModification)(nil) _ sdk.Msg = (*MsgChangeRewardDenoms)(nil) _ sdk.Msg = (*MsgSubmitConsumerMisbehaviour)(nil) _ sdk.Msg = (*MsgSubmitConsumerDoubleVoting)(nil) @@ -42,6 +43,7 @@ var ( _ sdk.HasValidateBasic = (*MsgAssignConsumerKey)(nil) _ sdk.HasValidateBasic = (*MsgConsumerAddition)(nil) _ sdk.HasValidateBasic = (*MsgConsumerRemoval)(nil) + _ sdk.HasValidateBasic = (*MsgConsumerModification)(nil) _ sdk.HasValidateBasic = (*MsgChangeRewardDenoms)(nil) _ sdk.HasValidateBasic = (*MsgSubmitConsumerMisbehaviour)(nil) _ sdk.HasValidateBasic = (*MsgSubmitConsumerDoubleVoting)(nil) @@ -244,10 +246,6 @@ func (msg *MsgConsumerAddition) ValidateBasic() error { return ErrBlankConsumerChainID } - if strings.TrimSpace(msg.ChainId) == "" { - return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "consumer chain id must not be blank") - } - if msg.InitialHeight.IsZero() { return errorsmod.Wrap(ErrInvalidConsumerAdditionProposal, "initial height cannot be zero") } @@ -306,6 +304,15 @@ func (msg *MsgConsumerRemoval) ValidateBasic() error { return nil } +// ValidateBasic implements the sdk.Msg interface. +func (msg *MsgConsumerModification) ValidateBasic() error { + if strings.TrimSpace(msg.ChainId) == "" { + return ErrBlankConsumerChainID + } + + return nil +} + // NewMsgOptIn creates a new NewMsgOptIn instance. func NewMsgOptIn(chainID string, providerValidatorAddress sdk.ValAddress, consumerConsensusPubKey, signer string) (*MsgOptIn, error) { return &MsgOptIn{