Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(authz): move ValidateBasic logic to msgServer #15754

Merged
merged 8 commits into from
Apr 8, 2023
Merged

refactor(authz): move ValidateBasic logic to msgServer #15754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ffc2ee7
refactor(authz): move `ValidateBasic` logic to `msgServer`
julienrbrt Apr 7, 2023
3c42e33
typo
julienrbrt Apr 7, 2023
d322ba3
test parity
julienrbrt Apr 7, 2023
6210824
remove duplicate tests and add checks in CLI
julienrbrt Apr 7, 2023
de07033
updates
julienrbrt Apr 7, 2023
b965c9d
fix e2e tests (as we've deleted tests, there is less now)
julienrbrt Apr 8, 2023
0a670cd
Merge branch 'main' into julien/validate-authz
julienrbrt Apr 8, 2023
36be83c
Merge branch 'main' into julien/validate-authz
julienrbrt Apr 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/e2e/authz/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (s *E2ETestSuite) TestQueryGranterGrantsGRPC() {
fmt.Sprintf("%s/cosmos/authz/v1beta1/grants/granter/%s", val.APIAddress, val.Address.String()),
false,
"",
8,
7,
},
}
for _, tc := range testCases {
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/authz/query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (s *E2ETestSuite) TestQueryGranterGrants() {
},
false,
"",
8,
7,
},
{
"valid case with pagination",
Expand Down
360 changes: 0 additions & 360 deletions tests/e2e/authz/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,366 +167,6 @@ var (
typeMsgSubmitProposal = sdk.MsgTypeURL(&govv1.MsgSubmitProposal{})
)

func (s *E2ETestSuite) TestCLITxGrantAuthorization() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many duplicated tests..

val := s.network.Validators[0]
grantee := s.grantee[0]

twoHours := time.Now().Add(time.Minute * 120).Unix()
pastHour := time.Now().Add(-time.Minute * 60).Unix()

testCases := []struct {
name string
args []string
expectedCode uint32
expectErr bool
expErrMsg string
}{
{
"Invalid granter Address",
[]string{
"grantee_addr",
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, "granter"),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
},
0,
true,
"key not found",
},
{
"Invalid grantee Address",
[]string{
"grantee_addr",
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
},
0,
true,
"invalid separator index",
},
{
"Invalid expiration time",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=true", flags.FlagBroadcastMode),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, pastHour),
},
0,
true,
"",
},
{
"fail with error invalid msg-type",
[]string{
grantee.String(),
"generic",
fmt.Sprintf("--%s=invalid-msg-type", cli.FlagMsgType),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
},
0x1d,
false,
"",
},
{
"failed with error both validators not allowed",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"cannot set both allowed & deny list",
},
{
"invalid bond denom for tx delegate authorization allowed validators",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"invalid denom",
},
{
"invalid bond denom for tx delegate authorization deny validators",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"invalid denom",
},
{
"invalid bond denom for tx undelegate authorization",
[]string{
grantee.String(),
"unbond",
fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"invalid denom",
},
{
"invalid bond denon for tx redelegate authorization",
[]string{
grantee.String(),
"redelegate",
fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"invalid denom",
},
{
"invalid decimal coin expression with more than single coin",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100stake,20xyz", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"invalid decimal coin expression",
},
{
"valid tx delegate authorization allowed validators",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"valid tx delegate authorization deny validators",
[]string{
grantee.String(),
"delegate",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"valid tx undelegate authorization",
[]string{
grantee.String(),
"unbond",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"valid tx redelegate authorization",
[]string{
grantee.String(),
"redelegate",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"Valid tx send authorization",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"Valid tx send authorization with allow list",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, s.grantee[1]),
},
0,
false,
"",
},
{
"Invalid tx send authorization with duplicate allow list",
[]string{
grantee.String(),
"send",
fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", cli.FlagAllowList, fmt.Sprintf("%s,%s", s.grantee[1], s.grantee[1])),
},
0,
true,
"duplicate entry",
},
{
"Valid tx generic authorization",
[]string{
grantee.String(),
"generic",
fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
false,
"",
},
{
"fail when granter = grantee",
[]string{
grantee.String(),
"generic",
fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
},
0,
true,
"grantee and granter should be different",
},
{
"Valid tx with amino",
[]string{
grantee.String(),
"generic",
fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", flags.FlagSignMode, flags.SignModeLegacyAminoJSON),
},
0,
false,
"",
},
}

for _, tc := range testCases {
s.Run(tc.name, func() {
out, err := authzclitestutil.CreateGrant(val.ClientCtx, tc.args)
if tc.expectErr {
s.Require().Error(err, out)
s.Require().Contains(err.Error(), tc.expErrMsg)
} else {
var txResp sdk.TxResponse
s.Require().NoError(err)
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &txResp), out.String())
s.Require().NoError(clitestutil.CheckTxCode(s.network, val.ClientCtx, txResp.TxHash, tc.expectedCode))
}
})
}
}

func execDelegate(val *network.Validator, args []string) (testutil.BufferWriter, error) {
cmd := stakingcli.NewDelegateCmd()
clientCtx := val.ClientCtx
Expand Down
4 changes: 4 additions & 0 deletions x/authz/authorization_grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func (g Grant) GetAuthorization() (Authorization, error) {
}

func (g Grant) ValidateBasic() error {
if g.Authorization == nil {
return sdkerrors.ErrInvalidType.Wrap("authorization is nil")
}

av := g.Authorization.GetCachedValue()
a, ok := av.(Authorization)
if !ok {
Expand Down
Loading