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

[WIP] EEI: charge for memory gas cost in useGas #65

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

axic
Copy link
Member

@axic axic commented Jan 15, 2018

No description provided.

@axic
Copy link
Member Author

axic commented Jan 24, 2018

Depends on decision in ewasm/design#75

@axic axic changed the title EEI: charge for memory gas cost in useGas [WIP] EEI: charge for memory gas cost in useGas Jan 24, 2018
@axic axic force-pushed the memorygas branch 2 times, most recently from b9d8b07 to c4ba6a1 Compare March 12, 2018 00:17
@@ -199,6 +199,9 @@ string toHex(evmc_uint256be const& value) {

takeGas(gas);

heraAssert((ffsl(gas) + ffsl(memory.size()) <= 64), "Memory gas calculation overflow."); //may need to find alternative to ffsl for cross-libc portability
takeGas(gas * memory.size() / GasSchedule::memoryPageSize * GasSchedule::memoryCostPerPage + 1); //round gas cost up
Copy link
Member

Choose a reason for hiding this comment

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

Unsure whether this should be rounded up or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure. What are the pros/cons for each?

@jakelang
Copy link
Member

rebased

@jakelang jakelang added the blocked by spec ⚠️ Depends on design decision label May 29, 2018
@axic axic marked this pull request as draft June 9, 2020 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked by spec ⚠️ Depends on design decision in review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants