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

fix: add fix for checking for correct messageId in OPL2ToL1Ism and ArbL2ToL1Ism via external call #4437

Merged
merged 29 commits into from
Oct 22, 2024

Conversation

aroralanuk
Copy link
Contributor

@aroralanuk aroralanuk commented Sep 5, 2024

Description

  • In agreement with Chainlight team's recommendation, added a second isVerified() check after portal call to make sure an arbitrary call which passes the check for metadata length and messageId cannot be verified without calling is verifyMessageId(messageId) in OPL2ToL1Ism and ArbL2ToL1

Drive-by changes

None

Related issues

Backward compatibility

No, but the contracts aren't deployed anywhere

Testing

Unit testing

Copy link

changeset-bot bot commented Sep 5, 2024

🦋 Changeset detected

Latest commit: 5f47590

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

This PR includes changesets to release 9 packages
Name Type
@hyperlane-xyz/core Patch
@hyperlane-xyz/helloworld Patch
@hyperlane-xyz/sdk Patch
@hyperlane-xyz/infra Patch
@hyperlane-xyz/cli Patch
@hyperlane-xyz/widgets Patch
@hyperlane-xyz/ccip-server Patch
@hyperlane-xyz/github-proxy Patch
@hyperlane-xyz/utils Patch

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

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 73.76%. Comparing base (9f5a17b) to head (5f47590).
Report is 79 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (9f5a17b) and HEAD (5f47590). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (9f5a17b) HEAD (5f47590)
2 1
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4437      +/-   ##
==========================================
- Coverage   82.33%   73.76%   -8.57%     
==========================================
  Files         100      100              
  Lines        1421     1422       +1     
  Branches      180      182       +2     
==========================================
- Hits         1170     1049     -121     
- Misses        251      352     +101     
- Partials        0       21      +21     
Components Coverage Δ
core 84.61% <ø> (-12.31%) ⬇️
hooks 75.71% <ø> (-10.96%) ⬇️
isms 78.41% <33.33%> (-15.40%) ⬇️
token 88.23% <ø> (-5.23%) ⬇️
middlewares 77.39% <ø> (-9.14%) ⬇️

@aroralanuk aroralanuk changed the base branch from main to kunal/external-bridge-refactor September 20, 2024 10:31
@aroralanuk aroralanuk changed the title fix: add fix for checking for correct messageId in OPL2ToL1Ism via _verifyWithPortalCall() call fix: add fix for checking for correct messageId in OPL2ToL1Ism and ArbL2ToL1Ism via external call Sep 20, 2024
Base automatically changed from kunal/external-bridge-refactor to main September 20, 2024 20:22
@aroralanuk aroralanuk requested a review from ltyu as a code owner September 24, 2024 12:52
@yorhodes yorhodes self-requested a review October 8, 2024 14:56
@aroralanuk aroralanuk added this pull request to the merge queue Oct 22, 2024
Merged via the queue into main with commit c9085af Oct 22, 2024
31 of 38 checks passed
@aroralanuk aroralanuk deleted the kunal/HL2408-002-fix branch October 22, 2024 07:22
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…`ArbL2ToL1Ism` via external call (hyperlane-xyz#4437)

### Description

- In agreement with Chainlight team's recommendation, added a second
isVerified() check after portal call to make sure an arbitrary call
which passes the check for metadata length and messageId cannot be
verified without calling is verifyMessageId(messageId) in `OPL2ToL1Ism`
and `ArbL2ToL1`

### Drive-by changes

None

### Related issues

- fixes https://github.com/chainlight-io/2024-08-hyperlane/issues/2

### Backward compatibility

No, but the contracts aren't deployed anywhere

### Testing

Unit testing
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…`ArbL2ToL1Ism` via external call (hyperlane-xyz#4437)

### Description

- In agreement with Chainlight team's recommendation, added a second
isVerified() check after portal call to make sure an arbitrary call
which passes the check for metadata length and messageId cannot be
verified without calling is verifyMessageId(messageId) in `OPL2ToL1Ism`
and `ArbL2ToL1`

### Drive-by changes

None

### Related issues

- fixes https://github.com/chainlight-io/2024-08-hyperlane/issues/2

### Backward compatibility

No, but the contracts aren't deployed anywhere

### Testing

Unit testing
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…`ArbL2ToL1Ism` via external call (hyperlane-xyz#4437)

### Description

- In agreement with Chainlight team's recommendation, added a second
isVerified() check after portal call to make sure an arbitrary call
which passes the check for metadata length and messageId cannot be
verified without calling is verifyMessageId(messageId) in `OPL2ToL1Ism`
and `ArbL2ToL1`

### Drive-by changes

None

### Related issues

- fixes https://github.com/chainlight-io/2024-08-hyperlane/issues/2

### Backward compatibility

No, but the contracts aren't deployed anywhere

### Testing

Unit testing
tiendn pushed a commit to tiendn/hyperlane-monorepo that referenced this pull request Oct 25, 2024
…`ArbL2ToL1Ism` via external call (hyperlane-xyz#4437)

### Description

- In agreement with Chainlight team's recommendation, added a second
isVerified() check after portal call to make sure an arbitrary call
which passes the check for metadata length and messageId cannot be
verified without calling is verifyMessageId(messageId) in `OPL2ToL1Ism`
and `ArbL2ToL1`

### Drive-by changes

None

### Related issues

- fixes https://github.com/chainlight-io/2024-08-hyperlane/issues/2

### Backward compatibility

No, but the contracts aren't deployed anywhere

### Testing

Unit testing
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.

2 participants