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.

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

feat(wasm): add contract access control #151

Merged
merged 12 commits into from
May 3, 2021
1 change: 1 addition & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Write the changes made after branching from cosmos-sdk v0.42.1.
* (global) [\#97](https://github.com/line/lbm-sdk/pull/97) Add codespace to query error
* (config) [\#114](https://github.com/line/lbm-sdk/pull/114) Add idle-timeout to rest server and rpc server config
* (x/wasm) [\#127](https://github.com/line/lbm-sdk/pull/127) Add wasm with Staragate migration completed.
* (x/wasm) [\#151](https://github.com/line/lbm-sdk/pull/151) Add contract access control.

### IMPROVEMENTS

Expand Down
32 changes: 32 additions & 0 deletions x/wasm/client/cli/new_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,35 @@ func ClearContractAdminCmd() *cobra.Command {
flags.AddTxFlagsToCmd(cmd)
return cmd
}

// UpdateContractStatusCmd clears an admin for a contract
func UpdateContractStatusCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "update-contract-status [contract_addr_bech32] [status(Active|Inactive)]",
Short: "Clears admin for a contract to prevent further migrations",
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientTxContext(cmd)
if err != nil {
return err
}

status := types.ContractStatusUnspecified
if err := (&status).UnmarshalText([]byte(args[1])); err != nil {
return err
}

msg := types.MsgUpdateContractStatus{
Sender: clientCtx.GetFromAddress().String(),
Contract: args[0],
Status: status,
}
if err := msg.ValidateBasic(); err != nil {
return err
}
return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), &msg)
},
}
flags.AddTxFlagsToCmd(cmd)
return cmd
}
1 change: 1 addition & 0 deletions x/wasm/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func GetTxCmd() *cobra.Command {
MigrateContractCmd(),
UpdateContractAdminCmd(),
ClearContractAdminCmd(),
UpdateContractStatusCmd(),
)
return txCmd
}
Expand Down
2 changes: 2 additions & 0 deletions x/wasm/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ func NewHandler(k *Keeper) sdk.Handler {
res, err = msgServer.UpdateAdmin(sdk.WrapSDKContext(ctx), msg)
case *MsgClearAdmin:
res, err = msgServer.ClearAdmin(sdk.WrapSDKContext(ctx), msg)
case *types.MsgUpdateContractStatus:
res, err = msgServer.UpdateContractStatus(sdk.WrapSDKContext(ctx), msg)
default:
errMsg := fmt.Sprintf("unrecognized wasm message type: %T", msg)
return nil, sdkerrors.Wrap(sdkerrors.ErrUnknownRequest, errMsg)
Expand Down
9 changes: 9 additions & 0 deletions x/wasm/internal/keeper/authz_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type AuthorizationPolicy interface {
CanCreateCode(c types.AccessConfig, creator sdk.AccAddress) bool
CanInstantiateContract(c types.AccessConfig, actor sdk.AccAddress) bool
CanModifyContract(admin, actor sdk.AccAddress) bool
CanUpdateContractStatus(c types.AccessConfig, actor sdk.AccAddress) bool
}

type DefaultAuthorizationPolicy struct {
Expand All @@ -26,6 +27,10 @@ func (p DefaultAuthorizationPolicy) CanModifyContract(admin, actor sdk.AccAddres
return admin != nil && admin.Equals(actor)
}

func (p DefaultAuthorizationPolicy) CanUpdateContractStatus(config types.AccessConfig, actor sdk.AccAddress) bool {
return config.Allowed(actor)
}

type GovAuthorizationPolicy struct {
}

Expand All @@ -40,3 +45,7 @@ func (p GovAuthorizationPolicy) CanInstantiateContract(types.AccessConfig, sdk.A
func (p GovAuthorizationPolicy) CanModifyContract(sdk.AccAddress, sdk.AccAddress) bool {
return true
}

func (p GovAuthorizationPolicy) CanUpdateContractStatus(types.AccessConfig, sdk.AccAddress) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

When is this (and above 3 functions) called and what is this for? Want comment doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the comment to the code.

return true
}
9 changes: 7 additions & 2 deletions x/wasm/internal/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,10 @@ func TestImportContractWithCodeHistoryReset(t *testing.T) {
"permission": "Everybody"
},
"instantiate_default_permission": "Everybody",
"max_wasm_code_size": 500000
"max_wasm_code_size": 500000,
"contract_status_access": {
"permission": "Nobody"
}
},
"codes": [
{
Expand All @@ -469,7 +472,8 @@ func TestImportContractWithCodeHistoryReset(t *testing.T) {
"code_id": "1",
"creator": "link1p0yx9c9q4xsnedlcn24gqfry5dcu6e9xkhv9aj",
"admin": "link1qyqszqgpqyqszqgpqyqszqgpqyqszqgp8apuk5",
"label": "ȀĴnZV芢毤"
"label": "ȀĴnZV芢毤",
"status": "Active"
Copy link
Contributor

@loloicci loloicci Apr 28, 2021

Choose a reason for hiding this comment

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

Mixed Tab vs Space in these templates. Want to unify.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have fixed it to use spaces.

}
}
],
Expand Down Expand Up @@ -534,6 +538,7 @@ func TestImportContractWithCodeHistoryReset(t *testing.T) {
Admin: adminAddr,
Label: "ȀĴnZV芢毤",
Created: &types.AbsoluteTxPosition{BlockHeight: 0, TxIndex: 0},
Status: wasmTypes.ContractStatusActive,
}
assert.Equal(t, expContractInfo, *gotContractInfo)

Expand Down
34 changes: 33 additions & 1 deletion x/wasm/internal/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,12 @@ func (k Keeper) getInstantiateAccessConfig(ctx sdk.Context) types.AccessType {
return a
}

func (k Keeper) getContractStatusAccessConfig(ctx sdk.Context) types.AccessConfig {
var a types.AccessConfig
k.paramSpace.Get(ctx, types.ParamStoreKeyContractStatusAccess, &a)
return a
}

func (k Keeper) GetMaxWasmCodeSize(ctx sdk.Context) uint64 {
var a uint64
k.paramSpace.Get(ctx, types.ParamStoreKeyMaxWasmCodeSize, &a)
Expand Down Expand Up @@ -286,7 +292,7 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A

// persist instance first
createdAt := types.NewAbsoluteTxPosition(ctx)
contractInfo := types.NewContractInfo(codeID, creator, admin, label, createdAt)
contractInfo := types.NewContractInfo(codeID, creator, admin, label, createdAt, types.ContractStatusActive)

// check for IBC flag
report, err := k.wasmer.AnalyzeCode(codeInfo.CodeHash)
Expand Down Expand Up @@ -321,6 +327,9 @@ func (k Keeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller
if err != nil {
return nil, err
}
if contractInfo.Status != types.ContractStatusActive {
return nil, sdkerrors.Wrap(types.ErrInvalid, "inactive contract")
}

if !k.IsPinnedCode(ctx, contractInfo.CodeID) {
ctx.GasMeter().ConsumeGas(InstanceCost, "Loading CosmWasm module: execute")
Expand Down Expand Up @@ -374,6 +383,9 @@ func (k Keeper) migrate(ctx sdk.Context, contractAddress sdk.AccAddress, caller
if contractInfo == nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "unknown contract")
}
if contractInfo.Status != types.ContractStatusActive {
return nil, sdkerrors.Wrap(types.ErrInvalid, "inactive contract")
}
if !authZ.CanModifyContract(contractInfo.AdminAddr(), caller) {
return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not migrate")
}
Expand Down Expand Up @@ -530,11 +542,31 @@ func (k Keeper) ClearContractAdmin(ctx sdk.Context, contractAddress sdk.AccAddre
return k.setContractAdmin(ctx, contractAddress, caller, nil, k.authZPolicy)
}

// UpdateContractStatus sets a new status of the contract on the ContractInfo.
func (k Keeper) UpdateContractStatus(ctx sdk.Context, contractAddress sdk.AccAddress, caller sdk.AccAddress, status types.ContractStatus) error {
if !k.authZPolicy.CanUpdateContractStatus(k.getContractStatusAccessConfig(ctx), caller) {
return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not update contract status")
}

contractInfo := k.GetContractInfo(ctx, contractAddress)
if contractInfo == nil {
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "unknown contract")
}
if contractInfo.Status != status {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can status be nil?

Copy link
Contributor Author

@whylee259 whylee259 Apr 30, 2021

Choose a reason for hiding this comment

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

No, it is a value type.
It can be ContractStatusUnspecified but it's not a problem.

contractInfo.Status = status
k.storeContractInfo(ctx, contractAddress, contractInfo)
}
return nil
}

func (k Keeper) setContractAdmin(ctx sdk.Context, contractAddress, caller, newAdmin sdk.AccAddress, authZ AuthorizationPolicy) error {
contractInfo := k.GetContractInfo(ctx, contractAddress)
if contractInfo == nil {
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "unknown contract")
}
if contractInfo.Status != types.ContractStatusActive {
return sdkerrors.Wrap(types.ErrInvalid, "inactive contract")
}
if !authZ.CanModifyContract(contractInfo.AdminAddr(), caller) {
return sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "can not modify contract")
}
Expand Down
Loading