feat: ContractAuthorization (authz) #966

giansalex · 2022-08-29T03:27:17Z

closes #803

cli examples:

CONTRACT=wasm14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9s0phg4d
GRANTEE=wasm14vhcdsyf83ngsrrqc92kmw8q9xakqjm048cr0y

wasmd tx wasm grant $GRANTEE $CONTRACT --allow-msgs claim,bond --from granter
wasmd tx authz revoke $GRANTEE /cosmwasm.wasm.v1.MsgExecuteContract --from granter

query grants

grants:
- authorization:
    '@type': /cosmwasm.wasm.v1.ContractAuthorization
    contract: wasm14hj2tavq8fpesdwxxcu44rty3hh90vhujrvcmstl4zr3txmfvw9s0phg4d
    messages:
    - claim
    - bond 
    once: false
  expiration: "2024-08-28T02:03:09Z"
  grantee: wasm14vhcdsyf83ngsrrqc92kmw8q9xakqjm048cr0y
  granter: wasm1tr944mwl56j7yhqkgq5xmah8w7udjwpmj33a4y
pagination:
  next_key: null
  total: "0"

codecov · 2022-08-29T03:43:40Z

Codecov Report

Merging #966 (b54ad2d) into main (1fdd378) will decrease coverage by 0.22%.
The diff coverage is 41.17%.

@@            Coverage Diff             @@
##             main     #966      +/-   ##
==========================================
- Coverage   59.36%   59.13%   -0.23%     
==========================================
  Files          51       52       +1     
  Lines        6231     6299      +68     
==========================================
+ Hits         3699     3725      +26     
- Misses       2266     2306      +40     
- Partials      266      268       +2

Impacted Files	Coverage Δ
x/wasm/client/cli/tx.go	`20.48% <0.00%> (-4.52%)`	⬇️
x/wasm/types/authz.go	`88.46% <88.46%> (ø)`
x/wasm/types/codec.go	`100.00% <100.00%> (ø)`
x/wasm/keeper/keeper.go	`88.24% <0.00%> (-0.34%)`	⬇️

alpe

Thanks a lot for the PR and driving this forward. Very nice! I had a quick look and added some comments.

I assume the authz module ensures that the sender has permission to set the ContractAutorization. But how does it work when not called from the contract? The CLI does not have keys for the contract address. I need to 👁️ at the module but if you can point me to the code line , that would help

alpe · 2022-08-29T15:27:11Z

proto/cosmwasm/wasm/v1/authz.proto

+  repeated string messages = 2;
+
+  // Once specifies if the contract can only be called once
+  bool once = 3; 


Would it make sense to have a counter to decrement instead?

yes, can be executions ?

alpe · 2022-08-29T15:29:49Z

x/wasm/types/authz.go

+
+// ValidateBasic implements Authorization.ValidateBasic.
+func (a ContractAuthorization) ValidateBasic() error {
+	if len(a.Contract) == 0 {


It does not hurt but the empty string is covered by AccAddressFromBech32() also

alpe · 2022-08-29T15:30:53Z

x/wasm/types/authz.go

+	if len(a.Messages) == 0 {
+		return sdkerrors.ErrInvalidRequest.Wrap("empty msg list")
+	}
+	return nil


Are there any constraints on the message content? Like no empty strings, no whitespaces start/end, no duplicates?

alpe · 2022-08-29T15:32:41Z

x/wasm/types/authz.go

+		return authztypes.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf("cannot run %s contract", exec.Contract)
+	}
+
+	gasForDeserialization := gasDeserializationCostPerByte * uint64(len(exec.Msg))


👍 very nice that you have gas costs on your list. All the prices have been encapsulated into the GasRegister to make it customizable for chains via Options.

alpe · 2022-08-29T15:50:28Z

x/wasm/types/authz.go

+
+	gasForDeserialization := gasDeserializationCostPerByte * uint64(len(exec.Msg))
+	ctx.GasMeter().ConsumeGas(gasForDeserialization, "contract authorization")
+	if err := IsJSONObjectWithTopLevelKey(exec.Msg, a.Messages); err != nil {


The design decision was to not make assumptions about the message data other than it is valid json. I can understand that you want more fine grained control in the authorization but [{"foo":"bar"}]is valid json as well.
Any thoughts to cover or document this?

There is also no way to grant access to all contract messages. Would it make sense to support this?

I think IsJSONObjectWithTopLevelKey verifies the requirements.

alpe · 2022-08-29T15:55:42Z

proto/cosmwasm/wasm/v1/authz.proto

+  string contract = 1;
+
+  // Messages is the list of messages that can be executed
+  repeated string messages = 2;


messages is a bit ambiguous. Technically they are just json object keys. Now about allowed_json_keys ?

I saw a similarity to GenericAuthorization, it says msg although it is only the type URL

Are the messages the same as "execute methods" here? Amazing work, just catching up now

alpe · 2022-08-29T15:57:01Z

proto/cosmwasm/wasm/v1/authz.proto

+option go_package = "github.com/CosmWasm/wasmd/x/wasm/types";
+
+// ContractAuthorization defines authorization for wasm execute.
+message ContractAuthorization {


How about ContractExecuteAuthorization? There may be other contract authz like set permission, set admin,... in the future

alpe · 2022-08-29T15:59:32Z

x/wasm/types/authz.go

+
+// Accept implements Authorization.Accept.
+func (a ContractAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authztypes.AcceptResponse, error) {
+	exec, ok := msg.(*MsgExecuteContract)


Good to check!

giansalex · 2022-08-29T19:42:38Z

Any user can grant ContractAutorization, a contract can give an authorization using CosmosMsg::Stargate

Full example:

User grants the "claim" msg

wasmd tx wasm grant $GRANTEE_ADDR $STAKE_CONTRACT --allow-msgs claim --from granter

Grantee executes the contract

wasmd tx wasm execute $STAKE_CONTRACT '{"claim":{}}' --from $GRANTER_ADDR --generate-only > claim.json

wasmd tx authz exec claim.json --from grantee

giansalex · 2022-08-29T21:47:29Z

I need to add a list in ContractAuthorization, because with the current implementation I can only give permission to a single contract per grantee.

eg. I could not give permissions ContractX:claim and ContractY:swap to the same grantee.

alpe · 2022-08-30T08:42:31Z

Thanks for the examples! This helped me to get a better picture!

I need to add a list in ContractAuthorization, because with the current implementation I can only give permission to a single contract per grantee.

Good finding. The store key is built from grantee, granter, msgType.
Adding a list of contract addresses would be a simple solution but this enables ContractX:claim,swap and ContractY:claim,swap while you probably want ContractX:claim and ContractY:swap.
With a wildcard things get even more in the wrong direction. I think it makes sense to extract the contract, keys tuple into a new proto type and iterate over the list to find the matching contract authorization. WDYT?

alpe · 2022-09-02T13:20:44Z

@giansalex I have started a spike based on your work to model this. I may be able to push this later today or on Monday. The proto would looks like below. WDYT?

// ContractExecutionAuthorization defines authorization for wasm execute.
message ContractExecutionAuthorization {
  option (cosmos_proto.implements_interface) = "Authorization";

  repeated ContractExecutionGrant grants = 1 [ (gogoproto.nullable) = false ];
}

// ContractExecutionGrant a granted execute permission for a single contract
message ContractExecutionGrant {
  // Contract is the address of the smart contract
  string contract = 1;

  // Filter rules to apply
  oneof filter {
    AcceptedMessageKeysFilter accepted_message_keys = 2;
    AllowAllWildcard allow_all_wildcard = 3;
  }

  // RemainingCalls specifies the number of authorized executions remaining
  uint64 remaining_calls = 4;
}

// AllowAllWildcard is a wildcard to allow any type of contract execution
// message
message AllowAllWildcard {}

// AcceptedMessageKeysFilter accept specific contract message keys in the json
// object that can be executed
message AcceptedMessageKeysFilter {
  // Messages is the list of unique keys
  repeated string messages = 1;
}

giansalex · 2022-09-02T17:50:33Z

I think it makes sense to extract the contract, keys tuple into a new proto type and iterate over the list to find the matching contract authorization.

Yes, I was also thinking about a list.

I have started a spike based on your work to model this. I may be able to push this later today or on Monday. The proto would looks like below. WDYT?

About remaining_calls, what happens when it becomes unlimited, will 0 be used?

Staking authz uses nil for unlimited max-tokens https://github.com/cosmos/cosmos-sdk/blob/v0.45.8/x/staking/types/authz.go#L101

alpe · 2022-09-05T16:07:55Z

About remaining_calls, what happens when it becomes unlimited, will 0 be used?

good point! IMHO it makes sense to make it explicit or do not support unlimited. I have pushed an example to:
https://github.com/CosmWasm/wasmd/tree/966_grants_spike

giansalex · 2022-09-06T05:49:40Z

I am not against unlimited because authz MsgGrant has an expiration field, although with a high value in remaining_calls may be enough.

I did not see any new changes in 966_grants_spike branch

ethanfrey · 2022-09-06T12:10:24Z

These features will be in the next v0.30 release, maybe the version in #978 based on it.

Let's work together to get some clear design we all agree on.

giansalex requested a review from alpe as a code owner August 29, 2022 03:27

alpe reviewed Aug 29, 2022

View reviewed changes

alpe mentioned this pull request Sep 5, 2022

966 grants spike #978

Closed

giansalex added 7 commits September 6, 2022 00:30

Add contract authz proto

9b9e54a

Implement contract autorization

2de19ea

Register contract authz

4d6914c

Add contract-authz tests

9cd92d7

Consume gas for contract authz

ecd4d35

Add contract authz cli

4784ba4

Update cli usage

b54ad2d

giansalex force-pushed the authz branch from 4c485d8 to b54ad2d Compare September 6, 2022 05:33

ethanfrey added this to the v0.30.0 milestone Sep 6, 2022

ethanfrey mentioned this pull request Sep 6, 2022

More Granular Authz perms for cosmwasm contracts #817

Closed

giansalex closed this Nov 2, 2022

alpe mentioned this pull request Nov 4, 2022

Contract authz - redesign #1077

Merged

giansalex deleted the authz branch May 29, 2023 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ContractAuthorization (authz) #966

feat: ContractAuthorization (authz) #966

giansalex commented Aug 29, 2022

codecov bot commented Aug 29, 2022 •

edited

Loading

alpe left a comment

alpe Aug 29, 2022

giansalex Aug 29, 2022 •

edited

Loading

alpe Aug 29, 2022

alpe Aug 29, 2022 •

edited

Loading

alpe Aug 29, 2022

alpe Aug 29, 2022

giansalex Aug 29, 2022 •

edited

Loading

alpe Aug 29, 2022

giansalex Aug 29, 2022

mikedotexe May 29, 2023

alpe Aug 29, 2022

alpe Aug 29, 2022

giansalex commented Aug 29, 2022 •

edited

Loading

giansalex commented Aug 29, 2022

alpe commented Aug 30, 2022

alpe commented Sep 2, 2022

giansalex commented Sep 2, 2022 •

edited

Loading

alpe commented Sep 5, 2022

giansalex commented Sep 6, 2022 •

edited

Loading

ethanfrey commented Sep 6, 2022

feat: ContractAuthorization (authz) #966

feat: ContractAuthorization (authz) #966

Conversation

giansalex commented Aug 29, 2022

codecov bot commented Aug 29, 2022 • edited Loading

Codecov Report

alpe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giansalex Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alpe Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giansalex Aug 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giansalex commented Aug 29, 2022 • edited Loading

giansalex commented Aug 29, 2022

alpe commented Aug 30, 2022

alpe commented Sep 2, 2022

giansalex commented Sep 2, 2022 • edited Loading

alpe commented Sep 5, 2022

giansalex commented Sep 6, 2022 • edited Loading

ethanfrey commented Sep 6, 2022

codecov bot commented Aug 29, 2022 •

edited

Loading

giansalex Aug 29, 2022 •

edited

Loading

alpe Aug 29, 2022 •

edited

Loading

giansalex Aug 29, 2022 •

edited

Loading

giansalex commented Aug 29, 2022 •

edited

Loading

giansalex commented Sep 2, 2022 •

edited

Loading

giansalex commented Sep 6, 2022 •

edited

Loading