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: Create EvmCoreModule with create() #3737

Merged
merged 32 commits into from
May 27, 2024
Merged

feat: Create EvmCoreModule with create() #3737

merged 32 commits into from
May 27, 2024

Conversation

ltyu
Copy link
Contributor

@ltyu ltyu commented May 8, 2024

Description

This Pull Request introduces the new EVM core module be implementing the create() function. Currently, create() will call the existing CoreDeployer

Related issues

Backward compatibility

Yes

Testing

Unit Tests

Copy link

changeset-bot bot commented May 8, 2024

🦋 Changeset detected

Latest commit: 11fb842

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@hyperlane-xyz/sdk Minor
@hyperlane-xyz/cli Minor
@hyperlane-xyz/helloworld Minor
@hyperlane-xyz/infra Minor
@hyperlane-xyz/core Minor
@hyperlane-xyz/ccip-server Minor
@hyperlane-xyz/utils Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ltyu
Copy link
Contributor Author

ltyu commented May 8, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ltyu and the rest of your teammates on Graphite Graphite

@ltyu ltyu changed the title Add initial commit Create EvmCoreModule May 8, 2024
@ltyu ltyu marked this pull request as ready for review May 8, 2024 19:21
@ltyu ltyu changed the title Create EvmCoreModule [Draft] Create EvmCoreModule May 8, 2024
@nbayindirli nbayindirli changed the title [Draft] Create EvmCoreModule Create EvmCoreModule May 9, 2024
@ltyu ltyu changed the title Create EvmCoreModule feat: Create EvmCoreModule with create() May 9, 2024
typescript/sdk/src/core/types.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Your new classes mostly look good to me! Raised some concerns about the config handling that I see emerging but it doesn't have to be solved here.

typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/test/testUtils.ts Outdated Show resolved Hide resolved
Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

glad we could mostly reuse existing deployer code :)

typescript/sdk/src/core/CoreDeployer.hardhat-test.ts Outdated Show resolved Hide resolved
typescript/sdk/src/test/testUtils.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.96%. Comparing base (3dabcbd) to head (11fb842).

Additional details and impacted files
@@           Coverage Diff            @@
##           cli-2.0    #3737   +/-   ##
========================================
  Coverage    63.96%   63.96%           
========================================
  Files          124      124           
  Lines         1543     1543           
  Branches       167      167           
========================================
  Hits           987      987           
  Misses         530      530           
  Partials        26       26           
Components Coverage Δ
core 84.61% <ø> (ø)
hooks 67.50% <ø> (ø)
isms 39.52% <ø> (ø)
token 65.41% <ø> (ø)
middlewares 77.05% <ø> (ø)

@ltyu ltyu requested review from yorhodes and paulbalaji and removed request for nambrot, Skunkchain and avious00 May 23, 2024 20:01
Copy link
Contributor

@paulbalaji paulbalaji left a comment

Choose a reason for hiding this comment

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

just nits but lgtm, will let yorhodes ✅

typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
@paulbalaji
Copy link
Contributor

don't forget to add a changeset!

ltyu and others added 10 commits May 24, 2024 12:26
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Co-authored-by: Paul Balaji <paul@hyperlane.xyz>
Copy link
Member

@yorhodes yorhodes left a comment

Choose a reason for hiding this comment

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

lgtm
I expect there to be a little more complexity in the update

typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
typescript/sdk/src/core/EvmCoreModule.ts Outdated Show resolved Hide resolved
@ltyu ltyu merged commit eb23e77 into cli-2.0 May 27, 2024
29 checks passed
@ltyu ltyu deleted the ltyu/core-module branch May 27, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants