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

Remove gas charges related to source file and input JSONs #1036

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

vaivaswatha
Copy link
Contributor

@vaivaswatha vaivaswatha commented Jul 27, 2021

These charges, to make it uniform with compiled Scilla, will be charged at the blockchain layer, so we must remove it here from Scilla.

Zilliqa/Zilliqa#2678

The blockchain runs the checker prior to running the interpreter
during a deployment. Since this check must be done at the earliest,
move it to the checker.
@vaivaswatha vaivaswatha changed the title Move cost of parsing from deployment in interpreter to checker Remove gas charges related to source file and input JSONs Jul 28, 2021
Copy link
Contributor

@jjcnn jjcnn left a comment

Choose a reason for hiding this comment

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

Something doesn't look right.

I would expect all init output files to have a different gas_remaining with this PR. 31 files of the form init_*.json are changed in this PR, but we appear to have 87 of them in our testsuite:

cnn@tautology:~/Projects/scilla$ ls -la tests/runner/*/init_*.json | wc -l
87

tests/runner/Testcontracts.ml Show resolved Hide resolved
@vaivaswatha
Copy link
Contributor Author

Something doesn't look right.

I would expect all init output files to have a different gas_remaining with this PR. 31 files of the form init_*.json are changed in this PR, but we appear to have 87 of them in our testsuite:

cnn@tautology:~/Projects/scilla$ ls -la tests/runner/*/init_*.json | wc -l
87

You are looking for init_*.json, that would include init JSONs and init state JSONs too. Gas remaining is part of only the output JSONs.

@jjcnn
Copy link
Contributor

jjcnn commented Aug 3, 2021

Something doesn't look right.
I would expect all init output files to have a different gas_remaining with this PR. 31 files of the form init_*.json are changed in this PR, but we appear to have 87 of them in our testsuite:

cnn@tautology:~/Projects/scilla$ ls -la tests/runner/*/init_*.json | wc -l
87

You are looking for init_*.json, that would include init JSONs and init state JSONs too. Gas remaining is part of only the output JSONs.

I'm clearly not on top of my game. :-(

It looks fine, then.

@jjcnn
Copy link
Contributor

jjcnn commented Nov 1, 2021

@vaivaswatha : Status on this?

@vaivaswatha
Copy link
Contributor Author

@vaivaswatha : Status on this?

I can't merge this till Zilliqa/Zilliqa#2678 is done.

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.

2 participants