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

R4R: Update gas costs to more reasonable numbers for GoS #3052

Merged
merged 3 commits into from
Dec 10, 2018

Conversation

jaekwon
Copy link
Contributor

@jaekwon jaekwon commented Dec 9, 2018

TODO: Create a new issue to unify gas constants in one place, and improve tests.

Recommended best-guess estimates for gas for GoS:

50,000 bytes per block.
1,500,000 gas per block.

50,000 bytes being written x 30 gas/byte-written ~= 1,500,000 gas
750 writes to store x 2000 gas/store-set ~= 1,500,000 gas
  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer

@ValarDragon
Copy link
Contributor

Would be cool to have 3 app-set constants, gas per nanosecond of computation, gas per byte read, and gas per p2p byte, and then derive gas numbers from there.

@jaekwon
Copy link
Contributor Author

jaekwon commented Dec 10, 2018

Sounds like it could be a good solution in the long run... the storage system should evolve to return some values upon operations that return the above kinds of values, and then compute the overall gas from those figures though, sounds useful. I'm not convinced that we need manage cpu costs, at least for the hub.

@jaekwon jaekwon changed the title WIP update gas figuresg R4R: Update gas costs to more reasonable numbers for GoS Dec 10, 2018
@jaekwon jaekwon merged commit 40a30b7 into develop Dec 10, 2018
@jaekwon
Copy link
Contributor Author

jaekwon commented Dec 10, 2018

Merging this in early for sake of release. Lets still discuss here though.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Code changes look fine to me. We might want to set a non-zero default minimum fee for GoS.


// how much gas = 1 atom
gasPerUnitCost = 1000
gasPerUnitCost = 10000
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only efficacious if ctx.MinimumFees() is nonzero, maybe we should set that for GoS

Copy link
Contributor

Choose a reason for hiding this comment

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

How do we set this if it's set per validator? Simply state a recommendation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just set a non-zero default. I think @zmanian will mention it in the docs, which works too

@cwgoes cwgoes deleted the jae/gasconstants branch December 10, 2018 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants