refactor(x/circuit): audit QA (#21370)

cosmos · Aug 26, 2024 · 9bc5df6 · 9bc5df6
1 parent 59e0a3d
commit 9bc5df6
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 8 deletions.
diff --git a/x/circuit/README.md b/x/circuit/README.md
@@ -19,7 +19,7 @@ https://github.com/cosmos/cosmos-sdk/blob/x/circuit/v0.1.0/x/circuit/ante/circui
 * With a [message router check](https://docs.cosmos.network/main/learn/advanced/baseapp#msg-service-router):
 
 ```go reference
-https://github.com/cosmos/cosmos-sdk/blob/v0.50.1/baseapp/msg_service_router.go#L104-L115
+https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/baseapp/msg_service_router.go#L123-L133
 ``` 
 
 :::note
@@ -101,7 +101,7 @@ Reset is called by an authorized account to enable execution for a specific msgU
 ### MsgAuthorizeCircuitBreaker
 
 ```protobuf reference
-https://github.com/cosmos/cosmos-sdk/blob/main/x/circuit/proto/cosmos/circuit/v1/tx.proto#L25-L40
+https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/circuit/proto/cosmos/circuit/v1/tx.proto#L25-L40
 ```
 
 This message is expected to fail if:
@@ -111,7 +111,7 @@ This message is expected to fail if:
 ### MsgTripCircuitBreaker
 
 ```protobuf reference 
-https://github.com/cosmos/cosmos-sdk/blob/main/x/circuit/proto/cosmos/circuit/v1/tx.proto#L47-L60
+https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/circuit/proto/cosmos/circuit/v1/tx.proto#L47-L60
 ```
 
 This message is expected to fail if:
@@ -121,7 +121,7 @@ This message is expected to fail if:
 ### MsgResetCircuitBreaker
 
 ```protobuf reference
-https://github.com/cosmos/cosmos-sdk/blob/main/x/circuit/proto/cosmos/circuit/v1/tx.proto#L67-L78
+https://github.com/cosmos/cosmos-sdk/blob/release/v0.52.x/x/circuit/proto/cosmos/circuit/v1/tx.proto#L67-L78
 ```
 
 This message is expected to fail if:

diff --git a/x/circuit/ante/circuit_test.go b/x/circuit/ante/circuit_test.go
@@ -31,7 +31,7 @@ type MockCircuitBreaker struct {
 	isAllowed bool
 }
 
-func (m MockCircuitBreaker) IsAllowed(ctx context.Context, typeURL string) (bool, error) {
+func (m MockCircuitBreaker) IsAllowed(_ context.Context, typeURL string) (bool, error) {
 	return typeURL == "/cosmos.circuit.v1.MsgAuthorizeCircuitBreaker", nil
 }
 

diff --git a/x/circuit/depinject.go b/x/circuit/depinject.go
@@ -8,6 +8,7 @@ import (
 	"cosmossdk.io/depinject/appconfig"
 	authtypes "cosmossdk.io/x/auth/types"
 	"cosmossdk.io/x/circuit/keeper"
+	"cosmossdk.io/x/circuit/types"
 
 	"github.com/cosmos/cosmos-sdk/baseapp"
 	"github.com/cosmos/cosmos-sdk/codec"
@@ -46,7 +47,7 @@ type ModuleOutputs struct {
 
 func ProvideModule(in ModuleInputs) ModuleOutputs {
 	// default to governance authority if not provided
-	authority := authtypes.NewModuleAddress("gov")
+	authority := authtypes.NewModuleAddress(types.GovModuleName)
 	if in.Config.Authority != "" {
 		authority = authtypes.NewModuleAddressOrBech32Address(in.Config.Authority)
 	}

diff --git a/x/circuit/keeper/genesis_test.go b/x/circuit/keeper/genesis_test.go
@@ -44,7 +44,7 @@ func (s *GenesisTestSuite) SetupTest() {
 	s.cdc = codec.NewProtoCodec(encCfg.InterfaceRegistry)
 	ac := addresscodec.NewBech32Codec("cosmos")
 
-	authority, err := ac.BytesToString(authtypes.NewModuleAddress("gov"))
+	authority, err := ac.BytesToString(authtypes.NewModuleAddress(types.GovModuleName))
 	s.Require().NoError(err)
 
 	bz, err := ac.StringToBytes(authority)

diff --git a/x/circuit/keeper/keeper_test.go b/x/circuit/keeper/keeper_test.go
@@ -46,7 +46,7 @@ func initFixture(t *testing.T) *fixture {
 	mockStoreKey := storetypes.NewKVStoreKey("test")
 
 	env := runtime.NewEnvironment(runtime.NewKVStoreService(mockStoreKey), coretesting.NewNopLogger())
-	authority, err := ac.BytesToString(authtypes.NewModuleAddress("gov"))
+	authority, err := ac.BytesToString(authtypes.NewModuleAddress(types.GovModuleName))
 	require.NoError(t, err)
 	k := keeper.NewKeeper(env, encCfg.Codec, authority, ac)
 
@@ -166,3 +166,41 @@ func TestIterateDisabledList(t *testing.T) {
 	require.Equal(t, mockMsgs[1], returnedDisabled[0])
 	require.Equal(t, mockMsgs[2], returnedDisabled[1])
 }
+
+func TestIsAllowed(t *testing.T) {
+	t.Parallel()
+	f := initFixture(t)
+
+	testCases := []struct {
+		name        string
+		msgURL      string
+		setDisabled bool
+		expected    bool
+	}{
+		{
+			name:        "allowed message",
+			msgURL:      "test_allowed",
+			setDisabled: false,
+			expected:    true,
+		},
+		{
+			name:        "disabled message",
+			msgURL:      "test_disabled",
+			setDisabled: true,
+			expected:    false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.setDisabled {
+				err := f.keeper.DisableList.Set(f.ctx, tc.msgURL)
+				require.NoError(t, err)
+			}
+
+			allowed, err := f.keeper.IsAllowed(f.ctx, tc.msgURL)
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, allowed)
+		})
+	}
+}
diff --git a/x/circuit/types/keys.go b/x/circuit/types/keys.go
@@ -8,6 +8,11 @@ const (
 
 	// StoreKey defines the primary module store key
 	StoreKey = ModuleName
+
+	// GovModuleName duplicates the gov module's name to avoid a cyclic dependency with x/gov.
+	// It should be synced with the gov module's name if it is ever changed.
+	// See: https://github.com/cosmos/cosmos-sdk/blob/b62a28aac041829da5ded4aeacfcd7a42873d1c8/x/gov/types/keys.go#L9
+	GovModuleName = "gov"
 )
 
 // KVStore keys