Environment in context

[julienrbrt]

With the migration of cosmos sdk modules to be compatible with server/v2, modules must not use sdk.UnwrapSDKContext.
The environment provides everything a module needs.

The goal is to keep cosmos sdk modules compatible with baseapp and server/v2, with the minimum api break possible.
However, there are a few places where the SDK context is still required, and it cannot be easily refactored by taking an environment due to the api/interface breakage this would bring.

x/feegrant

feegrant.FeeAllowanceI Accept method takes only a sdk.Context:

cosmos-sdk/x/feegrant/basic_fee.go

Lines 25 to 26 in 3e63309

	func (a *BasicAllowance) Accept(ctx context.Context, fee sdk.Coins, _ []sdk.Msg) (bool, error) {
	sdkCtx := sdk.UnwrapSDKContext(ctx)

x/authz

Like feegrant, authz Accept method takes only a sdk.Context:

cosmos-sdk/x/authz/keeper/keeper.go

Line 126 in 3e63309

resp, err := authorization.Accept(sdkCtx, msg)

x/gov legacy proposals

x/gov v1beta1 proposals handler takes only a sdk.Context:

cosmos-sdk/x/gov/types/v1beta1/content.go

Line 20 in 3e63309

type Handler func(ctx sdk.Context, content Content) error

Ante Handlers

Lastly, some ante handlers, such as TxFeeChecker, need access to services provided by the environment:

cosmos-sdk/x/auth/ante/fee.go

Lines 14 to 16 in 3e63309

	// TxFeeChecker check if the provided fee is enough and returns the effective fee and tx priority,
	// the effective fee should be deducted later, and the priority should be returned in abci response.
	type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error)

Solutions

A few solutions have been discussed, but consensus haven't been reached.

• One solution is to have those modules set their environment in context before passing it to those methods.
  We could make the caller aware that it is a module-specific context by having a module-scope context key: FeeGrantEnvContextKey, AuthzEnvConextKey, GovEnvContextKey.
  Drawback: The module developer needs then to be aware of which module calls the method and unwrap the right context.
  The module implementing those methods has no access to their environment.

• A second proposal is to store a subset of appmodule.Environment containing non module-scoped services
  Drawback: It adds a new notion differing from the environment, and may not be compatible with gov v1beta1 proposal handler usage (needs to be checked on sourcegraph.com).

• The third proposal is to add environment in the interfaces
  Drawback: It breaks everyone, and still has the same drawbacks as 1.

We are as well thinking of deprecating gov v1beta1 proposals totally or only for server/v2 if none of the solution above is a right fit.

[kocubinski]

I'm in favor of solution (1) here, I think it's an appropriate use of context.WithValue, the godoc reads

// Use context Values only for request-scoped data that transits processes and
// APIs, not for passing optional parameters to functions.

This is request (and module) scope data required for state machine execution that transits APIs. I prefer scoping to module since it will force us to be more careful with this pattern and it should be an exception to normal flow not the rule.

[julienrbrt]

We could make the caller aware that it is a module-specific context by having a module-scope context key: FeeGrantEnvContextKey, AuthzEnvConextKey, GovEnvContextKey.
Drawback: The module developer needs then to be aware of which module calls the method and unwrap the right context.
The module implementing those methods has no access to their environment.

Unfortunately, this wasn't possible, as we had to make the module defining an authz grant or a feegrant import x/authz or x/feegrant for the context key. As we want to avoid unnecessary deps, we have made a singel EnvironementContextKey available in core and precise in the documentation to not assume this key is present in all contexts.