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

StoreAndInstantiate gov proposal #785

Closed
jhernandezb opened this issue Mar 14, 2022 · 15 comments · Fixed by #1074
Closed

StoreAndInstantiate gov proposal #785

jhernandezb opened this issue Mar 14, 2022 · 15 comments · Fixed by #1074
Assignees
Labels
M Estimated medium
Milestone

Comments

@jhernandezb
Copy link
Contributor

jhernandezb commented Mar 14, 2022

Would be nice to have a single proposal that does both, specially if the store code is uploaded with instantiate permissions Nobody having to wait an entire voting period for store + instantiate might be just too long depending on each chain.

@alpe
Copy link
Contributor

alpe commented Mar 14, 2022

👍 Good thinking.

@ethanfrey
Copy link
Member

I question this.

There is a desire to move away from the whole Store Proposal as it is too heavy to vote on. I would look for another way to do that before we consider adding more Proposals that include the entire wasm blob.

Also, I wonder which project will require votes on both store and instantiate?

This is related to #780 and I would consider a small re-architecture. If we do not allow StoreCode message to override the permissions, we could check a lot of boxes for permissioned chains. If we set the default InstanitatePermission to Nobody (which cannot be overridden):

  1. StoreCodeMsg, then vote to make one Instantitate.
  2. StoreCodeMsg, then vote to open up the permissions to instantiate (Freeze Code ID Proposal #780), then anyone can instantiate without a vote

We could also allow the StoreCodeProposal to set a non-Nobody instantiate permissions if people still want to do this vote, but I think the above two will make this whole flow much easier (and avoid gas intensive votes - see #761 )

@ethanfrey
Copy link
Member

If you agree on this, I would say #780 plus a new issue to prevent StoreCodeMsg from overriding the InstantiatePermissions (quite simple) would cover this, and #761 as well.

@jhernandezb
Copy link
Contributor Author

jhernandezb commented Mar 16, 2022

That kinda makes sense, I like that it will fix storing the blob inside the proposal, I'd be worried mostly about having a lot of contracts being uploaded and not used, but perhaps that could be solved by setting a fee that can be in a way a bit high (let chains to configure their own fee) for example creating a pool in osmosis costs 100 OSMO iirc, creating an nft collection in stargaze costs 1000 STARS.

@ethanfrey ethanfrey added this to the v0.25.0 milestone Mar 17, 2022
@jhernandezb
Copy link
Contributor Author

Let me know your thoughts about a configurable upload fee, I might be able get started on #780 which is needed for either option

@ethanfrey
Copy link
Member

Let me know your thoughts about a configurable upload fee, I might be able get started on #780 which is needed for either option

We have a high store code gas price. I think this is configurable in code when compiling the chain - if not please raise an issue. We don't support these all as params yet, there is an open issue to discuss that.

I think the fee should be passed on gas price. And chains should have a min-gas-price for that. Adding a separate fee handling mechanism for each module is a bit confusing and I would avoid this for now.

@jhernandezb
Copy link
Contributor Author

Well that doesn't quite work for chains like Osmosis and even with that its minimal but I understand your point.

@ethanfrey
Copy link
Member

Then this is a bigger problem for those chains.

You suggest a fee for anti-spam resistance.
The gas fee was introduced for anti-spam resistance.
Let's use it.

If people have such low gas fees it doesn't serve at all to stop spam, then why do they want to only stop this one spam possibility and not the others?

We charge plenty of gas to cover compilation and storage of the upload. You could run a few dozen ibc client updates to achieve a similar result.

@jhernandezb
Copy link
Contributor Author

jhernandezb commented Mar 22, 2022

My point wasn't about anti-spam but preventing contracts from being upload that would never be used because of the nature of a permissioned cosmwasm chain. For example random people uploading contracts but not submitting a gov proposal.

With the current flow if a proposal doesn't pass no contract is stored and it has a deposit fee. Moving away from this flow eliminates the fee (deposit) and the ability prevent uploads in the first place.

Having said that I agree having a fee on top of the gas is weird and also think it should not be added at least directly into this module.

@alpe
Copy link
Contributor

alpe commented Mar 25, 2022

Just adding some context on customizing wasmd gas values:
We have an extension point prepared that can be used to provide a custom Gas register to match your requirements.

Just from personal experience, not using some project "defaults" may cause communication and support overhead elsewhere ...

@ethanfrey ethanfrey added the optional Not absolutely required for the milestone label Apr 25, 2022
@ethanfrey ethanfrey modified the milestones: v0.27.0, v0.28.0 May 4, 2022
@ethanfrey ethanfrey removed the optional Not absolutely required for the milestone label May 4, 2022
@ethanfrey
Copy link
Member

I would start with #796 and #840 as a useable workflow, aiming for them both in 0.27. And then revisit if this is still necessary for the following v0.28 release

@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 stretch goal in wasmd backlog Sep 23, 2022
@alpe alpe moved this from 🏄 v0.30 stretch goal to 🔖 v0.30 in wasmd backlog Sep 23, 2022
@alpe alpe moved this from 🔖 v0.30 to 🏄 v0.30 stretch goal in wasmd backlog Sep 23, 2022
@pinosu pinosu self-assigned this Nov 3, 2022
@pinosu
Copy link
Contributor

pinosu commented Nov 3, 2022

Is this still relevant? Since #796 and #840 are now merged, what is the work left for this issue?

@alpe
Copy link
Contributor

alpe commented Nov 3, 2022

There were requests for a single proposal that does upload + instantiate for restricted chains where #840 is not setup.

@ethanfrey
Copy link
Member

Note that as of cosmos-sdk 0.47, proposals will be much cheaper to vote on... not sure if this impacts the desire for this feature. Maybe @jhernandezb can comment here?

Repository owner moved this from 🏄 v0.30 stretch goal to ✅ Done in wasmd backlog Nov 11, 2022
@jhernandezb
Copy link
Contributor Author

Note that as of cosmos-sdk 0.47, proposals will be much cheaper to vote on... not sure if this impacts the desire for this feature. Maybe @jhernandezb can comment here?

Somehow I missed this, seems like it's closed now thanks! I think is still relevant when you want to save some governance burden eg. waiting 2 complete cycles of governance upload + instantiate.

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

Successfully merging a pull request may close this issue.

4 participants