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.

Sign up for GitHub

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

Jump to bottom

Add Gas Metering #1

Closed
johannbarbie opened this issue Jul 13, 2018 · 36 comments
Closed

Add Gas Metering #1

johannbarbie opened this issue Jul 13, 2018 · 36 comments
Labels
bounty enhancement New feature or request

Comments

@johannbarbie
Copy link
Member

Add a gasLimit parameter when a program is executed.

  • The EVM should return an outOfGas response if the program consumed to much gas.
  • On each step the remaining gas should be decreased by the cost of the specific OP-code.

Check https://github.com/ethereum/py-evm for reference.
Check the Architecture docs for more details.
@johannbarbie johannbarbie added the enhancement New feature or request label Jul 13, 2018
@johannbarbie johannbarbie changed the title Gas Metering Add Gas Metering Jul 13, 2018
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 3.662 ETH (1601.13 USD @ $437.23/ETH) attached to it.

@johannbarbie
Copy link
Member Author

@davidbanu thanks for applying! want to jump on a quick call and discuss architecture?

@gitcoinbot
Copy link

@davidbanu Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@davidbanu
Copy link

davidbanu commented Jul 18, 2018 •
edited

@johannbarbie, sorry for the silence...Had a hard weekend. I'm reading throughout the repository trying to come up with the best plan...

@vs77bb
Copy link

vs77bb commented Jul 20, 2018

Thanks for the update @davidbanu, please let us know if you have any questions 👍

@davidbanu
Copy link

I will submit a WIP PR soon for full transparency of the work I did until now. Any comments regarding my approach will be welcomed.

@johannbarbie
Copy link
Member Author

hi @davidbanu , thanks. excited to look at that.
do you also want to participate in our status call on Monday and say "hi"?

here are the details: https://hackmd.io/Kn0hwBA7Tvm6mfacCH1rIw

@davidbanu
Copy link

@johannbarbie, wow, sure! It will be my pleasure...

@davidbanu
Copy link

davidbanu commented Jul 21, 2018 •
edited

@johannbarbie, so I've added myself into the Topics of Interest list. Can I talk a little bit about the gas metering job and my progress on Monday? Or this is just an internal kind of a list?

@johannbarbie
Copy link
Member Author

yes yes, that would be great 😃 you can also join the discord link mentioned in the doc, and ask questions any time.

@davidbanu
Copy link

@johannbarbie, cool. Thanks a lot!

@davidbanu
Copy link

@johannbarbie, I need to solve an urgent matter and I will not be able to attend today discussion. I am deeply sorry. I will make the WIP PR, once I arrive at the office. Sorry for any inconvenience I've produced.

@johannbarbie
Copy link
Member Author

@davidbanu any updates on this? :)

@troggy troggy added the bounty label Jul 26, 2018
@davidbanu
Copy link

@johannbarbie sorry for not saying anything. I've broke my leg and this changed my schedule by a lot. But I'm on it right now. A little bit concerned about the codesize of this thing once the gas metering is fully implemented. I want to make that WIP PR soon to exemplify what I'm referring to. But until then, I want to try just one more idea that I had last night. It might work better.

@gitcoinbot
Copy link

@davidbanu Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

gitcoinbot commented Aug 1, 2018 •
edited

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 7 months, 2 weeks ago.
Please review their action plans below:

1) pinkiebell has been approved to start work.

Happy to work on that.
I examine the codebase first to get more familiar with the project.
So far, I am optimistic for turnaround this week.

Learn more on the Gitcoin Issue Details page.

@pinkiebell pinkiebell mentioned this issue Aug 1, 2018
Merged
@pinkiebell
Copy link
Contributor

@johannbarbie
Please review #10 and let me know if I am on the right track ;).

@johannbarbie
Copy link
Member Author

johannbarbie commented Aug 3, 2018 •
edited

@pinkiebell thx for picking this up 🎉
you are exactly on the right track!

@gitcoinbot
Copy link

@pinkiebell Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@pinkiebell
Copy link
Contributor

@johannbarbie
Any requests for changes? 😃

@johannbarbie
Copy link
Member Author

@pinkiebell great work. formally fulfilled. i'm just worried about covering more op-codes in unit tests. currently only one example.
@troggy is preparing some tests that he would share until tonight.
would be great if you could include them for gas-metering as well.

@troggy
Copy link
Member

troggy commented Aug 10, 2018

sorry, it takes longer than I expected. @pinkiebell I've started wrting some tests here: #5. I guess the idea is that we can use the same fixtures and same test to check for gas usage for each opcode as well.

I hope to finish the whole test set by Tuesday.

@pinkiebell
Copy link
Contributor

@troggy Looks good 👍 .

@pinkiebell
Copy link
Contributor

@troggy
What's your status? 😃

@troggy
Copy link
Member

troggy commented Aug 21, 2018

stuck with storage state import/export for a some time already :/

I noticed you merged some of my changes 👍Will check it out this week. Do you plan to add anything else? I noticed todos in CALL, DELEGATECALL, STATICCALL handlers. Is there any way we can create a test for these?

@pinkiebell
Copy link
Contributor

These TODO's can go and the check for enough gas should be moved after the stack operations and return early if we don't have enough gas. I will fix this later.

We can test these three handlers by calling them with- and without enough gas remaining.

@pinkiebell
Copy link
Contributor

@troggy
I resolved my TODOs and added a basic tests for CALL, DELEGATECALL and STATICCALL 👍 .
Can you give me some more input on the storage-state import/export?
Happy to help you with that, if I can 😄

@troggy
Copy link
Member

troggy commented Aug 23, 2018

@pinkiebell PR looks solid 👍I think you can submit work as done on Gitcoin

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 3.662 ETH (984.11 USD @ $268.74/ETH) has been submitted by:

  1. @pinkiebell

@johannbarbie please take a look at the submitted work:

@troggy
Copy link
Member

troggy commented Aug 24, 2018

@pinkiebell I have a message from @johannbarbie. He is on vacation for one more week and unfortunately forgot the private key at home. I guess he will be able to release the payment once he got back on Sept 1st.

@pinkiebell
Copy link
Contributor

No problem 👍

@gitcoinbot
Copy link

⚡️ A tip worth 0.18310 ETH (53.51 USD @ $292.25/ETH) has been granted to @pinkiebell for this issue from @johannbarbie. ⚡️

Nice work @pinkiebell! Your tip has automatically been deposited in the ETH address we have on file.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 3.662 ETH (1070.21 USD @ $292.25/ETH) attached to this issue has been approved & issued to @pinkiebell.

@troggy
Copy link
Member

troggy commented Aug 29, 2018

@pinkiebell congratulations on completing the bounty! Well done!

@jdkanani
Copy link

@pinkiebell Great work!

@pinkiebell
Copy link
Contributor

@johannbarbie @troggy @jdkanani
Thanks! Hope to see this going forward.
There is much potential for contract profiling & analysis here 👍

troggy pushed a commit that referenced this issue Sep 1, 2018
@pinkiebell @troggy
Initial gas metering (#10)
6635ce8 
* Initial gas metering

Fixes #1 Depends on #5

* gas metering: check gas earlier in the loop & fix/add additional tests
@troggy troggy closed this as completed Sep 14, 2018
troggy added a commit that referenced this issue Sep 17, 2018
@troggy
Off-Chain Executor (#5)
1e5cc6c 
* add naive executeAndStop method

* broken code, wip

* broken code fixed

* improve naming: EVMOverride → EVMCallContext

* fix 'out of gas' for EthereumRuntime.new()

* fix linting after rebase

* add naive initAndExecute.

Only stack and memory are initialized so far.

* restore executeFlat method

* add tests for stack-only opcodes

* tightly packing bytes in flatten Result to spare some stack for local vars

* extend initAndExecute to initialize accounts

* allow to init EVM call with gasLimit

* add test cases for context+stack opcodes

* add gasLimit tests

* export program counter as part of the state

* add test cases for code+stack opcodes

* test cases for data opcodes

* fix: update program counter in EVM state to the last executed opcode position

* add memory + stack test cases

* fix GAS and CODESIZE tests

we are testing just one opcode execution, so gasUsed in these test will always show only gas price for a single opcode disregarding the code size

* add CALLDATACOPY and CODECOPY tests

* Initial gas metering (#10)

* Initial gas metering

Fixes #1 Depends on #5

* gas metering: check gas earlier in the loop & fix/add additional tests

* new ABI encoder is merged in web3 1.0.0-beta.36 (we don't use it though yet)

* allow to init storage, add tests for SSTORE and SLOAD

* init log data, tests for LOG0-4, fighting out of gas

* add missing file

* remove .vscode settings from git

* Reduce gas consumption in EthereumRuntime (#11)

Example gas consumption before = 6582400 and after = 6485785.
@johannbarbie johannbarbie mentioned this issue Aug 6, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bounty enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@troggy @johannbarbie @jdkanani @davidbanu @vs77bb @gitcoinbot @pinkiebell