Skip to content

Commit

Permalink
refactor(x/circuit): audit QA (#21370)
Browse files Browse the repository at this point in the history
(cherry picked from commit 9bc5df6)
  • Loading branch information
lucaslopezf authored and mergify[bot] committed Aug 26, 2024
1 parent cb1b284 commit e5cde23
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 8 deletions.
8 changes: 4 additions & 4 deletions x/circuit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion x/circuit/ante/circuit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion x/circuit/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion x/circuit/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
40 changes: 39 additions & 1 deletion x/circuit/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
})
}
}
5 changes: 5 additions & 0 deletions x/circuit/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit e5cde23

Please sign in to comment.