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

feat: ban deposits interop #11454

Conversation

0xDiscotech
Copy link
Contributor

Description

Added benchmark for the setValuesEcotone() and setValuesIsthmus() functions.

Additional context

#11362 (review)

* refactor: not cross l2 inbox error
* refactor: not cross l2 inbox error
* fix: stick to natspec standards
…mism/optimism into feat/ban-deposits-interop
* refactor: move the isthmus test logic to the l1 block test contract
* chore: remove unused imports
…mism/optimism into feat/ban-deposits-interop
@0xDiscotech 0xDiscotech requested a review from a team as a code owner August 12, 2024 23:29
@0xDiscotech 0xDiscotech requested review from refcell and removed request for a team August 12, 2024 23:29
@tynes
Copy link
Contributor

tynes commented Aug 13, 2024

To be backwards compatible, we should follow this document: https://github.com/ethereum-optimism/design-docs/blob/main/smart-contract-feature-development.md

This would mean we create a L1BlockInterop contract (we may already have one)

@tynes
Copy link
Contributor

tynes commented Aug 13, 2024

Did you mean to make this just a benchmark PR? If so this looks good to me

@0xDiscotech
Copy link
Contributor Author

Did you mean to make this just a benchmark PR? If so this looks good to me

Yes! The idea is to add it that branch, so it can be reviewed on this PR: #11362
I had to push it from our repo because I don't have permission to push directly. But the idea is that @skeletor_spaceman merges it into his branch the solidity files have the requested changes :)

@0xDiscotech
Copy link
Contributor Author

0xDiscotech commented Aug 13, 2024

This would mean we create a L1BlockInterop contract (we may already have one)

Yes, this was also pushed to that same branch. The L1BlockInterop already exists so we basically added the new functions on it!

But of course, if something is wrong or missing, let us know so we can push the changes

@tynes
Copy link
Contributor

tynes commented Aug 13, 2024

This would mean we create a L1BlockInterop contract (we may already have one)

Yes, this was also pushed to that same branch. The L1BlockInterop already exists so we basically added the new functions on it!

But of course, if something is wrong or missing, let us know so we can push the changes

Ah ok, apologies if i misread something!

Thank you for breaking this up into a small PR, i mistook this as for onto develop

@skeletor-spaceman skeletor-spaceman merged commit da494fd into ethereum-optimism:feat/ban-deposits-interop Aug 13, 2024
52 of 58 checks passed
@skeletor-spaceman skeletor-spaceman deleted the feat/ban-deposits-interop branch August 13, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants