chore: backport #778 (#868)

Co-authored-by: Chenyao Yu <[email protected]>
skip-mev · Dec 11, 2024 · 3f78e9b · 3f78e9b
1 parent 9eb4eeb
commit 3f78e9b
Show file tree

Hide file tree

Showing 11 changed files with 200 additions and 199 deletions.
diff --git a/api/slinky/marketmap/v1/tx.pulsar.go b/api/slinky/marketmap/v1/tx.pulsar.go
diff --git a/proto/slinky/marketmap/v1/tx.proto b/proto/slinky/marketmap/v1/tx.proto
@@ -143,11 +143,11 @@ message MsgRemoveMarketAuthoritiesResponse {}
 // MsgRemoveMarkets defines the Msg/RemoveMarkets request type. It contains the
 // new markets to be removed from the market map.
 message MsgRemoveMarkets {
-  option (cosmos.msg.v1.signer) = "admin";
+  option (cosmos.msg.v1.signer) = "authority";
 
-  // Admin defines the authority that is the x/marketmap
-  // Admin account.  This account is set in the module parameters.
-  string admin = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ];
+  // Authority is the signer of this transaction.  This authority must be
+  // authorized by the module to execute the message.
+  string authority = 1 [ (cosmos_proto.scalar) = "cosmos.AddressString" ];
 
   // Markets is the list of markets to remove.
   repeated string markets = 2;

diff --git a/tests/integration/slinky_setup.go b/tests/integration/slinky_setup.go
@@ -482,8 +482,8 @@ func (s *SlinkyIntegrationSuite) RemoveMarket(
 	}
 
 	msg := &mmtypes.MsgRemoveMarkets{
-		Admin:   s.user.FormattedAddress(),
-		Markets: marketString,
+		Authority: s.user.FormattedAddress(),
+		Markets:   marketString,
 	}
 
 	tx := CreateTx(s.T(), s.chain, s.user, gasPrice, msg)

diff --git a/x/marketmap/keeper/msg_server.go b/x/marketmap/keeper/msg_server.go
@@ -263,13 +263,9 @@ func (ms msgServer) RemoveMarkets(
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	params, err := ms.k.GetParams(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	if msg.Admin != params.Admin {
-		return nil, fmt.Errorf("request admin %s does not match module admin %s", msg.Admin, params.Admin)
+	// perform basic msg validity checks
+	if err := ms.verifyMarketAuthorities(ctx, msg); err != nil {
+		return nil, fmt.Errorf("unable to verify market authorities: %w", err)
 	}
 
 	deletedMarkets := make([]string, 0, len(msg.Markets))
@@ -283,6 +279,10 @@ func (ms msgServer) RemoveMarkets(
 			ctx.Logger().Info(fmt.Sprintf("deleted market %s", market))
 			deletedMarkets = append(deletedMarkets, market)
 		}
+
+		if err := ms.k.hooks.AfterMarketRemoved(ctx, market); err != nil {
+			return nil, fmt.Errorf("unable to run market removal hook: %w", err)
+		}
 	}
 
 	// check if the resulting state is valid: it may not be valid if the removed market is used as a normalization pair

diff --git a/x/marketmap/keeper/msg_server_test.go b/x/marketmap/keeper/msg_server_test.go
@@ -531,7 +531,7 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 
 	s.Run("unable to process for invalid authority", func() {
 		msg := &types.MsgRemoveMarkets{
-			Admin: "invalid",
+			Authority: "invalid",
 		}
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)
 		s.Require().Error(err)
@@ -540,8 +540,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 
 	s.Run("only remove existing markets - no error", func() {
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{"BTC/USD", "ETH/USDT"},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{"BTC/USD", "ETH/USDT"},
 		}
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)
 		s.Require().NoError(err)
@@ -550,8 +550,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 
 	s.Run("unable to remove non-existent market - single", func() {
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{"BTC/USD"},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{"BTC/USD"},
 		}
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)
 		s.Require().NoError(err)
@@ -563,8 +563,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 		copyBTC.Ticker.Enabled = false
 
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyBTC.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyBTC.Ticker.String()},
 		}
 
 		err := s.keeper.CreateMarket(s.ctx, copyBTC)
@@ -587,8 +587,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 		s.Require().NoError(err)
 
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyBTC.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyBTC.Ticker.String()},
 		}
 
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)
@@ -638,13 +638,13 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 		s.Require().NoError(err)
 
 		msgRemoveBTC := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyBTC.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyBTC.Ticker.String()},
 		}
 
 		msgRemoveETH := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyETH.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyETH.Ticker.String()},
 		}
 
 		resp, err := msgServer.RemoveMarkets(s.ctx, msgRemoveBTC)
@@ -673,8 +673,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 		s.Require().NoError(err)
 
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyBTC.Ticker.String(), copyETH.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyBTC.Ticker.String(), copyETH.Ticker.String()},
 		}
 
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)
@@ -713,8 +713,8 @@ func (s *KeeperTestSuite) TestMsgServerRemoveMarkets() {
 		s.Require().NoError(err)
 
 		msg := &types.MsgRemoveMarkets{
-			Admin:   s.admin,
-			Markets: []string{copyBTC.Ticker.String(), copyETH.Ticker.String()},
+			Authority: s.marketAuthorities[0],
+			Markets:   []string{copyBTC.Ticker.String(), copyETH.Ticker.String()},
 		}
 
 		resp, err := msgServer.RemoveMarkets(s.ctx, msg)

diff --git a/x/marketmap/types/hooks.go b/x/marketmap/types/hooks.go
@@ -16,7 +16,7 @@ type MarketMapHooks interface {
 	AfterMarketGenesis(ctx sdk.Context, tickers map[string]Market) error
 
 	// AfterMarketRemoved is called after a market is removed.
-	AfterMarketRemoved(ctx sdk.Context, market Market) error
+	AfterMarketRemoved(ctx sdk.Context, key string) error
 }
 
 var _ MarketMapHooks = &MultiMarketMapHooks{}
@@ -58,9 +58,9 @@ func (mh MultiMarketMapHooks) AfterMarketGenesis(ctx sdk.Context, markets map[st
 }
 
 // AfterMarketRemoved calls all AfterMarketRemoved hooks registered to the MultiMarketMapHooks.
-func (mh MultiMarketMapHooks) AfterMarketRemoved(ctx sdk.Context, market Market) error {
+func (mh MultiMarketMapHooks) AfterMarketRemoved(ctx sdk.Context, key string) error {
 	for i := range mh {
-		if err := mh[i].AfterMarketRemoved(ctx, market); err != nil {
+		if err := mh[i].AfterMarketRemoved(ctx, key); err != nil {
 			return err
 		}
 	}
@@ -88,6 +88,6 @@ func (n *NoopMarketMapHooks) AfterMarketGenesis(_ sdk.Context, _ map[string]Mark
 	return nil
 }
 
-func (n *NoopMarketMapHooks) AfterMarketRemoved(_ sdk.Context, _ Market) error {
+func (n *NoopMarketMapHooks) AfterMarketRemoved(_ sdk.Context, _ string) error {
 	return nil
 }
diff --git a/x/marketmap/types/mocks/MarketMapHooks.go b/x/marketmap/types/mocks/MarketMapHooks.go
diff --git a/x/marketmap/types/msg.go b/x/marketmap/types/msg.go
@@ -132,7 +132,7 @@ func (m *MsgRemoveMarketAuthorities) ValidateBasic() error {
 // whether the signer is a valid acc-address.
 func (m *MsgRemoveMarkets) ValidateBasic() error {
 	// validate signer address
-	if _, err := sdk.AccAddressFromBech32(m.Admin); err != nil {
+	if _, err := sdk.AccAddressFromBech32(m.Authority); err != nil {
 		return err
 	}
 

diff --git a/x/marketmap/types/msg_test.go b/x/marketmap/types/msg_test.go
@@ -528,39 +528,39 @@ func TestValidateBasicMsgRemoveMarkets(t *testing.T) {
 		{
 			"if the Authority is not an acc-address - fail",
 			types.MsgRemoveMarkets{
-				Admin: "invalid",
+				Authority: "invalid",
 			},
 			false,
 		},
 		{
 			name: "invalid message (no markets) - fail",
 			msg: types.MsgRemoveMarkets{
-				Markets: nil,
-				Admin:   sample.Address(rng),
+				Markets:   nil,
+				Authority: sample.Address(rng),
 			},
 			expectPass: false,
 		},
 		{
 			name: "valid message - single market",
 			msg: types.MsgRemoveMarkets{
-				Markets: []string{"USDT/USD"},
-				Admin:   sample.Address(rng),
+				Markets:   []string{"USDT/USD"},
+				Authority: sample.Address(rng),
 			},
 			expectPass: true,
 		},
 		{
 			name: "valid message - multiple markets",
 			msg: types.MsgRemoveMarkets{
-				Markets: []string{"USDT/USD", "ETH/USD"},
-				Admin:   sample.Address(rng),
+				Markets:   []string{"USDT/USD", "ETH/USD"},
+				Authority: sample.Address(rng),
 			},
 			expectPass: true,
 		},
 		{
 			name: "invalid message (duplicate markets",
 			msg: types.MsgRemoveMarkets{
-				Markets: []string{"USDT/USD", "USDT/USD"},
-				Admin:   sample.Address(rng),
+				Markets:   []string{"USDT/USD", "USDT/USD"},
+				Authority: sample.Address(rng),
 			},
 			expectPass: false,
 		},