Skip to content
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.

Sign up for GitHub

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

fix: permissionless added event + cli changes #2185

Merged
merged 4 commits into from
Aug 29, 2024
Merged

fix: permissionless added event + cli changes #2185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b017871
add consumer registration event
bermuell Aug 15, 2024
8db2423
fix some queries, CLI and permissionless logic
bermuell Aug 21, 2024
0173c0d
cli changes
bermuell Aug 29, 2024
525afc0
addressed review comments
bermuell Aug 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions x/ccv/provider/client/cli/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ func NewQueryCmd() *cobra.Command {
// parameters managed by the x/params module.
func CmdConsumerGenesis() *cobra.Command {
cmd := &cobra.Command{
Use: "consumer-genesis [chainid]",
Short: "Query for consumer chain genesis state by chain id",
Use: "consumer-genesis [consumer-id]",
Short: "Query for consumer chain genesis state by consumer id",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
Expand Down Expand Up @@ -241,12 +241,12 @@ $ %s query provider validator-consumer-key foochain %s1gghjut3ccd8ay0zduzj64hwre
func CmdProviderValidatorKey() *cobra.Command {
bech32PrefixConsAddr := sdk.GetConfig().GetBech32ConsensusAddrPrefix()
cmd := &cobra.Command{
Use: "validator-provider-key [chainid] [consumer-validator-address]",
Use: "validator-provider-key [consumer-id] [consumer-validator-address]",
Short: "Query validator consensus public key for the provider chain",
Long: strings.TrimSpace(
fmt.Sprintf(`Returns the currently assigned validator consensus public key for the provider chain.
Example:
$ %s query provider validator-provider-key foochain %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj
$ %s query provider validator-provider-key 333 %s1gghjut3ccd8ay0zduzj64hwre2fxs9ldmqhffj
`,
version.AppName, bech32PrefixConsAddr,
),
Expand All @@ -259,15 +259,15 @@ $ %s query provider validator-provider-key foochain %s1gghjut3ccd8ay0zduzj64hwre
}
queryClient := types.NewQueryClient(clientCtx)

consumerChainID := args[0]
consumerID := args[0]

addr, err := sdk.ConsAddressFromBech32(args[1])
if err != nil {
return err
}

req := &types.QueryValidatorProviderAddrRequest{
ChainId: consumerChainID,
ConsumerId: consumerID,
ConsumerAddress: addr.String(),
}
res, err := queryClient.QueryValidatorProviderAddr(cmd.Context(), req)
Expand Down
80 changes: 22 additions & 58 deletions x/ccv/provider/client/cli/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,17 @@ Example:

func NewCreateConsumerCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "create-consumer [chain-id] [metadata] [initialization-parameters] [power-shaping-parameters]",
Use: "create-consumer [consumer-parameters]",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will the user differentiate between the json they need to provide for create-consumer and the one for update-consumer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, we need to provide an example.
I can also imagine to have the common part's in a json file (e.g. metadata, init-, powershaping-parameters) and those which differ as mandatory/optional arguments (optional would be 'new_owner').
If you're ok I'll keep this change for later once we have a preference.
In any case we'll end up with a 'one-file solution' I'd say.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just needs an update to the instructions.

All json fields have omitempty tag.

Short: "create a consumer chain",
Long: strings.TrimSpace(
fmt.Sprintf(`Create a consumer chain and get the assigned consumer id of this chain.
Note that the one that signs this message is the owner of this consumer chain. The owner can be later
changed by updating the consumer chain.

Example:
%s tx provider create-consumer [chain-id] [path/to/metadata.json] [path/to/initialization-parameters.json] [path/to/power-shaping-parameters.json] --from node0 --home ../node0 --chain-id $CID
%s tx provider create-consumer [path/to/create_consumer.json] --from node0 --home ../node0 --chain-id $CID
mpoke marked this conversation as resolved.
Show resolved Hide resolved
`, version.AppName)),
Args: cobra.ExactArgs(4),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
Expand All @@ -240,37 +240,16 @@ Example:

signer := clientCtx.GetFromAddress().String()

chainId := args[0]

metadata := types.ConsumerMetadata{}
metadataJson, err := os.ReadFile(args[1])
if err != nil {
return err
}
if err = json.Unmarshal(metadataJson, &metadata); err != nil {
return fmt.Errorf("metadata unmarshalling failed: %w", err)
}

initializationParameters := types.ConsumerInitializationParameters{}
initializationParametersJson, err := os.ReadFile(args[2])
if err != nil {
return err
}
if err = json.Unmarshal(initializationParametersJson, &initializationParameters); err != nil {
return fmt.Errorf("initialization parameters unmarshalling failed: %w", err)
}

powerShapingParameters := types.PowerShapingParameters{}

powerShapingParametersJson, err := os.ReadFile(args[3])
consCreateJson, err := os.ReadFile(args[0])
if err != nil {
return err
}
if err = json.Unmarshal(powerShapingParametersJson, &powerShapingParameters); err != nil {
return fmt.Errorf("power-shaping parameters unmarshalling failed: %w", err)
consCreate := types.MsgCreateConsumer{}
if err = json.Unmarshal(consCreateJson, &consCreate); err != nil {
return fmt.Errorf("consumer data unmarshalling failed: %w", err)
}

msg, err := types.NewMsgCreateConsumer(signer, chainId, metadata, &initializationParameters, &powerShapingParameters)
msg, err := types.NewMsgCreateConsumer(signer, consCreate.ChainId, consCreate.Metadata, consCreate.InitializationParameters, consCreate.PowerShapingParameters)
if err != nil {
return err
}
Expand All @@ -291,16 +270,16 @@ Example:

func NewUpdateConsumerCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "update-consumer [consumer-id] [owner-address] [metadata] [initialization-parameters] [power-shaping-parameters]",
Use: "update-consumer [consumer-parameters]",
Short: "update a consumer chain",
Long: strings.TrimSpace(
fmt.Sprintf(`Update a consumer chain to change its parameters (e.g., spawn time, allow list, etc.).
Note that only the owner of the chain can initialize it.

Example:
%s tx provider update-consumer [consumer-id] [owner-address] [path/to/metadata.json] [path/to/initialization-parameters.json] [path/to/power-shaping-parameters.json] --from node0 --home ../node0 --chain-id $CID
%s tx provider update-consumer [path/to/consumer-update.json] --from node0 --home ../node0 --chain-id $CID
bermuell marked this conversation as resolved.
Show resolved Hide resolved
`, version.AppName)),
Args: cobra.ExactArgs(5),
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
Expand All @@ -314,37 +293,22 @@ Example:
txf = txf.WithTxConfig(clientCtx.TxConfig).WithAccountRetriever(clientCtx.AccountRetriever)

signer := clientCtx.GetFromAddress().String()
consumerId := args[0]
ownerAddress := args[1]

metadata := types.ConsumerMetadata{}
metadataJson, err := os.ReadFile(args[2])
consUpdateJson, err := os.ReadFile(args[0])
if err != nil {
return err
}
if err = json.Unmarshal(metadataJson, &metadata); err != nil {
return fmt.Errorf("metadata unmarshalling failed: %w", err)
}

initializationParameters := types.ConsumerInitializationParameters{}
initializationParametersJson, err := os.ReadFile(args[3])
if err != nil {
return err
}
if err = json.Unmarshal(initializationParametersJson, &initializationParameters); err != nil {
return fmt.Errorf("initialization parameters unmarshalling failed: %w", err)
consUpdate := types.MsgUpdateConsumer{}
if err = json.Unmarshal(consUpdateJson, &consUpdate); err != nil {
return fmt.Errorf("consumer data unmarshalling failed: %w", err)
}

powerShapingParameters := types.PowerShapingParameters{}
powerShapingParametersJson, err := os.ReadFile(args[4])
if err != nil {
return err
}
if err = json.Unmarshal(powerShapingParametersJson, &powerShapingParameters); err != nil {
return fmt.Errorf("power-shaping parameters unmarshalling failed: %w", err)
if strings.TrimSpace(consUpdate.ConsumerId) == "" {
return fmt.Errorf("consumer_id can't be empty")
}

msg, err := types.NewMsgUpdateConsumer(signer, consumerId, ownerAddress, &metadata, &initializationParameters, &powerShapingParameters)
msg, err := types.NewMsgUpdateConsumer(signer, consUpdate.ConsumerId, consUpdate.NewOwnerAddress, consUpdate.Metadata, consUpdate.InitializationParameters, consUpdate.PowerShapingParameters)
if err != nil {
return err
}
Expand Down Expand Up @@ -418,7 +382,7 @@ Example:

func NewOptInCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "opt-in [consumer-chain-id] [consumer-pubkey]",
Use: "opt-in [consumer-id] [consumer-pubkey]",
Short: "opts in validator to the consumer chain, and if given uses the " +
"provided consensus public key for this consumer chain",
Args: cobra.RangeArgs(1, 2),
Expand Down Expand Up @@ -466,7 +430,7 @@ func NewOptInCmd() *cobra.Command {

func NewOptOutCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "opt-out [consumer-chain-id]",
Use: "opt-out [consumer-id]",
Short: "opts out validator from this consumer chain",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -505,12 +469,12 @@ func NewOptOutCmd() *cobra.Command {

func NewSetConsumerCommissionRateCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "set-consumer-commission-rate [consumer-chain-id] [commission-rate]",
Use: "set-consumer-commission-rate [consumer-id] [commission-rate]",
bermuell marked this conversation as resolved.
Show resolved Hide resolved
Short: "set a per-consumer chain commission",
Long: strings.TrimSpace(
fmt.Sprintf(`Note that the "commission-rate" argument is a fraction and should be in the range [0,1].
Example:
%s set-consumer-commission-rate consumer-1 0.5 --from node0 --home ../node0`,
%s set-consumer-commission-rate 123 0.5 --from node0 --home ../node0`,
version.AppName),
),
Args: cobra.ExactArgs(2),
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func (k Keeper) QueryValidatorProviderAddr(goCtx context.Context, req *types.Que
}
consumerAddr := types.NewConsumerConsAddress(consumerAddrTmp)

providerAddr, found := k.GetValidatorByConsumerAddr(ctx, req.ChainId, consumerAddr)
providerAddr, found := k.GetValidatorByConsumerAddr(ctx, req.ConsumerId, consumerAddr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch. @insumity why do we have deprecated fields in query requests. Given that both chanId and consumerId have the same type, it's easy to have bugs where we call functions with a chainId.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward compatibility. We try to prevent chainIds from being used by doing:

} else if req.ChainId != "" {
    return nil, status.Errorf(codes.InvalidArgument, "ChainId has been deprecated. Use ConsumerId instead.")
}

But in this case we unfortunately missed it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward compatibility.

Can you please give more details about this. What's the use case that we want to keep compatibility with?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could imagine that Hermes is using this query (QueryValidatorProviderAddr) so they use our protos to build Hermes. If we were to simply go and remove chain_id, we would break their builds. It seems, in those cases, the right approach is to just mark chain_id as deprecated. It is true that the actual query is still broken from their side but this seems less severe.
We could remove all those chain_ids if we were to bump the proto version to v2 but this seems like it would be more time consuming at this stage.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumping the proto version is something we should talk to @MSalopek.

Regarding breaking the queries for Hermes for example, as you stated it will be broken anyway. All the users of these queries will need to update their code. Leaving chainId there seems even more dangerous to me as there is a chance that we forget to return an error and the query goes through and returns weird stuff. So my recommendation would be to remove deprecated fields from the queries.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid v2 and inform hermes that there will be a compatibility break and that they should update their code.

if !found {
return &types.QueryValidatorProviderAddrResponse{}, nil
}
Expand Down
6 changes: 6 additions & 0 deletions x/ccv/provider/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,12 @@ func (k msgServer) CreateConsumer(goCtx context.Context, msg *types.MsgCreateCon
}
}

ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(ccvtypes.EventTypeConsumerCreation,
sdk.NewAttribute(ccvtypes.AttributeConsumerID, consumerId),
),
})

return &types.MsgCreateConsumerResponse{ConsumerId: consumerId}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions x/ccv/types/events.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const (
EventTypeExecuteConsumerChainSlash = "execute_consumer_chain_slash"
EventTypeFeeDistribution = "fee_distribution"
EventTypeConsumerSlashRequest = "consumer_slash_request"
EventTypeConsumerCreation = "consumer_creation"

AttributeKeyAckSuccess = "success"
AttributeKeyAck = "acknowledgement"
Expand All @@ -34,6 +35,7 @@ const (
AttributeMisbehaviourHeight1 = "misbehaviour_height_1"
AttributeMisbehaviourHeight2 = "misbehaviour_height_2"
AttributeConsumerDoubleVoting = "consumer_double_voting"
AttributeConsumerID = "consumer_id"
AttributeChainID = "chain_id"
AttributeValidatorAddress = "validator_address"
AttributeInfractionType = "infraction_type"
Expand Down
Loading