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

Authz module integration - more granularity for WasmExecuteMsg authorizations #803

Closed
JakeHartnell opened this issue Apr 12, 2022 · 10 comments
Labels
L Estimated large
Milestone

Comments

@JakeHartnell
Copy link

Currently, it's possible to grant permission for an address to perform the WasmExecuteMsg against any contract. This not a very useful granularity.

It would be nice to grant an account permission to execute only against a specific contract. Even better, if the message could be optionally specified.

This could be useful for DAOs, that want to grant a user permission to execute a contract on behalf of the DAO. For example, granting a user (or cron bot) permission to claim rewards on behalf of the DAO.

@ethanfrey
Copy link
Member

I have not looked into authz much. I assume we need a custom permission extension.

If you could add more context to the sdk modules and a design of what you need, that would help us evaluate and prioritise this

@ethanfrey ethanfrey added this to the v0.28.0 milestone Apr 25, 2022
@jackzampolin
Copy link
Contributor

Here is some stubbed out code to do this. I'll work on this more later this week but if we could specify exactly what we need here the code will fall right out.

#817

@alpe
Copy link
Contributor

alpe commented May 18, 2022

I had a chance to talk with @the-frey and @JakeHartnell about this issue at gateway Prague and explained the problem that the message content can be a slice or an object in json. This makes it complicated to build logic for message filtering.
Therefore the idea was to descope this part and do authZ for contract addresses first. The message filter can be a second PR but would require some discussion and refinement before to make it work for all CosmWasm chains.

Btw. It was nice to meet you!

@ethanfrey ethanfrey modified the milestones: v0.29.0, v0.30.0 Aug 15, 2022
@alpe alpe moved this to 🆕 New in wasmd backlog Sep 23, 2022
@alpe alpe moved this from 🆕 New to 🔖 v0.30 in wasmd backlog Sep 23, 2022
@alpe alpe mentioned this issue Sep 28, 2022
@alpe alpe removed the XL Estimated extreme large label Sep 28, 2022
@george-aj
Copy link

george-aj commented Nov 4, 2022

Hi,

This is looking like a great PR, I'm wondering if I could outline a use case for JunoSwap:

Claim RAW > Swap RAW into 2 Tokens > Add Liquidity to a pool > Bond said Liquidity

Claim RAW: rewards executes a claim msg in contract: juno1s43j4ct4tp3s5ywk0j4flr6e5ev74ce2lnuv8xwtzx2yfr6kmwaqmxfkdt

Swap: Rewards are paid out in RAW, so two swaps need to be made to be able to deposit the liquidity back into the pool. Which are increase_allowance and swap msgs in contract: juno15u3dt79t6sxxa3x3kpkhzsy56edaa5a66wvt3kxmukqjz2sx0hes5sn38g

Add liquidity: executes increase_allowance and add_liquidity msgs in contract juno168ctmpyppk90d34p3jjy658zf5a5l3w8wk35wht6ccqj4mr0yv8s4j5awr

Bond: the above liquidity executes a send msg in contract juno12sulrvp220gpsp8jsr7dpk9sdydhe8plasltftc6fnxl7yukh24qjvqcu9

What would be the design/structure required to be issued with the least amount of privilege's to be able to do the above? Thanks!

@alpe
Copy link
Contributor

alpe commented Nov 9, 2022

What would be the design/structure required to be issued with the least amount of privilege's to be able to do the above?

@george-aj thanks for sharing the advanced use case. I am assuming you want to setup a single sdk.Grant for the 4 contracts. The new wasm.ContractExecutionAuthorization can look like this:

// 1) Claim RAW: rewards executes a claim msg in contract:
// -> no tokens can be sent from the granter account
contract1 := sdk.MustAccAddressFromBech32("juno1s43j4ct4tp3s5ywk0j4flr6e5ev74ce2lnuv8xwtzx2yfr6kmwaqmxfkdt")
grant1 := types.MustNewContractGrant(contract1,
	types.NewMaxCallsLimit(math.MaxUint64), // what would be a good max value here?
	types.NewAcceptedMessageKeysFilter("claim"),
)
// 2) Swap: Rewards are paid out in RAW, so two swaps need to be made to be able to deposit the liquidity
//    back into the pool. Which are increase_allowance and swap msgs in contract:
// -> only RAW tokens can be sent from the granter account
contract2 := sdk.MustAccAddressFromBech32("juno15u3dt79t6sxxa3x3kpkhzsy56edaa5a66wvt3kxmukqjz2sx0hes5sn38g")
grant2 := types.MustNewContractGrant(contract2,
	types.NewMaxFundsLimit(sdk.NewCoin("RAW", sdk.NewInt(1_000_000_000_000))), // what would be a good max amount?
	types.NewAcceptedMessageKeysFilter("increase_allowance", "swap"),
)
// 3) Add liquidity: executes increase_allowance and add_liquidity msgs in contract
// -> only TokenA tokens can be sent from the granter account
contract3 := sdk.MustAccAddressFromBech32("juno168ctmpyppk90d34p3jjy658zf5a5l3w8wk35wht6ccqj4mr0yv8s4j5awr")
grant3 := types.MustNewContractGrant(contract3,
	types.NewMaxFundsLimit(sdk.NewCoin("TokenA", sdk.NewInt(1_000_000_000_000))),
	types.NewAcceptedMessageKeysFilter("increase_allowance", "add_liquidity"),
)
// 4) Bond: the above liquidity executes a send msg in contract
// -> only TokenB tokens can be sent from the granter account
contract4 := sdk.MustAccAddressFromBech32("juno12sulrvp220gpsp8jsr7dpk9sdydhe8plasltftc6fnxl7yukh24qjvqcu9")
grant4 := types.MustNewContractGrant(contract4,
	types.NewMaxFundsLimit(sdk.NewCoin("TokenB", sdk.NewInt(1_000_000_000_000))),
	types.NewAcceptedMessageKeysFilter("send"),
)
// build wasm authorization
authorization := types.NewContractExecutionAuthorization(grant1, grant2, grant3, grant4)
// build sdk authz grant
grantMsg, err := authz.NewMsgGrant(granterAddr, granteeAddr, authorization, time.Now().Add(time.Hour))
require.NoError(t, err)
// send authz message
_, err = chain.SendMsgs(grantMsg)

As I tried to outline in the code above, the problem is with the limits. Although all amounts depend on the first contract call, there is no way that the other grants can know about the real token values coming from the previous step. Even if the wasm execute messages are in the same TX, they are all handled separately (by the SDK).
A better solution could be built as a smart contract that does the orchestration via submessages. In this case a single authz grant to call the orchestration contract would be enough but then the complexity is on tracking and managing users funds within the orchestration contract. Not a simple problem...
I hope this helps. More feedback and use cases are very welcome!

If anybody has a good idea how the CLI could support such a multi grant use case, please share some ideas 🙏

@george-aj
Copy link

Hey @alpe,

This is awesome, thank you for taking the time. I had one question about 2), is there a way to also limit the tokens that can be swapped to? Say RAW can only be used to buy a list of tokens? Thanks!

@alpe
Copy link
Contributor

alpe commented Nov 11, 2022

@george-aj

is there a way to also limit the tokens that can be swapped to?

The current filters are not able to handle this kind of payload checks. Either you can grant authz for a concrete binary representation of the json message, all or a first level object key. Your use case would specify the object content with a mixture of fix and variable fields. The problem here is not the authz model but how to describe and ensure the content + CLI integration.
An idea would be a jq like description language to declare accept or reject rules. In tgrade system tests, I had good experience with gjson as library on client side for example. But it is stable and deterministic enough to run on a chain? Let's iterate on what we have and extend the limits and filters in coming versions.
I would be happy to have more use cases defined in an issue so that we can extend the filters and limits.

@alpe
Copy link
Contributor

alpe commented Nov 15, 2022

We have some basic CLI integration now merged in #1079

I will close this issue now and follow up on server side extension in #1090 and CLI in #1093. Please add your ideas and feedback to the new issues now.

Big thanks to everybody contribution to this feature. I think it will be very useful 👏

@alpe alpe closed this as completed Nov 15, 2022
Repository owner moved this from 🔖 v0.30 to ✅ Done in wasmd backlog Nov 15, 2022
@alpe alpe removed this from wasmd backlog Mar 3, 2023
@mikedotexe
Copy link

This is massive. Thanks, Jake, and Confio for pushing us forward a significant leap. I've been DM'ing with several people about this, who have found this issue, so for the sake of discoverability, want to link the amazing new protobuf.

https://github.com/CosmWasm/wasmd/blob/main/proto/cosmwasm/wasm/v1/authz.proto

Incredible potential. 🥇

@alpe
Copy link
Contributor

alpe commented Jun 8, 2023

@mikedotexe thank you for your warm words! 💐
I had created #1090 + #1093 to follow up and improve authz for wasm even more. If you have ideas or feature requests, please add them to the issues.

And please spread the word.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L Estimated large
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants