-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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.
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
fix: remove time.now check from authz #11129
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
c0f3519
Merge pull request from GHSA-2p6r-37p9-89p2
robert-zaremba a880e95
master update
robert-zaremba f8c86ca
update changelog and cli test
robert-zaremba 9825643
fix keeper tests
robert-zaremba 972a7b8
fix decoder test
robert-zaremba f2799e1
cleaning
robert-zaremba 2dd1818
wip - tests
robert-zaremba bed10a3
fix cli test
robert-zaremba 44acf3f
add a comment
robert-zaremba 5b1a54f
update TestMsgGrantAuthorization
robert-zaremba db3eeb7
Merge branch 'master' into robert/backport-0.44.2-master
robert-zaremba 3bf3409
udpate changelog
robert-zaremba 7163865
fix sims
aleem1314 b307813
Merge branch 'master' into aleem/11115-authz-sims
aleem1314 c5e211b
fix sims
aleem1314 d2045b1
update changelog
aleem1314 e5a3683
Update x/authz/authorization_grant.go
amaury1093 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
package authz | ||
|
||
import ( | ||
"testing" | ||
"time" | ||
|
||
"github.com/stretchr/testify/require" | ||
) | ||
|
||
// TODO: remove and use: robert/expect-error | ||
func expecError(r *require.Assertions, expected string, received error) { | ||
if expected == "" { | ||
r.NoError(received) | ||
} else { | ||
r.Error(received) | ||
r.Contains(received.Error(), expected) | ||
} | ||
} | ||
|
||
func TestNewGrant(t *testing.T) { | ||
a := NewGenericAuthorization("some-type") | ||
var tcs = []struct { | ||
title string | ||
a Authorization | ||
blockTime time.Time | ||
expire time.Time | ||
err string | ||
}{ | ||
{"wrong expire time (1)", a, time.Unix(10, 0), time.Unix(8, 0), "expiration must be after"}, | ||
{"wrong expire time (2)", a, time.Unix(10, 0), time.Unix(10, 0), "expiration must be after"}, | ||
{"good expire time (1)", a, time.Unix(10, 0), time.Unix(10, 1), ""}, | ||
{"good expire time (2)", a, time.Unix(10, 0), time.Unix(11, 0), ""}, | ||
} | ||
|
||
for _, tc := range tcs { | ||
tc := tc | ||
t.Run(tc.title, func(t *testing.T) { | ||
_, err := NewGrant(tc.blockTime, tc.a, tc.expire) | ||
expecError(require.New(t), tc.err, err) | ||
}) | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,13 +55,12 @@ func (s *TestSuite) TestKeeper() { | |
s.Require().Nil(authorization) | ||
s.Require().Equal(expiration, time.Time{}) | ||
now := s.ctx.BlockHeader().Time | ||
s.Require().NotNil(now) | ||
|
||
newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) | ||
s.T().Log("verify if expired authorization is rejected") | ||
x := &banktypes.SendAuthorization{SpendLimit: newCoins} | ||
err := app.AuthzKeeper.SaveGrant(ctx, granterAddr, granteeAddr, x, now.Add(-1*time.Hour)) | ||
s.Require().NoError(err) | ||
s.Require().Error(err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh wow, it seems that tests were incorrect |
||
authorization, _ = app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, bankSendAuthMsgType) | ||
s.Require().Nil(authorization) | ||
|
||
|
@@ -105,14 +104,13 @@ func (s *TestSuite) TestKeeperIter() { | |
authorization, expiration := app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, "Abcd") | ||
s.Require().Nil(authorization) | ||
s.Require().Equal(time.Time{}, expiration) | ||
now := s.ctx.BlockHeader().Time | ||
s.Require().NotNil(now) | ||
now := s.ctx.BlockHeader().Time.Add(time.Second) | ||
|
||
newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100)) | ||
s.T().Log("verify if expired authorization is rejected") | ||
x := &banktypes.SendAuthorization{SpendLimit: newCoins} | ||
err := app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, x, now.Add(-1*time.Hour)) | ||
s.Require().NoError(err) | ||
s.Require().Error(err) | ||
authorization, _ = app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, "abcd") | ||
s.Require().Nil(authorization) | ||
|
||
|
@@ -131,8 +129,7 @@ func (s *TestSuite) TestKeeperFees() { | |
granteeAddr := addrs[1] | ||
recipientAddr := addrs[2] | ||
s.Require().NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, sdk.NewCoins(sdk.NewInt64Coin("steak", 10000)))) | ||
now := s.ctx.BlockHeader().Time | ||
s.Require().NotNil(now) | ||
expiration := s.ctx.BlockHeader().Time.Add(1 * time.Second) | ||
|
||
smallCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 20)) | ||
someCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 123)) | ||
|
@@ -157,7 +154,7 @@ func (s *TestSuite) TestKeeperFees() { | |
|
||
s.T().Log("verify dispatch executes with correct information") | ||
// grant authorization | ||
err = app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, now) | ||
err = app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, expiration) | ||
s.Require().NoError(err) | ||
authorization, _ := app.AuthzKeeper.GetCleanAuthorization(s.ctx, granteeAddr, granterAddr, bankSendAuthMsgType) | ||
s.Require().NotNil(authorization) | ||
|
@@ -206,8 +203,7 @@ func (s *TestSuite) TestDispatchedEvents() { | |
granteeAddr := addrs[1] | ||
recipientAddr := addrs[2] | ||
require.NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, sdk.NewCoins(sdk.NewInt64Coin("steak", 10000)))) | ||
now := s.ctx.BlockHeader().Time | ||
require.NotNil(now) | ||
expiration := s.ctx.BlockHeader().Time.Add(1 * time.Second) // must be in the future | ||
|
||
smallCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 20)) | ||
msgs := authz.NewMsgExec(granteeAddr, []sdk.Msg{ | ||
|
@@ -219,7 +215,7 @@ func (s *TestSuite) TestDispatchedEvents() { | |
}) | ||
|
||
// grant authorization | ||
err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, now) | ||
err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, expiration) | ||
require.NoError(err) | ||
authorization, _ := app.AuthzKeeper.GetCleanAuthorization(s.ctx, granteeAddr, granterAddr, bankSendAuthMsgType) | ||
require.NotNil(authorization) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we continue here? I think we should return an error (or panic since we can't return).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking more about. It's OK to continue - the grant will be valid but it will be pruned immediately... I would issue warning though.