-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
EIP 5: Gas costs for return values #8
Comments
This would be quite useful from a contract/library author's perspective. It currently requires some major hoop jumping to get bytes from another contract. |
Voting for this proposal. IMHO this will be a huge jump to multi-contract apps. |
So would there be a max allocation size the the callee would specify? |
I have thought about this a bit more in the last day and tried to implement some generic call forwarding stuff in serpent, and have really come to appreciate the wisdom of this change and how much easier it would make it to do a lot of things.
Yes. However, if you trust the callee absolutely, you can just set it to 2**100; but this isn't as scary as it seems as contracts trust the callee with all of their gas by default already. |
👍 |
Here is my proposed implementation in python: The way it currently works: http://vitalik.ca/files/old.py |
lgtm |
+1 |
+1! This unlocks the full potential of "dynamic" contracts. Without this, you can't use Solidity to write a contract that could be updated to implement an arbitrary Solidity ABI. |
What is the status of this proposal? I'm currently running into this issue. :) |
@kumavis suggested that I expand on my usecase for this. Basically, this would allow generic middleware and generic call-forwarding in a way that you can't really do right now. This is particularly useful for "identity" in various forms. I want to write a contract that serves as the canonical address for an entity (a person, dapp, DAO, token, etc.), but that is capable of forwarding incoming or outgoing calls to other contracts. Right now, this is only possible if this middle contract has special knowledge about the interior workings of the destination contract, and I would like to be able to do it generically. For an example of how it currently has to be done, check out the experiments I'm doing here: https://github.com/PeterBorah/ether-router/blob/master/contracts/EtherRouter.sol In line 11, I have to explicitly look up the expected return size in a separately-maintained database, so that I can allocate enough memory to pass back the return value. |
There's also a simpler usecase, which is that Solidity is currently unable to call a function that returns a dynamically-sized array, because it needs to know the size of the array before it makes the call. |
In the meantime, another possible solution to the problem was proposed: Charge the gas to the callee instead of the caller. Advantages:
Disadvantages:
In comparison, the original proposal of paying from caller's gas after returning the gas left in the callee's context: Advantages:
Disadvantages:
|
+1 |
Disadvantage of original proposal:
To be clear, until now the formal spec for the EVM dispatch sequence is:
This would fundamentally alter to:
This would add a base layer of additional complexity and reduce our ability to reason about edge circumstances particularly regarding possible attack scenarios due to gas usage. |
While indeed, this adds another step for call-like opcodes, you are hiding a lot of complexity that is already present, especially with regards to call-like opcodes:
And perhaps some other steps I have missed here. With this Proposal, this turns into:
Or, to make that even simpler: Every opcode and step that accesses memory grows memory and this growing of memory has to be paid with gas. Before this proposal, the call semantics were:
With this proposal:
I am sorry, but I think it is just ridiculous that this has been open for over half a year now although we have general acceptance inside the community and among most of the client developers and a gigantic need for this feature. |
@chriseth @gavofyork: An alternative solution is to charge the gas to the caller for the reserved memory area and refund (include in the refund counter) the difference of reserved and actually written bytes. The disadvantage is that a much higher gas limit is needed as the refund counter is only processed after the transaction has finished. |
@axic unfortunately, that will not solve the problem, because the calling contract does not know how much to reserve. |
I would like to give a simple summary of the two proposals again: The designated output area is not taken into account for enlarging memory before the call. At the point where the call returns, if the return value is shorter than the output area, the output area effectively shrinks to the size of the return value. Memory is enlarged if necessary. Proposal A: remaining gas from the call is refunded to the caller; caller pays for enlarging memory for output, throws OOG if not enough gas. Otherwise, return value is written to memory. Proposal B: memory is enlarged and paid for from the remaining gas of the call. If that is not enough, the call fails (returns zero). Otherwise, return value is written to memory and remaining gas is refunded to the caller. Proposal B is backwards incompatible, but this case is not used by the solidity compiler except if you manually specify gas. |
Proposal B also plays nicely with #90 |
Can you elaborate why it would play nicer than A, or is that not what you are saying? |
My general opinion is that it is much easier (and better) to add a new opcode, instead of reusing existing one and creating complex combos from single propose opcodes. So in this case I would add |
Proposal B is problematic with regards to existing code: Calls to e.g. the identity precompile might be more expensive than before (meaning the call will consume more gas, not the call opcode itself), but there are existing contracts that always supply a very specific amount of gas for the identity precompile. In order to not break these contracts, we can alter the proposal so that the new rules only take effect if the specified size of the output area is If the size of the output area is different from If the size of the output area is equal to |
To follow up on that ^ Specifying output
That means only resizing to increase, not decrease the memory area, right ? |
There currently is no way to reduce the allocation of memory, and with resize I actually meant "enlarge if needed". I don't think that it is worth the hassle to provide such a means. If you reduce the size, would that mean that you get a gas refund? Also, I don't think we should include this complication into this EIP. |
so...what's the final decision? |
Please clarify that this means it also puts a 0 on the stack, so default caller behavior will be to re-throw (hopefully via REVERT) - in other words, it still fails by "running out of gas" |
See #211. |
+1 |
I guess this can be closed now. @Souptacular what is the process here? |
@chriseth @Souptacular I think this can be closed given the draft of this is merged and has been deprecated since by EIP211 (the latter also states "replaces: 5"). |
* EIP Motivation and Specification for "Multi-operator, per-token ERC721 approval." * Add remaining authors' names * Interface overhaul with explanation in EIP markdown. * Add `Abstract` and `@dev` note about interplay with `setApprovalForAll()`. * Partial response to cxkoda review * Full response to cxkoda review * Explicitly include OZ `IERC{165,721}.sol` instead of using a submodule * Minor typo and wording changes to draft. * Prune authors to active participants as they can always be re-added later * Revert `.gitignore` so as to not change it in the EIP draft (#7) * Set presumptive EIP number and fix linter complaints (#5) * Rename files and directories to include EIP number * Address linter (`EIP Walidator`) issues * Add presumed EIP number based on PR: 6464 * Revert `.gitignore` to get through initial draft acceptance * Do the last commit properly ;) * Add ERC category * Fix linter issues (#8) * Update EIP- references to ERC- * Remove OpenZeppelin interfaces and replace with original EIP versions; change 6464 license to CC0 * Add `solidity` language tag to code block
* EIP Motivation and Specification for "Multi-operator, per-token ERC721 approval." * Add remaining authors' names * Interface overhaul with explanation in EIP markdown. * Add `Abstract` and `@dev` note about interplay with `setApprovalForAll()`. * Partial response to cxkoda review * Full response to cxkoda review * Explicitly include OZ `IERC{165,721}.sol` instead of using a submodule * Minor typo and wording changes to draft. * Prune authors to active participants as they can always be re-added later * Revert `.gitignore` so as to not change it in the EIP draft (ethereum#7) * Set presumptive EIP number and fix linter complaints (ethereum#5) * Rename files and directories to include EIP number * Address linter (`EIP Walidator`) issues * Add presumed EIP number based on PR: 6464 * Revert `.gitignore` to get through initial draft acceptance * Do the last commit properly ;) * Add ERC category * Fix linter issues (ethereum#8) * Update EIP- references to ERC- * Remove OpenZeppelin interfaces and replace with original EIP versions; change 6464 license to CC0 * Add `solidity` language tag to code block
* updates eip5573 to reflect changes in CACAO and UCAN specs (#7) * updates eip5573 to reflect changes in CACAO and UCAN specs * clearly define the meaning of lexicographic ordering * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * remove unnecessary comma * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * remove uri field from statement * update recap-uri example * clarify that recap uris go at the end of the resource list * clarify comparison to OIDC/OAuth2 * specify how recap objects can be combined * remove mention of namespace * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * better wording of recap purpose/niche * specify b64 without padding * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * consistent spelling * rename delegee -> Relying Party * update example recap uri * fix abstract wording a little bit * use reference for b64url link * fix references layout * apply markdown linter --------- Co-authored-by: Jacob <jacob.ward@spruceid.com> Co-authored-by: Samuel Gbafa <skgbafa@gmail.com> * change EIP to ERC, remove non-relative link (#8) --------- Co-authored-by: chunningham <c.a.cunningham6@gmail.com> Co-authored-by: Jacob <jacob.ward@spruceid.com>
* EIP Motivation and Specification for "Multi-operator, per-token ERC721 approval." * Add remaining authors' names * Interface overhaul with explanation in EIP markdown. * Add `Abstract` and `@dev` note about interplay with `setApprovalForAll()`. * Partial response to cxkoda review * Full response to cxkoda review * Explicitly include OZ `IERC{165,721}.sol` instead of using a submodule * Minor typo and wording changes to draft. * Prune authors to active participants as they can always be re-added later * Revert `.gitignore` so as to not change it in the EIP draft (ethereum#7) * Set presumptive EIP number and fix linter complaints (ethereum#5) * Rename files and directories to include EIP number * Address linter (`EIP Walidator`) issues * Add presumed EIP number based on PR: 6464 * Revert `.gitignore` to get through initial draft acceptance * Do the last commit properly ;) * Add ERC category * Fix linter issues (ethereum#8) * Update EIP- references to ERC- * Remove OpenZeppelin interfaces and replace with original EIP versions; change 6464 license to CC0 * Add `solidity` language tag to code block
) * updates eip5573 to reflect changes in CACAO and UCAN specs (ethereum#7) * updates eip5573 to reflect changes in CACAO and UCAN specs * clearly define the meaning of lexicographic ordering * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * remove unnecessary comma * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * remove uri field from statement * update recap-uri example * clarify that recap uris go at the end of the resource list * clarify comparison to OIDC/OAuth2 * specify how recap objects can be combined * remove mention of namespace * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * better wording of recap purpose/niche * specify b64 without padding * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * Update EIPS/eip-5573.md Co-authored-by: Jacob <jacob.ward@spruceid.com> * consistent spelling * rename delegee -> Relying Party * update example recap uri * fix abstract wording a little bit * use reference for b64url link * fix references layout * apply markdown linter --------- Co-authored-by: Jacob <jacob.ward@spruceid.com> Co-authored-by: Samuel Gbafa <skgbafa@gmail.com> * change EIP to ERC, remove non-relative link (ethereum#8) --------- Co-authored-by: chunningham <c.a.cunningham6@gmail.com> Co-authored-by: Jacob <jacob.ward@spruceid.com>
the test accidentally demonstrated the behavior of MCOPY with length=9, instead of length=8. moved the space to byte ethereum#8 for clarity
Apply latest discussions
Proposal
Change memory gas costs for return values of a call to be applied depending on the size of the written memory area, not the reserved memory area.
Current Situation
In a call, the caller has to provide an area where the returned data from a call is written to. Only reserving it already changes gas costs for memory.
Advantage
This makes it possible to read a return value whose size is not known to the caller prior to the call: The caller just reserves a huge amount of memory.
Possible Extension
The
CALL
opcode (and its friends) may even return the actual size of memory written to, so that the data can be used (e.g. forwarded) even if nothing is known about its structure.Implementation
The implementation might be a bit tricky: The
CALL
opcode has to be charged some gas upfront, then the call is performed without allocating memory for the return value. At the point, the return value is available, gas costs are computed and if some gas is still available, memory is allocated and written to.The text was updated successfully, but these errors were encountered: