Skip to content

Commit

Permalink
refactor: stop using signer field in messages (#2219)
Browse files Browse the repository at this point in the history
* remove error check

* stop using signer field in messages

* expand TODO comment

* replace signer with submitter

* fix UTs
  • Loading branch information
mpoke authored Sep 5, 2024
1 parent ffd3ae0 commit 6dc36bc
Show file tree
Hide file tree
Showing 10 changed files with 269 additions and 272 deletions.
45 changes: 23 additions & 22 deletions proto/interchain_security/ccv/provider/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ service Msg {


message MsgAssignConsumerKey {
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "submitter";
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

Expand All @@ -58,8 +58,7 @@ message MsgAssignConsumerKey {
// `{"@type":"/cosmos.crypto.ed25519.PubKey","key":"Ui5Gf1+mtWUdH8u3xlmzdKID+F3PK0sfXZ73GZ6q6is="}`
string consumer_key = 3;

// Tx signer address
string signer = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
string submitter = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];

// the consumer id of the consumer chain to assign a consensus public key to
string consumer_id = 5;
Expand Down Expand Up @@ -109,7 +108,7 @@ message MsgSubmitConsumerDoubleVotingResponse {}
message MsgUpdateParams {
option (cosmos.msg.v1.signer) = "authority";

// signer is the address of the governance account.
// authority is the address of the governance account.
string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

// params defines the x/provider parameters to update.
Expand Down Expand Up @@ -224,11 +223,12 @@ message MsgConsumerRemovalResponse {}
// MsgRemoveConsumer defines the message used to remove (and stop) a consumer chain.
// If it passes, all the consumer chain's state is eventually removed from the provider chain.
message MsgRemoveConsumer {
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "owner";

// the consumer id of the consumer chain to be stopped
string consumer_id = 1;
string signer = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// the address of the owner of the consumer chain to be stopped
string owner = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
}

// MsgRemoveConsumerResponse defines response type for MsgRemoveConsumer messages
Expand All @@ -245,7 +245,7 @@ message MsgChangeRewardDenoms {
repeated string denoms_to_add = 1;
// the list of consumer reward denoms to remove
repeated string denoms_to_remove = 2;
// signer address
// authority is the address of the governance account
string authority = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];

}
Expand All @@ -256,7 +256,7 @@ message MsgChangeRewardDenomsResponse {}
message MsgOptIn {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "submitter";
// [DEPRECATED] use `consumer_id` instead
string chain_id = 1 [deprecated = true];
// the validator address on the provider
Expand All @@ -266,8 +266,8 @@ message MsgOptIn {
// This field is optional and can remain empty (i.e., `consumer_key = ""`). A validator can always change the
// consumer public key at a later stage by issuing a `MsgAssignConsumerKey` message.
string consumer_key = 3;
// signer address
string signer = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// submitter address
string submitter = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// the consumer id of the consumer chain to opt in to
string consumer_id = 5;
}
Expand All @@ -277,13 +277,13 @@ message MsgOptInResponse {}
message MsgOptOut {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "submitter";
// [DEPRECATED] use `consumer_id` instead
string chain_id = 1 [deprecated = true];
// the validator address on the provider
string provider_addr = 2 [ (gogoproto.moretags) = "yaml:\"address\"" ];
// signer address
string signer = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// submitter address
string submitter = 3 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// the consumer id of the consumer chain to opt out from
string consumer_id = 4;
}
Expand All @@ -295,7 +295,7 @@ message MsgOptOutResponse {}
message MsgSetConsumerCommissionRate {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "submitter";

// The validator address on the provider
string provider_addr = 1 [ (gogoproto.moretags) = "yaml:\"address\"" ];
Expand All @@ -308,8 +308,8 @@ message MsgSetConsumerCommissionRate {
(gogoproto.customtype) = "cosmossdk.io/math.LegacyDec",
(gogoproto.nullable) = false
];
// signer address
string signer = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// submitter address
string submitter = 4 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// the consumer id of the consumer chain to set the commission rate
string consumer_id = 5;
}
Expand Down Expand Up @@ -358,10 +358,11 @@ message MsgConsumerModificationResponse {}

// MsgCreateConsumer defines the message that creates a consumer chain
message MsgCreateConsumer {
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "submitter";

// signer address
string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// Submitter address. If the message is successfully handled, the ownership of
// the consumer chain will given to this address.
string submitter = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

// the chain id of the new consumer chain
string chain_id = 2;
Expand All @@ -380,10 +381,10 @@ message MsgCreateConsumerResponse {

// MsgUpdateConsumer defines the message used to modify a consumer chain.
message MsgUpdateConsumer {
option (cosmos.msg.v1.signer) = "signer";
option (cosmos.msg.v1.signer) = "owner";

// signer address
string signer = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// the address of the owner of the consumer chain to be updated
string owner = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];

// the consumer id of the consumer chain to be updated
string consumer_id = 2;
Expand Down
6 changes: 3 additions & 3 deletions tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ func (tr Chain) submitConsumerAdditionProposal(

// - set PowerShaping params TopN > 0 for consumer chain
update.PowerShapingParameters.Top_N = action.TopN
update.Signer = authority
update.Owner = authority
update.NewOwnerAddress = ""
update.InitializationParameters = &initializationParameters
update.InitializationParameters.SpawnTime = spawnTime
Expand Down Expand Up @@ -761,7 +761,7 @@ func (tr Chain) submitConsumerRemovalProposal(

msg := types.MsgRemoveConsumer{
ConsumerId: consumerId,
Signer: authority,
Owner: authority,
}

jsonStr := e2e.GenerateGovProposalContent(title, summary, metadata, deposit, description, expedited, &msg)
Expand Down Expand Up @@ -881,7 +881,7 @@ func (tr Chain) submitConsumerModificationProposal(
authority := "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"

msg := types.MsgUpdateConsumer{
Signer: authority,
Owner: authority,
ConsumerId: consumerId,
PowerShapingParameters: &types.PowerShapingParameters{
Top_N: action.TopN,
Expand Down
2 changes: 1 addition & 1 deletion testutil/keeper/unit_test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func GetTestPowerShapingParameters() providertypes.PowerShapingParameters {

func GetTestMsgUpdateConsumer() providertypes.MsgUpdateConsumer {
return providertypes.MsgUpdateConsumer{
Signer: "signer",
Owner: "owner",
ConsumerId: "consumerId",
NewOwnerAddress: "newOwnerAddress",
}
Expand Down
4 changes: 0 additions & 4 deletions x/ccv/provider/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,6 @@ Example:
signer := clientCtx.GetFromAddress().String()
consumerId := args[0]

if err != nil {
return err
}

msg, err := types.NewMsgRemoveConsumer(signer, consumerId)
if err != nil {
return err
Expand Down
10 changes: 5 additions & 5 deletions x/ccv/provider/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon

consumerId := k.Keeper.FetchAndIncrementConsumerId(ctx)

k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.Signer)
k.Keeper.SetConsumerOwnerAddress(ctx, consumerId, msg.Submitter)
k.Keeper.SetConsumerChainId(ctx, consumerId, msg.ChainId)
k.Keeper.SetConsumerPhase(ctx, consumerId, types.CONSUMER_PHASE_REGISTERED)

Expand Down Expand Up @@ -360,8 +360,8 @@ func (k msgServer) UpdateConsumer(goCtx context.Context, msg *types.MsgUpdateCon
return &resp, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
}

if msg.Signer != ownerAddress {
return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
if msg.Owner != ownerAddress {
return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Owner)
}

// The new owner address can be empty, in which case the consumer chain does not change its owner.
Expand Down Expand Up @@ -463,8 +463,8 @@ func (k msgServer) RemoveConsumer(goCtx context.Context, msg *types.MsgRemoveCon
return &resp, errorsmod.Wrapf(types.ErrNoOwnerAddress, "cannot retrieve owner address %s", ownerAddress)
}

if msg.Signer != ownerAddress {
return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Signer)
if msg.Owner != ownerAddress {
return &resp, errorsmod.Wrapf(types.ErrUnauthorized, "expected owner address %s, got %s", ownerAddress, msg.Owner)
}

phase := k.Keeper.GetConsumerPhase(ctx, consumerId)
Expand Down
18 changes: 9 additions & 9 deletions x/ccv/provider/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func TestCreateConsumer(t *testing.T) {
Description: "description",
}
response, err := msgServer.CreateConsumer(ctx,
&providertypes.MsgCreateConsumer{Signer: "signer", ChainId: "chainId", Metadata: consumerMetadata,
&providertypes.MsgCreateConsumer{Submitter: "submitter", ChainId: "chainId", Metadata: consumerMetadata,
InitializationParameters: &providertypes.ConsumerInitializationParameters{},
PowerShapingParameters: &providertypes.PowerShapingParameters{}})
require.NoError(t, err)
Expand All @@ -32,7 +32,7 @@ func TestCreateConsumer(t *testing.T) {
require.Equal(t, consumerMetadata, actualMetadata)
ownerAddress, err := providerKeeper.GetConsumerOwnerAddress(ctx, "0")
require.NoError(t, err)
require.Equal(t, "signer", ownerAddress)
require.Equal(t, "submitter", ownerAddress)
phase := providerKeeper.GetConsumerPhase(ctx, "0")
require.Equal(t, providertypes.CONSUMER_PHASE_REGISTERED, phase)

Expand All @@ -41,7 +41,7 @@ func TestCreateConsumer(t *testing.T) {
Description: "description2",
}
response, err = msgServer.CreateConsumer(ctx,
&providertypes.MsgCreateConsumer{Signer: "signer2", ChainId: "chainId", Metadata: consumerMetadata,
&providertypes.MsgCreateConsumer{Submitter: "submitter2", ChainId: "chainId", Metadata: consumerMetadata,
InitializationParameters: &providertypes.ConsumerInitializationParameters{},
PowerShapingParameters: &providertypes.PowerShapingParameters{}})
require.NoError(t, err)
Expand All @@ -52,7 +52,7 @@ func TestCreateConsumer(t *testing.T) {
require.Equal(t, consumerMetadata, actualMetadata)
ownerAddress, err = providerKeeper.GetConsumerOwnerAddress(ctx, "1")
require.NoError(t, err)
require.Equal(t, "signer2", ownerAddress)
require.Equal(t, "submitter2", ownerAddress)
phase = providerKeeper.GetConsumerPhase(ctx, "1")
require.Equal(t, providertypes.CONSUMER_PHASE_REGISTERED, phase)
}
Expand All @@ -65,7 +65,7 @@ func TestUpdateConsumer(t *testing.T) {

// try to update a non-existing (i.e., no consumer id exists)
_, err := msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: "0", NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
&providertypes.MsgUpdateConsumer{Owner: "owner", ConsumerId: "0", NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
Expand All @@ -74,7 +74,7 @@ func TestUpdateConsumer(t *testing.T) {

// create a chain before updating it
createConsumerResponse, err := msgServer.CreateConsumer(ctx,
&providertypes.MsgCreateConsumer{Signer: "signer", ChainId: "chainId",
&providertypes.MsgCreateConsumer{Submitter: "submitter", ChainId: "chainId",
Metadata: providertypes.ConsumerMetadata{
Name: "name",
Description: "description",
Expand All @@ -88,7 +88,7 @@ func TestUpdateConsumer(t *testing.T) {

mocks.MockAccountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{Signer: "wrong signer", ConsumerId: consumerId, NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
&providertypes.MsgUpdateConsumer{Owner: "wrong owner", ConsumerId: consumerId, NewOwnerAddress: "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la",
Metadata: nil,
InitializationParameters: nil,
PowerShapingParameters: nil,
Expand All @@ -106,7 +106,7 @@ func TestUpdateConsumer(t *testing.T) {

expectedOwnerAddress := "cosmos1dkas8mu4kyhl5jrh4nzvm65qz588hy9qcz08la"
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{Signer: "signer", ConsumerId: consumerId, NewOwnerAddress: expectedOwnerAddress,
&providertypes.MsgUpdateConsumer{Owner: "submitter", ConsumerId: consumerId, NewOwnerAddress: expectedOwnerAddress,
Metadata: &expectedConsumerMetadata,
InitializationParameters: &expectedInitializationParameters,
PowerShapingParameters: &expectedPowerShapingParameters})
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestUpdateConsumer(t *testing.T) {
updatedSpawnTime := expectedInitializationParameters.SpawnTime.Add(time.Hour)
expectedInitializationParameters.SpawnTime = updatedSpawnTime
_, err = msgServer.UpdateConsumer(ctx,
&providertypes.MsgUpdateConsumer{Signer: expectedOwnerAddress, ConsumerId: consumerId,
&providertypes.MsgUpdateConsumer{Owner: expectedOwnerAddress, ConsumerId: consumerId,
Metadata: &expectedConsumerMetadata,
InitializationParameters: &expectedInitializationParameters,
PowerShapingParameters: &expectedPowerShapingParameters})
Expand Down
4 changes: 2 additions & 2 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (k Keeper) BlocksUntilNextEpoch(ctx sdk.Context) int64 {
// If the CCV channel is not established for a consumer chain,
// the updates will remain queued until the channel is established
//
// TODO (mpoke): iterate only over consumers with established channel
// TODO (mpoke): iterate only over consumers with established channel -- GetAllChannelToConsumers
func (k Keeper) SendVSCPackets(ctx sdk.Context) error {
for _, consumerId := range k.GetAllConsumersWithIBCClients(ctx) {
if k.GetConsumerPhase(ctx, consumerId) != providertypes.CONSUMER_PHASE_LAUNCHED {
Expand Down Expand Up @@ -202,7 +202,7 @@ func (k Keeper) SendVSCPacketsToChain(ctx sdk.Context, consumerId, channelId str
// QueueVSCPackets queues latest validator updates for every consumer chain
// with the IBC client created.
//
// TODO (mpoke): iterate only over consumers with established channel
// TODO (mpoke): iterate only over consumers with established channel -- GetAllChannelToConsumers
func (k Keeper) QueueVSCPackets(ctx sdk.Context) error {
valUpdateID := k.GetValidatorSetUpdateId(ctx) // current valset update ID

Expand Down
Loading

0 comments on commit 6dc36bc

Please sign in to comment.