Skip to content
This repository has been archived by the owner on Jul 11, 2019. It is now read-only.

Improve memory handling of the Proxy contract #70

Closed
facuspagnuolo opened this issue Apr 25, 2018 · 4 comments
Closed

Improve memory handling of the Proxy contract #70

facuspagnuolo opened this issue Apr 25, 2018 · 4 comments
Assignees
Labels
kind:discussion To discuss on a task before implementing it kind:enhancement An upgrade or a new feature that improves the system topic:upgradeability Upgreadeability for user contracts
Milestone

Comments

@facuspagnuolo
Copy link
Contributor

Some issues here:

  • the free memory pointer is never updated
  • we are using ptr again here without re-reading it.
@facuspagnuolo facuspagnuolo added kind:enhancement An upgrade or a new feature that improves the system topic:upgradeability Upgreadeability for user contracts labels Apr 25, 2018
@facuspagnuolo facuspagnuolo added this to the v0.1.0 (Kernel MVP) milestone Apr 25, 2018
@frangio
Copy link
Contributor

frangio commented Apr 25, 2018

Within the assembly block the free pointer should be guaranteed to not change, unless we explicitly do otherwise. Unfortunately I don't think we will find this documented, but it's the expected behavior of inline assembly in any programming language.

The delegated call uses a different memory space.

@facuspagnuolo
Copy link
Contributor Author

Agree, unfortunately I didn't find that in the official documentation too. But there is one inline assembly example showing how the the free memory pointer is updated. And there is another here showing how a memory allocate works updating the free memory pointer too.

However, there is no formal explanation whether it should be managed that way or not. Given the delegatecall uses a different memory space, I think we can just add a comment saying why there is no need to update the pointer

@facuspagnuolo facuspagnuolo added the kind:discussion To discuss on a task before implementing it label Apr 25, 2018
@alcuadrado
Copy link

alcuadrado commented Apr 27, 2018

Neither updating the free memory pointer nor using it is needed. Using 0 instead of ptr is safe because:

  1. Nothing can happen after the inline assembly block. It always run return or revert, and both of them stop the evm execution. So the ABI doesn't need to be followed after the assembly block.

  2. The proxied contract uses a new address space after the delegatecall, so we can do whatever we want with the proxy's one.

@Eenae
Copy link

Eenae commented Jul 27, 2018

Proper resource accounting is just a good programming style. It'll help you mitigate potential errors in future. Consider this potential case: someone is adding extra code before delegatecall, possibly with extra memory consumption, effectively overwriting delegatecall arguments.

Above said is no longer relevant in case of using memory pointer 0x0 (skipping solidity "allocator" altogether).

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind:discussion To discuss on a task before implementing it kind:enhancement An upgrade or a new feature that improves the system topic:upgradeability Upgreadeability for user contracts
Projects
None yet
Development

No branches or pull requests

4 participants