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: Polygon ZKevm v2 hook and ism #4547

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

iamzubin
Copy link

@iamzubin iamzubin commented Sep 22, 2024

Description

This PR adds a hook and ISM contract for Polygon's zkEVM v2 bridge.

The contracts tap into the Polygon zkEVM bridge to ensure cross-chain messages come from a trusted source. This works by using the zkEVM bridge's exit nodes, which handle verifying the message integrity.

This PR also includes code from PR #3136

PR for docs : PR 40

Drive-by changes

None

Related issues
Fixes #2848

Backward compatibility
Yes

Copy link

changeset-bot bot commented Sep 22, 2024

⚠️ No Changeset found

Latest commit: 1b9ac7a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@iamzubin iamzubin changed the title Polygon ZKevm v2 hook and ism feat: Polygon ZKevm v2 hook and ism Sep 22, 2024
* @notice Message hook to inform the {Polygon zkEVM chain Ism} of messages published through
* the native Polygon zkEVM bridge bridge.
*/
contract PolygonZkevmHook is IPostDispatchHook, MailboxClient {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you inherit from AbstractMessageIdAuthHook here, it'll mean less code duplication?

bytes calldata,
bytes calldata
) external pure override returns (uint256) {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

But I imagine there's still some additional overhead for the relayer to submit the tx on the destination chain. You can follow using a childHook which is effectively an IGP:

metadata.msgValue(0) + childHook.quoteDispatch(metadata, message);

Copy link
Contributor

@aroralanuk aroralanuk left a comment

Choose a reason for hiding this comment

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

It's a good start, take a look at the comments. It'll be easier to separate this into two PRs for v1 and v2.

);
bytes32 messageId = message.id();

zkEvmBridge.bridgeMessage{value: msg.value}(
Copy link
Contributor

Choose a reason for hiding this comment

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

Only send the metadata.msgValue(0) and refund the rest

* @notice Message hook to inform the {Polygon zkEVM chain Ism} of messages published through
* the native Polygon zkEVM bridge bridge.
*/
contract PolygonZkevmV2Hook is IPostDispatchHook, MailboxClient {
Copy link
Contributor

Choose a reason for hiding this comment

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

inherit from AbstractMessageIdAuthHook here instead too

"PolygonzkEVMv2Hook: invalid destination domain"
);
require(
_zkEvmBridgeDestinationNetId <= 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

this can only be 0 or 1 then?

bytes calldata
) external view override returns (uint256) {
return
interchainGasPaymaster.quoteGasPayment(
Copy link
Contributor

Choose a reason for hiding this comment

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

make this child hook

bytes calldata _message
) external payable override {
bytes32 messageId = keccak256(_message);
uint256 gasPayment = interchainGasPaymaster.quoteGasPayment(
Copy link
Contributor

Choose a reason for hiding this comment

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

childHook.quoteDispatch()

VERIFIED_MASK_INDEX
);
if (_msgValue > 0) {
verifiedMessages[messageId] -= _msgValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

send extra msgValue (via the metadata) so we don't have leftover here

* @dev off-chain verification information for a given message.
* @param _message The message for which off-chain verification information is requested.
*/
function getOffchainVerifyInfo(
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need CCIP here, the relayer will be calling the verify function, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe helpful to make a quick mermaid diagram.

Copy link
Author

Choose a reason for hiding this comment

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

@curlypoint did make a diagram for v1, v2 also follows similar flow.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, that's something that was needed to be discussed as bridge needs to generate a proof to claim. the ccip call was for the hyperlane relayer to get that proof using PolygonZKevm api (tho that does not follow the ccip spec.

https://github.com/curlypoint/hyperlane-zkevm-ccip

* @notice Polygon zkEVM chain Ism that uses the Polygon zkEVM bridge to verify messages
*/
contract PolygonZkevmV2Ism is
ICcipReadIsm,
Copy link
Contributor

Choose a reason for hiding this comment

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

similar comments to v1

* @inheritdoc AbstractMessageIdAuthorizedIsm
*/
function _isAuthorized() internal view override returns (bool) {
bytes32 originSender = abi.decode(msg.data[4:], (bytes32));
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we need to access control msg.sender since msg.data[4:] can be arbitrarily set by the caller?

@@ -0,0 +1,25 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

we already have TestInterchainGasPaymaster

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Polygon zkEVM Hooks and ISMs
2 participants