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

add metadata to megavault operator params #2509

Merged
merged 1 commit into from
Oct 16, 2024
Merged

add metadata to megavault operator params #2509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 92 additions & 1 deletion indexer/packages/v4-protos/src/codegen/dydxprotocol/vault/params.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,38 @@ export interface VaultParamsSDKType {
/** OperatorParams stores parameters regarding megavault operator. */

export interface OperatorParams {
/** Address of the operator. */
operator: string;
/** Metadata of the operator. */

metadata?: OperatorMetadata;
Comment on lines +97 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure Consistent Initialization of Optional Fields

The metadata field in OperatorParams and OperatorParamsSDKType is declared as optional. In the createBaseOperatorParams() function, it is initialized to undefined. To maintain consistency and avoid potential runtime errors when accessing metadata, consider initializing it with a default value.

Apply this diff to initialize metadata with a default OperatorMetadata object:

 function createBaseOperatorParams(): OperatorParams {
   return {
     operator: "",
-    metadata: undefined
+    metadata: createBaseOperatorMetadata()
   };
 }

This change ensures that metadata is always an object, simplifying usage throughout the codebase.

Also applies to: 106-110

}
/** OperatorParams stores parameters regarding megavault operator. */

export interface OperatorParamsSDKType {
/** Address of the operator. */
operator: string;
/** Metadata of the operator. */

metadata?: OperatorMetadataSDKType;
}
/** OperatorMetadata stores metadata regarding megavault operator. */

export interface OperatorMetadata {
/** Name of the operator. */
name: string;
/** Description of the operator. */

description: string;
}
/** OperatorMetadata stores metadata regarding megavault operator. */

export interface OperatorMetadataSDKType {
/** Name of the operator. */
name: string;
/** Description of the operator. */

description: string;
}
/**
* Deprecated: Params stores `x/vault` parameters.
Expand Down Expand Up @@ -342,7 +368,8 @@ export const VaultParams = {

function createBaseOperatorParams(): OperatorParams {
return {
operator: ""
operator: "",
metadata: undefined
Comment on lines +371 to +372
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Initialize 'metadata' with a Default Value

In the createBaseOperatorParams() function, metadata is set to undefined. To prevent possible undefined errors when accessing metadata properties, initialize it using the createBaseOperatorMetadata() function.

Apply this diff:

 function createBaseOperatorParams(): OperatorParams {
   return {
     operator: "",
-    metadata: undefined
+    metadata: createBaseOperatorMetadata()
   };
 }

This ensures that metadata has default values and aligns with the initialization pattern used elsewhere.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
operator: "",
metadata: undefined
operator: "",
metadata: createBaseOperatorMetadata()

};
}

Expand All @@ -352,6 +379,10 @@ export const OperatorParams = {
writer.uint32(10).string(message.operator);
}

if (message.metadata !== undefined) {
OperatorMetadata.encode(message.metadata, writer.uint32(18).fork()).ldelim();
}

return writer;
},

Expand All @@ -368,6 +399,10 @@ export const OperatorParams = {
message.operator = reader.string();
break;

case 2:
message.metadata = OperatorMetadata.decode(reader, reader.uint32());
break;

Comment on lines +402 to +405
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle Optional 'metadata' Field in Decoding Logic

In the decode() method for OperatorParams, the decoding of metadata does not account for its optional nature. Ensure that the decoder handles the absence of the metadata field gracefully to prevent runtime exceptions.

Consider checking if the next tag corresponds to metadata before decoding, or handle the possibility that metadata might not be present in the input.

default:
reader.skipType(tag & 7);
break;
Expand All @@ -380,6 +415,62 @@ export const OperatorParams = {
fromPartial(object: DeepPartial<OperatorParams>): OperatorParams {
const message = createBaseOperatorParams();
message.operator = object.operator ?? "";
message.metadata = object.metadata !== undefined && object.metadata !== null ? OperatorMetadata.fromPartial(object.metadata) : undefined;
return message;
}

};

function createBaseOperatorMetadata(): OperatorMetadata {
return {
name: "",
description: ""
};
}

export const OperatorMetadata = {
encode(message: OperatorMetadata, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
if (message.name !== "") {
writer.uint32(10).string(message.name);
}

if (message.description !== "") {
writer.uint32(18).string(message.description);
}

return writer;
},

decode(input: _m0.Reader | Uint8Array, length?: number): OperatorMetadata {
const reader = input instanceof _m0.Reader ? input : new _m0.Reader(input);
let end = length === undefined ? reader.len : reader.pos + length;
const message = createBaseOperatorMetadata();

while (reader.pos < end) {
const tag = reader.uint32();

switch (tag >>> 3) {
case 1:
message.name = reader.string();
break;

case 2:
message.description = reader.string();
break;

default:
reader.skipType(tag & 7);
break;
}
}

return message;
},

fromPartial(object: DeepPartial<OperatorMetadata>): OperatorMetadata {
const message = createBaseOperatorMetadata();
message.name = object.name ?? "";
message.description = object.description ?? "";
return message;
}

Expand Down
11 changes: 11 additions & 0 deletions proto/dydxprotocol/vault/params.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,18 @@ message VaultParams {

// OperatorParams stores parameters regarding megavault operator.
message OperatorParams {
// Address of the operator.
string operator = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ];
// Metadata of the operator.
OperatorMetadata metadata = 2 [ (gogoproto.nullable) = false ];
}

// OperatorMetadata stores metadata regarding megavault operator.
message OperatorMetadata {
// Name of the operator.
string name = 1;
// Description of the operator.
string description = 2;
}

// Deprecated: Params stores `x/vault` parameters.
Expand Down
6 changes: 5 additions & 1 deletion protocol/app/testdata/default_genesis_state.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,11 @@
"activation_threshold_quote_quantums": "1000000000"
},
"operator_params": {
"operator": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
"operator": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky",
"metadata": {
"name": "Governance",
"description": "Governance Module Account"
}
},
"owner_shares": [],
"total_shares": {
Expand Down
4 changes: 4 additions & 0 deletions protocol/scripts/genesis/sample_pregenesis.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3972,6 +3972,10 @@
"spread_min_ppm": 3000
},
"operator_params": {
"metadata": {
"description": "Governance Module Account",
"name": "Governance"
},
"operator": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
},
"owner_shares": [
Expand Down
2 changes: 2 additions & 0 deletions protocol/testing/genesis.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2273,6 +2273,8 @@ function edit_genesis() {
dasel put -t int -f "$GENESIS" ".app_state.vault.default_quoting_params.spread_min_ppm" -v '3000'
# Set operator params.
dasel put -t string -f "$GENESIS" ".app_state.vault.operator_params.operator" -v 'dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky'
dasel put -t string -f "$GENESIS" ".app_state.vault.operator_params.metadata.name" -v 'Governance'
dasel put -t string -f "$GENESIS" ".app_state.vault.operator_params.metadata.description" -v 'Governance Module Account'
Comment on lines +2276 to +2277
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

'metadata' field not found in 'OperatorParams'

The OperatorParams message does not include the metadata field as defined. Please ensure that the metadata field is properly defined within OperatorParams in the protocol buffer definitions. Without this field being defined, the genesis processing may fail.

  • File: protocol/testing/genesis.sh lines 2276-2277 use operator_params.metadata
🔗 Analysis chain

Verify the inclusion of 'metadata' fields in 'operator_params'.

The code adds metadata.name and metadata.description to operator_params. Ensure that these fields are supported in the operator_params structure and that all necessary consuming code correctly handles these new fields without causing issues during genesis processing.

Run the following script to check if metadata is a valid field in operator_params throughout the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that 'metadata' is a recognized field within 'operator_params'.

# Search for the definition of 'OperatorParams' and check for 'metadata' field.
ast-grep --lang proto --pattern $'message OperatorParams {
  $$$
  OperatorMetadata metadata = $_;
  $$$
}'

# Search for usages of 'operator_params.metadata' in the codebase.
rg --type shell --type go '\operator_params\.metadata' -A 5 -B 5

Length of output: 379

Script:

#!/bin/bash
# Description: Verify that 'metadata' is a recognized field within 'OperatorParams'.

# Search for the definition of 'OperatorParams' and check for 'metadata' field in .proto files.
rg 'message OperatorParams\s*\{[^}]*OperatorMetadata\s+metadata\s+=' --type proto

# Search for usages of 'operator_params.metadata' in shell and Go files.
rg 'operator_params\.metadata' --type sh --type go -A 5 -B 5

Length of output: 1264

Script:

#!/bin/bash
# Description: Verify that 'metadata' is a recognized field within 'OperatorParams'.

# Define the 'proto' file type for rg
rg --type-add 'proto:*.proto' --type proto 'message OperatorParams\s*\{[^}]*OperatorMetadata\s+metadata\s+='

# Search for usages of 'operator_params.metadata' in shell and Go files with corrected regex
rg 'operator_params\\.metadata' --type sh --type go -A 5 -B 5

Length of output: 174

# Set total shares and owner shares.
if [ -z "${INPUT_TEST_ACCOUNTS[0]}" ]; then
vault_owner_address='dydx199tqg4wdlnu4qjlxchpd7seg454937hjrknju4' # alice as default vault owner
Expand Down
6 changes: 5 additions & 1 deletion protocol/testutil/constants/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4594,7 +4594,11 @@ const GenesisState = `{
"activation_threshold_quote_quantums": "1000000000"
},
"operator_params": {
"operator": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky"
"operator": "dydx10d07y265gmmuvt4z0w9aw880jnsr700jnmapky",
"metadata": {
"name": "Governance",
"description": "Governance Module Account"
}
},
"owner_shares": [],
"total_shares": {
Expand Down
1 change: 1 addition & 0 deletions protocol/x/vault/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ func TestQueryParams(t *testing.T) {
DefaultQuotingParams: types.DefaultQuotingParams(),
OperatorParams: types.OperatorParams{
Operator: constants.GovAuthority,
Metadata: types.DefaultOperatorParams().Metadata,
},
},
err: nil,
Expand Down
5 changes: 5 additions & 0 deletions protocol/x/vault/keeper/params_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,18 @@ func TestGetSetOperatorParams(t *testing.T) {
t,
vaulttypes.OperatorParams{
Operator: constants.GovAuthority,
Metadata: vaulttypes.DefaultOperatorParams().Metadata,
},
params,
)

// Set operator to Alice.
newParams := vaulttypes.OperatorParams{
Operator: constants.AliceAccAddress.String(),
Metadata: vaulttypes.OperatorMetadata{
Name: "Alice",
Description: "Alice is a community-elected individual",
},
}
err := k.SetOperatorParams(ctx, newParams)
require.NoError(t, err)
Expand Down
4 changes: 4 additions & 0 deletions protocol/x/vault/types/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,9 @@ func (o OperatorParams) Validate() error {
func DefaultOperatorParams() OperatorParams {
return OperatorParams{
Operator: lib.GovModuleAddress.String(),
Metadata: OperatorMetadata{
Name: "Governance",
Description: "Governance Module Account",
},
}
}
Loading
Loading