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

Set upload fees for contracts higher #85

Closed
JakeHartnell opened this issue Nov 13, 2021 · 17 comments
Closed

Set upload fees for contracts higher #85

JakeHartnell opened this issue Nov 13, 2021 · 17 comments
Assignees
Labels
Priority This needs to be done ASAP

Comments

@JakeHartnell
Copy link
Contributor

It should cost more to upload a contract. I believe there is already a parameter for this? Let's update it on the Uni testnet.

If there is not a parameter, we should add this feature. Transactions should be cheap, but uploading a contract should be expensive.

@JakeHartnell JakeHartnell added the Priority This needs to be done ASAP label Nov 13, 2021
@blockcreators
Copy link
Contributor

Governance can change it later but what do you think the initial cost per byte should be?
And how much would the average contract be? (byte cost * average contract bytes)

To make a simple token via junomint, how much do we expect it to cost?

@the-frey
Copy link
Collaborator

Will have a look at this later today to work out what the param change would be. I think we also are going to need to document the param changes that this and #82 mean we will need to do on mainnet before we can upgrade. Moneta is gradually becoming quite hairy and we should have a pretty foolproof checklist for the upgrade to run imo with backout points if possible

@the-frey
Copy link
Collaborator

the-frey commented Nov 16, 2021

...which I guess in practical terms means I need to write the checklist lol

@JakeHartnell
Copy link
Contributor Author

JakeHartnell commented Nov 16, 2021

...which I guess in practical terms means I need to write the checklist lol

Appreciate all your help on this. 🙏

@JakeHartnell JakeHartnell added this to the v2.0.0 moneta: juno<->cw milestone Nov 16, 2021
@JakeHartnell
Copy link
Contributor Author

...which I guess in practical terms means I need to write the checklist lol

Let's try to keep the list here in GitHub issues. Let's try to use this Milestone: https://github.com/CosmosContracts/juno/milestone/1

@the-frey
Copy link
Collaborator

okay so there's also the upload size to consider too.

In wasm;

// DefaultMaxWasmCodeSize limit max bytes read to prevent gzip bombs
DefaultMaxWasmCodeSize = 600 * 1024 * 2

and

junod query params subspace wasm maxWasmCodeSize --chain-id uni

confirms we are using default

key: maxWasmCodeSize
subspace: wasm
value: '"1228800"'

@the-frey
Copy link
Collaborator

the-frey commented Nov 16, 2021

@JakeHartnell
Copy link
Contributor Author

JakeHartnell commented Nov 16, 2021

While the various costs are specified https://github.com/CosmWasm/wasmd/blob/57517b0c333e23aedd2bbccf80579cf75081241b/x/wasm/keeper/gas_register.go#L13

With reasoning https://github.com/CosmWasm/cosmwasm/blob/v1.0.0-beta/docs/GAS.md

Good find! Looks like this is maybe what we want:

// DefaultCompileCost is how much SDK gas is charged *per byte* for compiling WASM code.
// Benchmarks and numbers were discussed in: https://github.com/CosmWasm/wasmd/pull/634#issuecomment-938056803
DefaultCompileCost uint64 = 3

@the-frey
Copy link
Collaborator

Yup, although it's hard coded AFAICT which would mean forking. AFAIUI changes there are consensus-breaking, hence why they aren't params. That said, it's lateish in the day and my caffiene level is drooping so may have that wrong.

@faddat
Copy link
Contributor

faddat commented Nov 19, 2021

hate that caffeine drooping feeling

@the-frey
Copy link
Collaborator

I have a path to doing this, but given that Confio (as per the reasoning & discussion links above) have thought about this a fair bit, I am... still not sold on overriding these values.

Moreover, if we were to override these values we'd have to do a few more rounds of testing. Which isn't necessarily a bad thing, but November is flying by...

@JakeHartnell
Copy link
Contributor Author

JakeHartnell commented Nov 19, 2021

I have a path to doing this, but given that Confio (as per the reasoning & discussion links above) have thought about this a fair bit, I am... still not sold on overriding these values.

Moreover, if we were to override these values we'd have to do a few more rounds of testing. Which isn't necessarily a bad thing, but November is flying by...

Confio thought about these values in the context on Confio and tgrade, we need to think about them in the context of Juno.

Yup, although it's hard coded AFAICT which would mean forking. AFAIUI changes there are consensus-breaking, hence why they aren't params.

Well, we already have a fork. Would be great to make some of these params and contribute that upstream. Having DefaultCompileCost as a param would be the best option for us going forward.

@JakeHartnell

This comment has been minimized.

@the-frey

This comment has been minimized.

@JakeHartnell

This comment has been minimized.

@the-frey
Copy link
Collaborator

FWIW I'm not against changing this and then testing, it's a bunch more stuff to test. It's potentially doable in base Juno, no fork required - but I am against making them params. At the minute I'm dubious about exposing something that's an obvious attack vector to governance. Could be convinced in future however.

@the-frey the-frey self-assigned this Nov 19, 2021
@the-frey
Copy link
Collaborator

Okay have spiked something on https://github.com/CosmosContracts/juno/tree/override_cw_default_compile_costs - don't think it's quite there yet. Need to test a couple of things before PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority This needs to be done ASAP
Projects
None yet
Development

No branches or pull requests

4 participants