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

Outbox eip1153 draft #135

Closed
wants to merge 16 commits into from
Closed

Outbox eip1153 draft #135

wants to merge 16 commits into from

Conversation

gvladika
Copy link
Contributor

@gvladika gvladika commented Feb 7, 2024

Update Outbox to use transient storage for L2 to L1 context

@cla-bot cla-bot bot added the s label Feb 7, 2024
@gzeoneth
Copy link
Member

gzeoneth commented Feb 7, 2024

we can also use tstore on this

address prevOutbox = _activeOutbox;
_activeOutbox = msg.sender;
// We set and reset active outbox around external call so activeOutbox remains valid during call
// We use a low level call here since we want to bubble up whether it succeeded or failed to the caller
// rather than reverting on failure as well as allow contract and non-contract calls
(success, returnData) = _executeLowLevelCall(to, value, data);
_activeOutbox = prevOutbox;

@gvladika
Copy link
Contributor Author

gvladika commented Feb 7, 2024

we can also use tstore on this

address prevOutbox = _activeOutbox;
_activeOutbox = msg.sender;
// We set and reset active outbox around external call so activeOutbox remains valid during call
// We use a low level call here since we want to bubble up whether it succeeded or failed to the caller
// rather than reverting on failure as well as allow contract and non-contract calls
(success, returnData) = _executeLowLevelCall(to, value, data);
_activeOutbox = prevOutbox;

Added - 5a7b88c

Savings:

Gas diff compared to referent report:
depositEth: -216 (-0.34%)
withdrawEth_executeTransaction: -28508 (-40.58%)
withdrawToken_executeTransaction: -28093 (-21.58%)

@gvladika gvladika closed this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants