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(x/authz): set environment in context #20502

Merged
merged 11 commits into from
Jun 3, 2024
Merged

refactor(x/authz): set environment in context #20502

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
383f683
refactor: set environment in context
julienrbrt May 31, 2024
5a8e9d7
Merge branch 'main' into julien/env-context
julienrbrt May 31, 2024
1c2a04c
authz
julienrbrt May 31, 2024
cd43a04
updates
julienrbrt May 31, 2024
7056b64
revert unrelated
julienrbrt May 31, 2024
89fed3e
`make lint-fix`
julienrbrt May 31, 2024
bda005f
fix linting
julienrbrt May 31, 2024
0a6ee40
lint
julienrbrt May 31, 2024
d91da61
Merge branch 'main' into julien/env-context
julienrbrt Jun 3, 2024
8c3f27e
updates
julienrbrt Jun 3, 2024
c975318
swap changelog
julienrbrt Jun 3, 2024
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
8 changes: 8 additions & 0 deletions UPGRADING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,14 @@ To learn more see the [docs](https://docs.cosmos.network/main/learn/advanced/tra

### Modules

<!-- create server/v2 changes docs and mention it here
* mention changes in tx validators
* mention changes with appmodulev2
* mention changes with sdk context removal
* mention changes with environment
* mention changes with environment in context in interfaces
-->

#### `**all**`

* [RFC 001](https://docs.cosmos.network/main/rfc/rfc-001-tx-validation) has defined a simplification of the message validation process for modules.
Expand Down
5 changes: 5 additions & 0 deletions core/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ const (
ExecModeKey contextKey = iota
CometInfoKey contextKey = iota
)

// EnvironmentContextKey is the context key for the environment.
// A caller should not assume the environment is available in each context.
// ref: https://github.com/cosmos/cosmos-sdk/issues/19640
var EnvironmentContextKey = struct{}{}
9 changes: 6 additions & 3 deletions x/authz/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/cosmos/gogoproto/proto"

"cosmossdk.io/core/appmodule"
corecontext "cosmossdk.io/core/context"
errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/authz"
Expand Down Expand Up @@ -83,8 +84,7 @@ func (k Keeper) update(ctx context.Context, grantee, granter sdk.AccAddress, upd
// grants from the message signer to the grantee.
func (k Keeper) DispatchActions(ctx context.Context, grantee sdk.AccAddress, msgs []sdk.Msg) ([][]byte, error) {
results := make([][]byte, len(msgs))
sdkCtx := sdk.UnwrapSDKContext(ctx)
now := sdkCtx.HeaderInfo().Time
now := k.Environment.HeaderService.HeaderInfo(ctx).Time

for i, msg := range msgs {
signers, _, err := k.cdc.GetMsgSigners(msg)
Expand Down Expand Up @@ -118,7 +118,10 @@ func (k Keeper) DispatchActions(ctx context.Context, grantee sdk.AccAddress, msg
return nil, err
}

resp, err := authorization.Accept(sdkCtx, msg)
// pass the environment in the context
// users on server/v2 are expected to unwrap the environment from the context
// users on baseapp can still unwrap the sdk context
resp, err := authorization.Accept(context.WithValue(ctx, corecontext.EnvironmentContextKey, k.Environment), msg)
Comment on lines +121 to +124
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

The search results confirm that the only remaining instances of authorization.Accept using the old context are in the test file x/bank/types/send_authorization_test.go. These test cases need to be updated to reflect the new context requirement.

  • Update the test cases in x/bank/types/send_authorization_test.go to pass the modified context, similar to the change in x/authz/keeper/keeper.go.
Analysis chain

Passing the environment in the context to the authorization.Accept method is a significant change. Ensure that all consumers of this method are aware of the new context requirement and handle it appropriately.

Consider documenting this change prominently in the module's README or in the function's GoDoc to ensure that all developers are aware of the new context handling.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all calls to `authorization.Accept` correctly pass the modified context.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go $'authorization.Accept'

Length of output: 445

Script:

#!/bin/bash
# Description: Verify the context handling in `x/bank/types/send_authorization_test.go` to ensure it aligns with the new context requirement.

# Test: Display the lines around the calls to `authorization.Accept` in the test file.
rg --type go --context 5 $'authorization.Accept' x/bank/types/send_authorization_test.go

Length of output: 968

Script:

#!/bin/bash
# Description: Verify if there are any other test files or usage of `authorization.Accept` that need similar updates.

# Test: Search for the function usage across the entire codebase to ensure all instances are updated.
rg --type go $'authorization.Accept'

Length of output: 445

if err != nil {
return nil, err
}
Expand Down
6 changes: 2 additions & 4 deletions x/authz/keys.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
package authz

const (
// ModuleName is the module name constant used in many places
ModuleName = "authz"
)
// ModuleName is the module name constant used in many places
const ModuleName = "authz"
13 changes: 11 additions & 2 deletions x/bank/types/send_authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"

"cosmossdk.io/core/address"
"cosmossdk.io/core/appmodule/v2"
corecontext "cosmossdk.io/core/context"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/authz"
Expand Down Expand Up @@ -40,12 +42,19 @@ func (a SendAuthorization) Accept(ctx context.Context, msg sdk.Msg) (authz.Accep
return authz.AcceptResponse{}, sdkerrors.ErrInsufficientFunds.Wrapf("requested amount is more than spend limit")
}

authzEnv, ok := ctx.Value(corecontext.EnvironmentContextKey).(appmodule.Environment)
if !ok {
return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrap("environment not set")
}

isAddrExists := false
toAddr := mSend.ToAddress
allowedList := a.GetAllowList()
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, addr := range allowedList {
sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "send authorization")
if err := authzEnv.GasService.GasMeter(ctx).Consume(gasCostPerIteration, "send authorization"); err != nil {
return authz.AcceptResponse{}, err
}

if addr == toAddr {
isAddrExists = true
break
Expand Down
35 changes: 33 additions & 2 deletions x/bank/types/send_authorization_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package types_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"

"cosmossdk.io/core/header"
"cosmossdk.io/core/appmodule/v2"
corecontext "cosmossdk.io/core/context"
coregas "cosmossdk.io/core/gas"
coreheader "cosmossdk.io/core/header"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/bank/types"
Expand All @@ -25,9 +29,36 @@ var (
unknownAddrStr = "cosmos1ta047h6lw4hxkmn0wah97h6lta0sml880l"
)

type headerService struct{}

func (h headerService) HeaderInfo(ctx context.Context) coreheader.Info {
return sdk.UnwrapSDKContext(ctx).HeaderInfo()
}

type mockGasService struct {
coregas.Service
}

func (m mockGasService) GasMeter(ctx context.Context) coregas.Meter {
return mockGasMeter{}
}

type mockGasMeter struct {
coregas.Meter
}

func (m mockGasMeter) Consume(amount coregas.Gas, descriptor string) error {
return nil
}

func TestSendAuthorization(t *testing.T) {
ac := codectestutil.CodecOptions{}.GetAddressCodec()
ctx := testutil.DefaultContextWithDB(t, storetypes.NewKVStoreKey(types.StoreKey), storetypes.NewTransientStoreKey("transient_test")).Ctx.WithHeaderInfo(header.Info{})
sdkCtx := testutil.DefaultContextWithDB(t, storetypes.NewKVStoreKey(types.StoreKey), storetypes.NewTransientStoreKey("transient_test")).Ctx.WithHeaderInfo(coreheader.Info{})
ctx := context.WithValue(sdkCtx.Context(), corecontext.EnvironmentContextKey, appmodule.Environment{
HeaderService: headerService{},
GasService: mockGasService{},
})

allowList := make([]sdk.AccAddress, 1)
allowList[0] = toAddr
authorization := types.NewSendAuthorization(coins1000, nil, ac)
Expand Down
17 changes: 14 additions & 3 deletions x/staking/types/authz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"

"cosmossdk.io/core/address"
"cosmossdk.io/core/appmodule"
corecontext "cosmossdk.io/core/context"
errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -100,11 +102,17 @@ func (a StakeAuthorization) Accept(ctx context.Context, msg sdk.Msg) (authz.Acce
return authz.AcceptResponse{}, sdkerrors.ErrInvalidRequest.Wrap("unknown msg type")
}

authzEnv, ok := ctx.Value(corecontext.EnvironmentContextKey).(appmodule.Environment)
if !ok {
return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrap("environment not set")
}
isValidatorExists := false
allowedList := a.GetAllowList().GetAddress()
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, validator := range allowedList {
sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization")
if err := authzEnv.GasService.GasMeter(ctx).Consume(gasCostPerIteration, "stake authorization"); err != nil {
return authz.AcceptResponse{}, err
}

if validator == validatorAddress {
isValidatorExists = true
break
Expand All @@ -113,7 +121,10 @@ func (a StakeAuthorization) Accept(ctx context.Context, msg sdk.Msg) (authz.Acce

denyList := a.GetDenyList().GetAddress()
for _, validator := range denyList {
sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization")
if err := authzEnv.GasService.GasMeter(ctx).Consume(gasCostPerIteration, "stake authorization"); err != nil {
return authz.AcceptResponse{}, err
}

if validator == validatorAddress {
return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf("cannot delegate/undelegate to %s validator", validator)
}
Expand Down
43 changes: 37 additions & 6 deletions x/staking/types/authz_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package types_test

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"cosmossdk.io/core/appmodule/v2"
corecontext "cosmossdk.io/core/context"
coregas "cosmossdk.io/core/gas"
coreheader "cosmossdk.io/core/header"
storetypes "cosmossdk.io/store/types"
stakingtypes "cosmossdk.io/x/staking/types"
Expand Down Expand Up @@ -39,10 +43,37 @@ func accAddressToString(t *testing.T, addr sdk.AccAddress) string {
return r
}

type headerService struct{}

func (h headerService) HeaderInfo(ctx context.Context) coreheader.Info {
return sdk.UnwrapSDKContext(ctx).HeaderInfo()
}

type mockGasService struct {
coregas.Service
}

func (m mockGasService) GasMeter(ctx context.Context) coregas.Meter {
return mockGasMeter{}
}

type mockGasMeter struct {
coregas.Meter
}

func (m mockGasMeter) Consume(amount coregas.Gas, descriptor string) error {
return nil
}

func TestAuthzAuthorizations(t *testing.T) {
key := storetypes.NewKVStoreKey(stakingtypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test"))
ctx := testCtx.Ctx.WithHeaderInfo(coreheader.Info{})
sdkCtx := testCtx.Ctx.WithHeaderInfo(coreheader.Info{})
ctx := context.WithValue(sdkCtx.Context(), corecontext.EnvironmentContextKey, appmodule.Environment{
HeaderService: headerService{},
GasService: mockGasService{},
})

valAddressCodec := codectestutil.CodecOptions{}.GetValidatorCodec()
// verify ValidateBasic returns error for the AUTHORIZATION_TYPE_UNSPECIFIED authorization type
delAuth, err := stakingtypes.NewStakeAuthorization([]sdk.ValAddress{val1, val2}, []sdk.ValAddress{}, stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_UNSPECIFIED, &coin100, valAddressCodec)
Expand Down Expand Up @@ -313,7 +344,7 @@ func TestAuthzAuthorizations(t *testing.T) {
[]sdk.ValAddress{},
stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_CANCEL_UNBONDING_DELEGATION,
&coin100,
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), ctx.HeaderInfo().Height, coin100),
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), sdkCtx.HeaderInfo().Height, coin100),
false,
true,
nil,
Expand All @@ -324,7 +355,7 @@ func TestAuthzAuthorizations(t *testing.T) {
[]sdk.ValAddress{},
stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_CANCEL_UNBONDING_DELEGATION,
&coin100,
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), ctx.HeaderInfo().Height, coin50),
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), sdkCtx.HeaderInfo().Height, coin50),
false,
false,
&stakingtypes.StakeAuthorization{
Expand All @@ -341,7 +372,7 @@ func TestAuthzAuthorizations(t *testing.T) {
[]sdk.ValAddress{},
stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_CANCEL_UNBONDING_DELEGATION,
&coin100,
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val3), ctx.HeaderInfo().Height, coin50),
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val3), sdkCtx.HeaderInfo().Height, coin50),
true,
false,
nil,
Expand All @@ -352,7 +383,7 @@ func TestAuthzAuthorizations(t *testing.T) {
[]sdk.ValAddress{},
stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_CANCEL_UNBONDING_DELEGATION,
nil,
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val2), ctx.HeaderInfo().Height, coin100),
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val2), sdkCtx.HeaderInfo().Height, coin100),
false,
false,
&stakingtypes.StakeAuthorization{
Expand All @@ -369,7 +400,7 @@ func TestAuthzAuthorizations(t *testing.T) {
[]sdk.ValAddress{val1},
stakingtypes.AuthorizationType_AUTHORIZATION_TYPE_CANCEL_UNBONDING_DELEGATION,
&coin100,
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), ctx.HeaderInfo().Height, coin100),
stakingtypes.NewMsgCancelUnbondingDelegation(accAddressToString(t, delAddr), valAddressToString(t, val1), sdkCtx.HeaderInfo().Height, coin100),
true,
false,
nil,
Expand Down
Loading