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.

Sign up for GitHub

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

Jump to bottom

[DEPLOY-178]: Adds Scroll L2EP Contracts #11405

Merged
merged 23 commits into from
Dec 12, 2023
Merged

[DEPLOY-178]: Adds Scroll L2EP Contracts #11405

merged 23 commits into from
Dec 12, 2023

Conversation

chris-de-leon-cll
Copy link
Contributor

Overview

This PR adds the L2EP Scroll contracts and corresponding test cases.

Contracts

Core

  • ScrollCrossDomainForwarder
  • ScrollCrossDomainGovernor
  • ScrollSequencerUptimeFeed
  • ScrollValidator

Mocks

  • MockScrollL1CrossDomainMessenger
  • MockScrollL2CrossDomainMessenger
  • MockScrollCrossDomainMessenger

Tests

  • ScrollCrossDomainForwarder.test.ts
  • ScrollCrossDomainGovernor.test.ts
  • ScrollSequencerUptimeFeed.test.ts
  • ScrollValidator.test.ts

@github-actions GitHub Actions
Copy link
Contributor

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

@github-actions GitHub Actions
Copy link
Contributor

I see that you haven't updated any README files. Would it make sense to do so?

mohamed-mehany
mohamed-mehany requested changes Nov 29, 2023
View reviewed changes
Copy link
Collaborator

@mohamed-mehany mohamed-mehany left a comment

Choose a reason for hiding this comment

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

@chris-de-leon-cll Should we also add the test files? I can only find the mock files.

@chris-de-leon-cll
Copy link
Contributor Author

chris-de-leon-cll commented Nov 29, 2023 •
edited
Loading

@mohamed-mehany I'm aiming to add the test files by the end of today

Apologies for the confusion - I opened this as a draft PR to push up what I currently have so you can view the progress so far

I'll switch it from a draft PR to a real PR once all the changes are ready

@github-actions GitHub Actions
Copy link
Contributor

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

2 similar comments
@github-actions GitHub Actions
Copy link
Contributor

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

@github-actions GitHub Actions
Copy link
Contributor

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

@chris-de-leon-cll chris-de-leon-cll marked this pull request as ready for review November 30, 2023 06:01
@chris-de-leon-cll chris-de-leon-cll self-assigned this Nov 30, 2023
@mohamed-mehany
Copy link
Collaborator

Changes LGTM! Thanks @chris-de-leon-cll for working on this!
One minor question related to testing, would we be able to deploy those contracts to testnet and test calling the validate and updateStatus methods as an extra testing step, similar to what was done here: #6885?

@chris-de-leon-cll chris-de-leon-cll force-pushed the feat/DEPLOY-178-l2ep-scroll branch from da88761 to 8073592 Compare November 30, 2023 19:49
@mohamed-mehany mohamed-mehany force-pushed the feat/DEPLOY-178-l2ep-scroll branch 2 times, most recently from 140c53c to 9d906a1 Compare December 1, 2023 01:38
@mohamed-mehany mohamed-mehany force-pushed the feat/DEPLOY-178-l2ep-scroll branch from 9d906a1 to ab23752 Compare December 1, 2023 01:39
mohamed-mehany
mohamed-mehany previously approved these changes Dec 1, 2023
View reviewed changes
mohamed-mehany
mohamed-mehany previously approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@mohamed-mehany mohamed-mehany left a comment

Choose a reason for hiding this comment

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

Thank you @RensR for guiding us through the Solidity style and best practices.

@RensR
Copy link
Contributor

RensR commented Dec 12, 2023

Looks good from a Solidity perspective, I'll let the product team approve on actual business logic

@cl-sonarqube-production
Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@RensR RensR added this pull request to the merge queue Dec 12, 2023
Merged via the queue into develop with commit 82faf5d Dec 12, 2023
93 of 94 checks passed
@RensR RensR deleted the feat/DEPLOY-178-l2ep-scroll branch December 12, 2023 21:21
momentmaker added a commit that referenced this pull request Dec 13, 2023
@momentmaker
Merge branch 'develop' into re-2110/setup-ci-core-scripts
858315e 
* develop: (56 commits)
  [TT-367] [TT-745] Quick and Dirty OCRv2 Soak Test (#11487)
  [FUN-990] s4 observability improvements (#11512)
  fix health monitoring (#11558)
  Removes Optimism Goerli from Scheduled Tests (#11559)
  bump Foundry to the December release (#11540)
  Standardize LP filter logging (#11515)
  Change keepers to use the default contract transmitter (#11308)
  bump toml/v2 and prometheus to latest patch (#11541)
  Remove big from core utils (#11511)
  Handle edge case involving blocks not being found in the db (#11298)
  [DEPLOY-178]: Adds Scroll L2EP Contracts (#11405)
  disable kaniko fallback, increase deploy wait timeout (#11548)
  Use multiple EL clients with ocrv2 median smoke test (#11399)
  Remove core utils dependencies from common (#11425)
  [BCF-2760] Flakey test detection improvements (#11470)
  go.mods: rm libp2p; rm btcd replace (#11502)
  wrap devspace commands (#11530)
  small improvements based on comments (#11491)
  (test): Remove unnecessary fuzzing from Functions OnTokenTransfer tests (#11517)
  core/scripts/common: rm ava-labs/coreth; lint (#11451)
  ...
fbac pushed a commit that referenced this pull request Dec 14, 2023
@chris-de-leon-cll @RensR
[DEPLOY-178]: Adds Scroll L2EP Contracts (#11405)
c5459c6 
* Adds scroll L2EP contracts and tests

* fixes comments, adds fixed solidity compiler version, and adds scroll tech contracts as a dev dependency

* renames SCROLL_CROSS_DOMAIN_MESSENGER to i_SCROLL_CROSS_DOMAIN_MESSENGER for solhint

* removes unnecessary solhint disable rule

* moves scroll mocks from tests folder to l2ep folder

* resolve solhint errors for scroll

* proposed restructure to reduce inheritance

* removes extraneous comments from scroll contracts and refactors typeAndVersion

* use named parameters in mappings for scroll contracts

* removes extraneous empty comments from scroll contracts (again)

* removes unnecessary comment from scroll cross domain governor

* adds minor formatting updates to scroll mocks

* adds onlyL1Owner modifier back to ScrollCrossDomainGovernor

* adds style and formatting improvements

* adds formatting updates

* refactors scroll sequencer uptime feed to reduce gas and updates test suites

---------

Co-authored-by: Rens Rooimans <[email protected]>
This was referenced Feb 22, 2024
Closed
Closed
@snehaagni snehaagni mentioned this pull request Mar 7, 2024
Closed
This was referenced Mar 21, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RensR RensR RensR left review comments

@mohamed-mehany mohamed-mehany mohamed-mehany approved these changes

@simsonraj simsonraj simsonraj left review comments

@cds95 cds95 Awaiting requested review from cds95

@prashantkumar1982 prashantkumar1982 Awaiting requested review from prashantkumar1982

Assignees

@chris-de-leon-cll chris-de-leon-cll

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chris-de-leon-cll @mohamed-mehany @RensR @simsonraj