Merge pull request #587 from neutron-org/feat/whitelist-tf-hooks

Feat: Whitelist tokenfactory hooks

docs/static/swagger.yaml

@@ -18637,17 +18637,7 @@ definitions:
             type: array
         type: object
     type: object
-  osmosis.tokenfactory.v1beta1.DenomAuthorityMetadata:
-    description: |-
-      DenomAuthorityMetadata specifies metadata for addresses that have specific
-      capabilities over a token factory denom. Right now there is only one Admin
-      permission, but is planned to be extended to the future.
-    properties:
-      Admin:
-        title: Can be empty for no admin, or a valid osmosis address
-        type: string
-    type: object
-  osmosis.tokenfactory.v1beta1.Params:
+  osmosis.tokenfactory.Params:
     description: Params defines the parameters for the tokenfactory module.
     properties:
       denom_creation_fee:
@@ -18686,6 +18676,52 @@ definitions:
 
           are sent to
         type: string
+      whitelisted_hooks:
+        items:
+          properties:
+            code_id:
+              format: uint64
+              type: string
+            denom_creator:
+              type: string
+          title: >-
+            WhitelistedHook describes a beforeSendHook which is allowed to be
+            added and executed
+
+            SetBeforeSendHook can only be called on denoms where the denom
+            creator and
+
+            code_id for the `contract_addr` match a WhitelistedHook
+          type: object
+        title: >-
+          whitelisted_hooks is the list of hooks which are allowed to be added
+          and executed
+        type: array
+    type: object
+  osmosis.tokenfactory.WhitelistedHook:
+    properties:
+      code_id:
+        format: uint64
+        type: string
+      denom_creator:
+        type: string
+    title: >-
+      WhitelistedHook describes a beforeSendHook which is allowed to be added
+      and executed
+
+      SetBeforeSendHook can only be called on denoms where the denom creator and
+
+      code_id for the `contract_addr` match a WhitelistedHook
+    type: object
+  osmosis.tokenfactory.v1beta1.DenomAuthorityMetadata:
+    description: |-
+      DenomAuthorityMetadata specifies metadata for addresses that have specific
+      capabilities over a token factory denom. Right now there is only one Admin
+      permission, but is planned to be extended to the future.
+    properties:
+      Admin:
+        title: Can be empty for no admin, or a valid osmosis address
+        type: string
     type: object
   osmosis.tokenfactory.v1beta1.QueryBeforeSendHookAddressResponse:
     description: |-
@@ -18774,6 +18810,27 @@ definitions:
 
               are sent to
             type: string
+          whitelisted_hooks:
+            items:
+              properties:
+                code_id:
+                  format: uint64
+                  type: string
+                denom_creator:
+                  type: string
+              title: >-
+                WhitelistedHook describes a beforeSendHook which is allowed to
+                be added and executed
+
+                SetBeforeSendHook can only be called on denoms where the denom
+                creator and
+
+                code_id for the `contract_addr` match a WhitelistedHook
+              type: object
+            title: >-
+              whitelisted_hooks is the list of hooks which are allowed to be
+              added and executed
+            type: array
         type: object
     type: object
   packetforward.v1.Params:
@@ -43742,6 +43799,27 @@ paths:
 
                       are sent to
                     type: string
+                  whitelisted_hooks:
+                    items:
+                      properties:
+                        code_id:
+                          format: uint64
+                          type: string
+                        denom_creator:
+                          type: string
+                      title: >-
+                        WhitelistedHook describes a beforeSendHook which is
+                        allowed to be added and executed
+
+                        SetBeforeSendHook can only be called on denoms where the
+                        denom creator and
+
+                        code_id for the `contract_addr` match a WhitelistedHook
+                      type: object
+                    title: >-
+                      whitelisted_hooks is the list of hooks which are allowed
+                      to be added and executed
+                    type: array
                 type: object
             type: object
         default:

network/init-neutrond.sh

@@ -721,7 +721,7 @@ echo $DAO_CONTRACT_ADDRESS_B64
 
 set_genesis_param admins                                 "[\"$NEUTRON_CHAIN_MANAGER_CONTRACT_ADDRESS\"]"  # admin module
 set_genesis_param treasury_address                       "\"$DAO_CONTRACT_ADDRESS\""                      # feeburner
-set_genesis_param fee_collector_address                  "\"$DAO_CONTRACT_ADDRESS\""                      # tokenfactory
+set_genesis_param fee_collector_address                  "\"$DAO_CONTRACT_ADDRESS\","                      # tokenfactory
 set_genesis_param security_address                       "\"$SECURITY_SUBDAO_CORE_CONTRACT_ADDRESS\","    # cron
 set_genesis_param limit                                  5                                                # cron
 #set_genesis_param allow_messages                        "[\"*\"]"                                        # interchainaccounts

proto/osmosis/tokenfactory/params.proto

@@ -0,0 +1,43 @@
+syntax = "proto3";
+package osmosis.tokenfactory;
+
+import "cosmos/base/v1beta1/coin.proto";
+import "cosmos_proto/cosmos.proto";
+import "gogoproto/gogo.proto";
+
+option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
+
+// WhitelistedHook describes a beforeSendHook which is allowed to be added and executed
+// SetBeforeSendHook can only be called on denoms where the denom creator and
+// code_id for the `contract_addr` match a WhitelistedHook
+message WhitelistedHook {
+  uint64 code_id = 1 [(gogoproto.customname) = "CodeID"];
+  string denom_creator = 2;
+}
+
+// Params defines the parameters for the tokenfactory module.
+message Params {
+  // DenomCreationFee defines the fee to be charged on the creation of a new
+  // denom. The fee is drawn from the MsgCreateDenom's sender account, and
+  // transferred to the community pool.
+  repeated cosmos.base.v1beta1.Coin denom_creation_fee = 1 [
+    (gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
+    (gogoproto.moretags) = "yaml:\"denom_creation_fee\"",
+    (gogoproto.nullable) = false
+  ];
+
+  // DenomCreationGasConsume defines the gas cost for creating a new denom.
+  // This is intended as a spam deterrence mechanism.
+  //
+  // See: https://github.com/CosmWasm/token-factory/issues/11
+  uint64 denom_creation_gas_consume = 2 [
+    (gogoproto.moretags) = "yaml:\"denom_creation_gas_consume\"",
+    (gogoproto.nullable) = true
+  ];
+
+  // FeeCollectorAddress is the address where fees collected from denom creation
+  // are sent to
+  string fee_collector_address = 3;
+  // whitelisted_hooks is the list of hooks which are allowed to be added and executed
+  repeated WhitelistedHook whitelisted_hooks = 4;
+}

proto/osmosis/tokenfactory/v1beta1/genesis.proto

@@ -2,8 +2,8 @@ syntax = "proto3";
 package osmosis.tokenfactory.v1beta1;
 
 import "gogoproto/gogo.proto";
+import "osmosis/tokenfactory/params.proto";
 import "osmosis/tokenfactory/v1beta1/authorityMetadata.proto";
-import "osmosis/tokenfactory/v1beta1/params.proto";
 
 option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
 

proto/osmosis/tokenfactory/v1beta1/params.proto

@@ -5,7 +5,7 @@ import "cosmos/base/v1beta1/coin.proto";
 import "cosmos_proto/cosmos.proto";
 import "gogoproto/gogo.proto";
 
-option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
+option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types/v1beta1";
 
 // Params defines the parameters for the tokenfactory module.
 message Params {

proto/osmosis/tokenfactory/v1beta1/query.proto

@@ -3,8 +3,8 @@ package osmosis.tokenfactory.v1beta1;
 
 import "gogoproto/gogo.proto";
 import "google/api/annotations.proto";
+import "osmosis/tokenfactory/params.proto";
 import "osmosis/tokenfactory/v1beta1/authorityMetadata.proto";
-import "osmosis/tokenfactory/v1beta1/params.proto";
 
 option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
 

proto/osmosis/tokenfactory/v1beta1/tx.proto

@@ -7,7 +7,7 @@ import "cosmos/base/v1beta1/coin.proto";
 import "cosmos/msg/v1/msg.proto";
 import "cosmos_proto/cosmos.proto";
 import "gogoproto/gogo.proto";
-import "osmosis/tokenfactory/v1beta1/params.proto";
+import "osmosis/tokenfactory/params.proto";
 
 option go_package = "github.com/neutron-org/neutron/v4/x/tokenfactory/types";
 

wasmbinding/test/custom_message_test.go

@@ -76,6 +76,7 @@ func (suite *CustomMessengerTestSuite) SetupTest() {
 		sdk.NewCoins(sdk.NewInt64Coin(params.DefaultDenom, 10_000_000)),
 		0,
 		FeeCollectorAddress,
+		tokenfactorytypes.DefaultWhitelistedHooks,
 	))
 	suite.Require().NoError(err)
 

wasmbinding/test/custom_query_test.go

@@ -255,6 +255,7 @@ func (suite *CustomQuerierTestSuite) TestDenomAdmin() {
 		sdk.NewCoins(sdk.NewInt64Coin(params.DefaultDenom, 10_000_000)),
 		0,
 		FeeCollectorAddress,
+		tokenfactorytypes.DefaultWhitelistedHooks,
 	))
 	suite.Require().NoError(err)
 

x/tokenfactory/client/cli/query.go

@@ -27,6 +27,7 @@ func GetQueryCmd() *cobra.Command {
 		GetParams(),
 		GetCmdDenomAuthorityMetadata(),
 		GetCmdDenomsFromCreator(),
+		GetCmdBeforeSendHook(),
 	)
 
 	return cmd
@@ -124,10 +125,10 @@ func GetCmdDenomsFromCreator() *cobra.Command {
 	return cmd
 }
 
-// GetCmdDenomAuthorityMetadata returns the authority metadata for a queried denom
+// GetCmdBeforeSendHook returns the BeforeSendHook for a queried denom
 func GetCmdBeforeSendHook() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:   "before-send-hiik [denom] [flags]",
+		Use:   "before-send-hook [denom] [flags]",
 		Short: "Get the before send hook for a specific denom",
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -137,15 +138,15 @@ func GetCmdBeforeSendHook() *cobra.Command {
 			}
 			queryClient := types.NewQueryClient(clientCtx)
 
-			denom := strings.Split(args[0], "/")
-
-			if len(denom) != 3 {
-				return fmt.Errorf("invalid denom format, expected format: factory/[creator]/[subdenom]")
+			denom := args[0]
+			creator, subdenom, err := types.DeconstructDenom(denom)
+			if err != nil {
+				return err
 			}
 
 			res, err := queryClient.BeforeSendHookAddress(cmd.Context(), &types.QueryBeforeSendHookAddressRequest{
-				Creator:  denom[1],
-				Subdenom: denom[2],
+				Creator:  creator,
+				Subdenom: subdenom,
 			})
 			if err != nil {
 				return err

x/tokenfactory/keeper/before_send.go

@@ -68,6 +68,27 @@ func (k Keeper) Hooks() Hooks {
 	return Hooks{k}
 }
 
+func (k Keeper) AssertIsHookWhitelisted(ctx sdk.Context, denom string, contractAddress sdk.AccAddress) error {
+	contractInfo := k.contractKeeper.GetContractInfo(ctx, contractAddress)
+	if contractInfo == nil {
+		return types.ErrBeforeSendHookNotWhitelisted.Wrapf("contract with address (%s) does not exist", contractAddress.String())
+	}
+	codeID := contractInfo.CodeID
+	whitelistedHooks := k.GetParams(ctx).WhitelistedHooks
+	denomCreator, _, err := types.DeconstructDenom(denom)
+	if err != nil {
+		return types.ErrBeforeSendHookNotWhitelisted.Wrapf("invalid denom: %s", denom)
+	}
+
+	for _, hook := range whitelistedHooks {
+		if hook.CodeID == codeID && hook.DenomCreator == denomCreator {
+			return nil
+		}
+	}
+
+	return types.ErrBeforeSendHookNotWhitelisted.Wrapf("no whitelist for contract with codeID (%d) and denomCreator (%s) ", codeID, denomCreator)
+}
+
 // TrackBeforeSend calls the before send listener contract suppresses any errors
 func (h Hooks) TrackBeforeSend(ctx context.Context, from, to sdk.AccAddress, amount sdk.Coins) {
 	_ = h.k.callBeforeSendListener(ctx, from, to, amount, false)
@@ -99,6 +120,20 @@ func (k Keeper) callBeforeSendListener(ctx context.Context, from, to sdk.AccAddr
 				return err
 			}
 
+			// Do not invoke hook if denom is not whitelisted
+			// NOTE: hooks must already be whitelisted before they can be added, so under normal operation this check should never fail.
+			// It is here as an emergency override if we want to shutoff a hook. We do not return the error because once it is removed from the whitelist
+			// a hook should not be able to block a send.
+			if err := k.AssertIsHookWhitelisted(c, coin.Denom, cwAddr); err != nil {
+				c.Logger().Error(
+					"Skipped hook execution due to missing whitelist",
+					"err", err,
+					"denom", coin.Denom,
+					"contract", cwAddr.String(),
+				)
+				continue
+			}
+
 			var msgBz []byte
 
 			// get msgBz, either BlockBeforeSend or TrackBeforeSend
@@ -127,17 +162,16 @@ func (k Keeper) callBeforeSendListener(ctx context.Context, from, to sdk.AccAddr
 			if err != nil {
 				return err
 			}
-			em := sdk.NewEventManager()
 
 			// if its track before send, apply gas meter to prevent infinite loop
 			if blockBeforeSend {
-				_, err = k.contractKeeper.Sudo(c.WithEventManager(em), cwAddr, msgBz)
+				_, err = k.contractKeeper.Sudo(c, cwAddr, msgBz)
 				if err != nil {
 					return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
 				}
 			} else {
 				childCtx := c.WithGasMeter(types2.NewGasMeter(types.TrackBeforeSendGasLimit))
-				_, err = k.contractKeeper.Sudo(childCtx.WithEventManager(em), cwAddr, msgBz)
+				_, err = k.contractKeeper.Sudo(childCtx, cwAddr, msgBz)
 				if err != nil {
 					return errorsmod.Wrapf(err, "failed to call before send hook for denom %s", coin.Denom)
 				}