chore: x/tokenfactory audit (#1962)

* updates

* updates

* updates

* updates

* Update x/tokenfactory/README.md

Co-authored-by: Roman <[email protected]>

* Update x/tokenfactory/module.go

Co-authored-by: Roman <[email protected]>

* updates

* updates

Co-authored-by: Roman <[email protected]>

proto/osmosis/tokenfactory/v1beta1/genesis.proto

@@ -18,6 +18,9 @@ message GenesisState {
   ];
 }
 
+// GenesisDenom defines a tokenfactory denom that is defined within genesis
+// state. The structure contains DenomAuthorityMetadata which defines the
+// denom's admin.
 message GenesisDenom {
   option (gogoproto.equal) = true;
 
@@ -26,4 +29,4 @@ message GenesisDenom {
     (gogoproto.moretags) = "yaml:\"authority_metadata\"",
     (gogoproto.nullable) = false
   ];
-}
+}

proto/osmosis/tokenfactory/v1beta1/params.proto

@@ -8,7 +8,7 @@ import "cosmos/base/v1beta1/coin.proto";
 
 option go_package = "github.com/osmosis-labs/osmosis/v7/x/tokenfactory/types";
 
-// Params holds parameters for the tokenfactory module
+// Params defines the parameters for the tokenfactory module.
 message Params {
   repeated cosmos.base.v1beta1.Coin denom_creation_fee = 1 [
     (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",

proto/osmosis/tokenfactory/v1beta1/query.proto

@@ -11,17 +11,22 @@ option go_package = "github.com/osmosis-labs/osmosis/v7/x/tokenfactory/types";
 
 // Query defines the gRPC querier service.
 service Query {
-  // Params returns the total set of minting parameters.
+  // Params defines a gRPC query method that returns the tokenfactory module's
+  // parameters.
   rpc Params(QueryParamsRequest) returns (QueryParamsResponse) {
     option (google.api.http).get = "/osmosis/tokenfactory/v1beta1/params";
   }
 
+  // DenomAuthorityMetadata defines a gRPC query method for fetching
+  // DenomAuthorityMetadata for a particular denom.
   rpc DenomAuthorityMetadata(QueryDenomAuthorityMetadataRequest)
       returns (QueryDenomAuthorityMetadataResponse) {
     option (google.api.http).get =
         "/osmosis/tokenfactory/v1beta1/denoms/{denom}/authority_metadata";
   }
 
+  // DenomsFromCreator defines a gRPC query method for fetching all
+  // denominations created by a specific admin/creator.
   rpc DenomsFromCreator(QueryDenomsFromCreatorRequest)
       returns (QueryDenomsFromCreatorResponse) {
     option (google.api.http).get =
@@ -38,19 +43,29 @@ message QueryParamsResponse {
   Params params = 1 [ (gogoproto.nullable) = false ];
 }
 
+// QueryDenomAuthorityMetadataRequest defines the request structure for the
+// DenomAuthorityMetadata gRPC query.
 message QueryDenomAuthorityMetadataRequest {
   string denom = 1 [ (gogoproto.moretags) = "yaml:\"denom\"" ];
 }
+
+// QueryDenomAuthorityMetadataResponse defines the response structure for the
+// DenomAuthorityMetadata gRPC query.
 message QueryDenomAuthorityMetadataResponse {
   DenomAuthorityMetadata authority_metadata = 1 [
     (gogoproto.moretags) = "yaml:\"authority_metadata\"",
     (gogoproto.nullable) = false
   ];
 }
 
+// QueryDenomsFromCreatorRequest defines the request structure for the
+// DenomsFromCreator gRPC query.
 message QueryDenomsFromCreatorRequest {
   string creator = 1 [ (gogoproto.moretags) = "yaml:\"creator\"" ];
 }
+
+// QueryDenomsFromCreatorRequest defines the response structure for the
+// DenomsFromCreator gRPC query.
 message QueryDenomsFromCreatorResponse {
   repeated string denoms = 1 [ (gogoproto.moretags) = "yaml:\"denoms\"" ];
 }

proto/osmosis/tokenfactory/v1beta1/tx.proto

@@ -6,24 +6,27 @@ import "cosmos/base/v1beta1/coin.proto";
 
 option go_package = "github.com/osmosis-labs/osmosis/v7/x/tokenfactory/types";
 
-// Msg defines the Msg service.
+// Msg defines the tokefactory module's gRPC message service.
 service Msg {
   rpc CreateDenom(MsgCreateDenom) returns (MsgCreateDenomResponse);
   rpc Mint(MsgMint) returns (MsgMintResponse);
   rpc Burn(MsgBurn) returns (MsgBurnResponse);
+  rpc ChangeAdmin(MsgChangeAdmin) returns (MsgChangeAdminResponse);
+
   // ForceTransfer is deactivated for now because we need to think through edge
   // cases rpc ForceTransfer(MsgForceTransfer) returns
   // (MsgForceTransferResponse);
-  rpc ChangeAdmin(MsgChangeAdmin) returns (MsgChangeAdminResponse);
 }
 
-// MsgCreateDenom is the sdk.Msg type for allowing an account to create
-// a new denom. It requires a sender address and a subdenomination.
-// The (sender_address, sub_denomination) pair must be unique and cannot be
-// re-used. The resulting denom created is `factory/{creator
-// address}/{subdenom}`. The resultant denom's admin is originally set to be the
-// creator, but this can be changed later. The token denom does not indicate the
-// current admin.
+// MsgCreateDenom defines the message structure for the CreateDenom gRPC service
+// method. It allows an account to create a new denom. It requires a sender
+// address and a sub denomination. The (sender_address, sub_denomination) tuple
+// must be unique and cannot be re-used.
+//
+// The resulting denom created is defined as
+// <factory/{creatorAddress}/{subdenom}>. The resulting denom's admin is
+// originally set to be the creator, but this can be changed later. The token
+// denom does not indicate the current admin.
 message MsgCreateDenom {
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   // subdenom can be up to 44 "alphanumeric" characters long.
@@ -61,7 +64,18 @@ message MsgBurn {
 
 message MsgBurnResponse {}
 
-// // ===================== MsgForceTransfer
+// MsgChangeAdmin is the sdk.Msg type for allowing an admin account to reassign
+// adminship of a denom to a new account
+message MsgChangeAdmin {
+  string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
+  string denom = 2 [ (gogoproto.moretags) = "yaml:\"denom\"" ];
+  string new_admin = 3 [ (gogoproto.moretags) = "yaml:\"new_admin\"" ];
+}
+
+// MsgChangeAdminResponse defines the response structure for an executed
+// MsgChangeAdmin message.
+message MsgChangeAdminResponse {}
+
 // message MsgForceTransfer {
 //   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
 //   cosmos.base.v1beta1.Coin amount = 2 [
@@ -75,13 +89,3 @@ message MsgBurnResponse {}
 // }
 
 // message MsgForceTransferResponse {}
-
-// MsgChangeAdmin is the sdk.Msg type for allowing an admin account to reassign
-// adminship of a denom to a new account
-message MsgChangeAdmin {
-  string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
-  string denom = 2 [ (gogoproto.moretags) = "yaml:\"denom\"" ];
-  string new_admin = 3 [ (gogoproto.moretags) = "yaml:\"new_admin\"" ];
-}
-
-message MsgChangeAdminResponse {}

x/tokenfactory/README.md

@@ -11,33 +11,43 @@ created denom. Once a denom is created, the original creator is given
 - Mint their denom to any account
 - Burn their denom from any account
 - Create a transfer of their denom between any two accounts
-- Change the admin In the future, more admin capabilities may be
-    added. Admins can choose to share admin privileges with other
-    accounts using the authz module. The `ChangeAdmin` functionality,
-    allows changing the master admin account, or even setting it to
-    `""`, meaning no account has admin privileges of the asset.
-
+- Change the admin. In the future, more admin capabilities may be added. Admins
+  can choose to share admin privileges with other accounts using the authz
+  module. The `ChangeAdmin` functionality, allows changing the master admin
+  account, or even setting it to `""`, meaning no account has admin privileges
+  of the asset.
 
 ## Messages
 
 ### CreateDenom
-- Creates a denom of `factory/{creator address}/{subdenom}` given the denom creator address and the subdenom. Subdenoms can contain `[a-zA-Z0-9./]`.
-``` {.go}
+
+Creates a denom of `factory/{creator address}/{subdenom}` given the denom creator
+address and the subdenom. Subdenoms can contain `[a-zA-Z0-9./]`.
+
+```go
 message MsgCreateDenom {
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   string subdenom = 2 [ (gogoproto.moretags) = "yaml:\"subdenom\"" ];
 }
 ```
 
 **State Modifications:**
-- Fund community pool with the denom creation fee from the creator address, set in `Params`
-- Set `DenomMetaData` via bank keeper
-- Set `AuthorityMetadata` for the given denom to store the admin for the created denom `factory/{creator address}/{subdenom}`. Admin is automatically set as the Msg sender
-- Add denom to the `CreatorPrefixStore`, where a state of denoms created per creator is kept
+
+- Fund community pool with the denom creation fee from the creator address, set
+  in `Params`.
+- Set `DenomMetaData` via bank keeper.
+- Set `AuthorityMetadata` for the given denom to store the admin for the created
+  denom `factory/{creator address}/{subdenom}`. Admin is automatically set as the
+  Msg sender.
+- Add denom to the `CreatorPrefixStore`, where a state of denoms created per
+  creator is kept.
 
 ### Mint
-- Minting of a specific denom is only allowed for the creator of the denom registered during `CreateDenom`
-``` {.go}
+
+Minting of a specific denom is only allowed for the creator of the denom
+registered during `CreateDenom`.
+
+```go
 message MsgMint {
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   cosmos.base.v1beta1.Coin amount = 2 [
@@ -48,16 +58,18 @@ message MsgMint {
 ```
 
 **State Modifications:**
+
 - Safety check the following
   - Check that the denom minting is created via `tokenfactory` module
   - Check that the sender of the message is the admin of the denom
 - Mint designated amount of tokens for the denom via `bank` module
 
+### Burn
 
+Burning of a specific denom is only allowed for the creator of the denom
+registered during `CreateDenom`.
 
-### Burn
-- Burning of a specific denom is only allowed for the creator of the denom registered during `CreateDenom`
-``` {.go}
+```go
 message MsgBurn {
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   cosmos.base.v1beta1.Coin amount = 2 [
@@ -68,15 +80,18 @@ message MsgBurn {
 ```
 
 **State Modifications:**
+
 - Saftey check the following
   - Check that the denom minting is created via `tokenfactory` module
   - Check that the sender of the message is the admin of the denom
 - Burn designated amount of tokens for the denom via `bank` module
 
-
 ### ChangeAdmin
-- Burning of a specific denom is only allowed for the creator of the denom registered during `CreateDenom`
-``` {.go}
+
+Burning of a specific denom is only allowed for the creator of the denom
+registered during `CreateDenom`.
+
+```go
 message MsgChangeAdmin {
   string sender = 1 [ (gogoproto.moretags) = "yaml:\"sender\"" ];
   string denom = 2 [ (gogoproto.moretags) = "yaml:\"denom\"" ];
@@ -85,33 +100,45 @@ message MsgChangeAdmin {
 ```
 
 **State Modifications:**
+
 - Check that sender of the message is the admin of denom
 - Modify `AuthorityMetadata` state entry to change the admin of the denom
 
 ## Expectations from the chain
 
 The chain's bech32 prefix for addresses can be at most 16 characters long.
 
-This comes from denoms having a 128 byte maximum length, enforced from the SDK, and us setting longest_subdenom to be 44 bytes.
-A token factory token's denom is:
-`factory/{creator address}/{subdenom}`
+This comes from denoms having a 128 byte maximum length, enforced from the SDK,
+and us setting longest_subdenom to be 44 bytes.
+
+A token factory token's denom is: `factory/{creator address}/{subdenom}`
+
 Splitting up into sub-components, this has:
-* `len(factory) = 7`
-* `2 * len("/") = 2`
-* `len(longest_subdenom)`
-* `len(creator_address) = len(bech32(longest_addr_length, chain_addr_prefix))`.
-Longest addr length at the moment is `32 bytes`.
-Due to SDK error correction settings, this means `len(bech32(32, chain_addr_prefix)) = len(chain_addr_prefix) + 1 + 58`.
+
+- `len(factory) = 7`
+- `2 * len("/") = 2`
+- `len(longest_subdenom)`
+- `len(creator_address) = len(bech32(longest_addr_length, chain_addr_prefix))`.
+
+Longest addr length at the moment is `32 bytes`. Due to SDK error correction
+settings, this means `len(bech32(32, chain_addr_prefix)) = len(chain_addr_prefix) + 1 + 58`.
 Adding this all, we have a total length constraint of `128 = 7 + 2 + len(longest_subdenom) + len(longest_chain_addr_prefix) + 1 + 58`.
 Therefore `len(longest_subdenom) + len(longest_chain_addr_prefix) = 128 - (7 + 2 + 1 + 58) = 60`.
 
-The choice between how we standardized the split these 60 bytes between maxes from longest_subdenom and longest_chain_addr_prefix is somewhat arbitrary. Considerations going into this:
-* Per [BIP-0173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#bech32) the technically longest HRP for a 32 byte address ('data field') is 31 bytes. (Comes from encode(data) = 59 bytes, and max length = 90 bytes)
-* subdenom should be at least 32 bytes so hashes can go into it
-* longer subdenoms are very helpful for creating human readable denoms
-* chain addresses should prefer being smaller. The longest HRP in cosmos to date is 11 bytes. (`persistence`)
+The choice between how we standardized the split these 60 bytes between maxes
+from longest_subdenom and longest_chain_addr_prefix is somewhat arbitrary.
+Considerations going into this:
+
+- Per [BIP-0173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#bech32)
+  the technically longest HRP for a 32 byte address ('data field') is 31 bytes.
+  (Comes from encode(data) = 59 bytes, and max length = 90 bytes)
+- subdenom should be at least 32 bytes so hashes can go into it
+- longer subdenoms are very helpful for creating human readable denoms
+- chain addresses should prefer being smaller. The longest HRP in cosmos to date is 11 bytes. (`persistence`)
 
 For explicitness, its currently set to `len(longest_subdenom) = 44` and `len(longest_chain_addr_prefix) = 16`.
 
-Please note, if the SDK increases the maximum length of a denom from 128 bytes, these caps should increase.
-So please don't make code rely on these max lengths for parsing.
+Please note, if the SDK increases the maximum length of a denom from 128 bytes,
+these caps should increase.
+
+So please don't make code rely on these max lengths for parsing.