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

Kunal/v3 hooks #2580

Merged
merged 9 commits into from
Aug 2, 2023
Merged

Kunal/v3 hooks #2580

merged 9 commits into from
Aug 2, 2023

Conversation

aroralanuk
Copy link
Contributor

Description

  • Adding Optimism Hook for MailboxV3

Drive-by changes

  • dispatch function with hookMetadata

Related issues

Backward compatibility

No

Testing

None

@aroralanuk aroralanuk marked this pull request as ready for review July 31, 2023 19:46
@codecov
Copy link

codecov bot commented Jul 31, 2023

Codecov Report

Patch coverage: 6.12% and project coverage change: -1.34% ⚠️

Comparison is base (a78be69) 62.51% compared to head (d13bca4) 61.18%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v3    #2580      +/-   ##
==========================================
- Coverage   62.51%   61.18%   -1.34%     
==========================================
  Files          95       99       +4     
  Lines        1430     1466      +36     
  Branches      198      206       +8     
==========================================
+ Hits          894      897       +3     
- Misses        529      562      +33     
  Partials        7        7              
Files Changed Coverage Δ
solidity/contracts/MailboxV3.sol 13.95% <0.00%> (-2.27%) ⬇️
solidity/contracts/hooks/AbstractHook.sol 0.00% <0.00%> (ø)
solidity/contracts/hooks/DomainRoutingHook.sol 0.00% <0.00%> (ø)
solidity/contracts/hooks/MerkleTreeHook.sol 0.00% <0.00%> (ø)
solidity/contracts/hooks/PausableHook.sol 0.00% <0.00%> (ø)
solidity/contracts/isms/hook/OPStackISM.sol 0.00% <0.00%> (ø)
...idity/contracts/libs/hooks/OPStackHookMetadata.sol 0.00% <0.00%> (ø)
solidity/contracts/hooks/OPStackHook.sol 7.69% <7.69%> (ø)
...racts/isms/hook/AbstractMessageIdAuthorizedIsm.sol 20.00% <20.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

.github/workflows/node.yml Show resolved Hide resolved
solidity/contracts/MailboxV3.sol Outdated Show resolved Hide resolved
solidity/contracts/MailboxV3.sol Show resolved Hide resolved
solidity/contracts/hooks/OPStackHook.sol Show resolved Hide resolved
solidity/contracts/hooks/OPStackHook.sol Outdated Show resolved Hide resolved
solidity/contracts/isms/hook/AbstractHookISMV3.sol Outdated Show resolved Hide resolved
solidity/contracts/isms/hook/AbstractHookISMV3.sol Outdated Show resolved Hide resolved
solidity/contracts/libs/hooks/OPStackHookMetadata.sol Outdated Show resolved Hide resolved
.github/workflows/node.yml Show resolved Hide resolved
* @param _messageId Hyperlane ID for the message.
*/
function verifyMessageId(bytes32 _messageId) external virtual {
_isAuthorized();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have isAuthorized return a bool as the name suggests and require here

Comment on lines +4 to +10
library OPStackHookMetadata {
function msgValue(bytes calldata _metadata)
internal
pure
returns (uint256)
{
return uint256(bytes32(_metadata[0:32]));
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add a layout in natspec similar to the other metadata libs

@aroralanuk aroralanuk merged commit f56c9a4 into v3 Aug 2, 2023
15 of 19 checks passed
@aroralanuk aroralanuk deleted the kunal/v3-hooks branch August 2, 2023 17:38
@yorhodes yorhodes mentioned this pull request Aug 2, 2023
yorhodes pushed a commit that referenced this pull request Aug 18, 2023
### Description

- Adding Optimism Hook for MailboxV3

### Drive-by changes

- dispatch function with hookMetadata

### Related issues

- hyperlane-xyz/issues#513

### Backward compatibility

No

### Testing

None
@yorhodes yorhodes mentioned this pull request Aug 18, 2023
yorhodes pushed a commit that referenced this pull request Sep 13, 2023
Use mailbox callback to authenticate messages in hooks where necessary (#2609)

Co-authored-by: Kunal Arora <55632507+aroralanuk@users.noreply.github.com>
hashableric pushed a commit to many-things/hyperlane-monorepo that referenced this pull request Oct 14, 2023
- Adding Optimism Hook for MailboxV3

- dispatch function with hookMetadata

- https://github.com/hyperlane-xyz/issues/issues/513

No

None
hashableric pushed a commit to many-things/hyperlane-monorepo that referenced this pull request Oct 16, 2023
- Adding Optimism Hook for MailboxV3

- dispatch function with hookMetadata

- https://github.com/hyperlane-xyz/issues/issues/513

No

None
@yorhodes yorhodes mentioned this pull request Nov 16, 2023
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.

None yet

2 participants