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

More explicit support for getVerifiedWarpMessage as Anycast vs. Explicit Destination #868

Closed
aaronbuchwald opened this issue Sep 19, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request warp
Milestone

Comments

@aaronbuchwald
Copy link
Collaborator

The Warp precompile currently supports getVerifiedWarpMessage, which will verify that a warp message was signed by a threshold of the source blockchainID's validator set. The address-to-address payload type specifies a destinationChainID as well, and the current implementation does not require that the destinationChainID matches the actual blockchainID of the blockchain receiving this message.

The motivation for this was to support Anycast messages in a lightweight way and defer verification of the destinationChainID to the caller.

This ticket is a feature request to support a more explicit "safe" version of getVerifiedWarpMessage that does this check for you and separates the Anycast and explicit destination handler functions.

Specifically, this proposes that we break it down into two functions:

  • getVerifiedWarpMessage which verifies the destinationChainID matches the receiving chain's blockchainID
  • getAnyCastVerifiedWarpMessage which verifies the destinationChainID is exactly the AnycastChainID defined in AvalancheGo (or alternatively keeps the current behavior of deferring to the caller to allow any blockchainID is specified for the destinationChainID
@aaronbuchwald aaronbuchwald added the enhancement New feature or request label Sep 19, 2023
@github-project-automation github-project-automation bot moved this to Backlog 🗄 in EVM Sep 19, 2023
@michaelkaplan13
Copy link
Contributor

This is an interesting idea, though my initial feeling that it is best to make as few assumptions as possible on how messages will be used by developers.

For instance, it's imaginable that a project would want to be able to receive warp messages sent to a specific chain that is neither their own chain ID or the anycast chain ID. It could be used for reporting messages sent or other uses cases aside from "anycasting". It'd be odd to have to use getAnyCastVerifiedWarpMessage in that case.

100% agree that it needs to be very clear that the destinationChainID for messages returned by getVerifiedWarpMessage can have any value and is not necessarily the current chain ID. Ultimately though, I think this may result in more code that needs to be maintained to offer the same functionality.

@michaelkaplan13
Copy link
Contributor

michaelkaplan13 commented Sep 19, 2023

Trying to think how else an additional guard rail around the destinationChainID could be implemented here, which is what I think the real goal is.

One option could be to add an allowOtherChainIDs boolean parameter to getVerifiedWarpMessage. It would need to be explicitly set to true to allow chain IDs that are not the chain ID of the current chain. Protocol contracts such as TeleporterMessenger would set it to false rather than having to veryify the destinationChainID in their own logic.

This may make it more clear what the parameter is used for rather than having two different precompile methods.

@richardpringle
Copy link
Contributor

I think (but don't know) that in most cases, if you (the smart-contract) already know the index of the message you're looking for, you should know the destinationChainID. The only way I see this breaking down is if a contract is looping over indices or something like that.

If we think that arbitrarily reading warp messages is a use-case we want to support (as we do now), I recommend the following interface:

    // only returns messages with `chainID` == this.getBlockchainID()
    function getVerifiedWarpMessage(uint32 index)
        external view
        returns (WarpMessage calldata message, bool valid);

    // only returns messages with `chainID` == `blockchainID` 
    function getVerifiedWarpMessage(uint32 index, bytes32 blockchainID)
        external view
        returns (WarpMessage calldata message, bool valid);
        
    // exact same functionality as the current `getVerifiedWarpMessage`    
    // return any message without validating the `chainID` 
    function getAnyVerifiedWarpMessage(uint32 index)
        external view
        returns (WarpMessage calldata message, bool valid);

@michaelkaplan13
Copy link
Contributor

I think (but don't know) that in most cases, if you (the smart-contract) already know the index of the message you're looking for, you should know the destinationChainID.

I agree that smart contracts should know the expected destinationChainID of any message, though I don't think it's necessarily related to the message index. In Teleporter for example, the message index is passed by the caller to TeleporterMessage.receiveCrossChainMessage as a parameter itself (because there isn't a good way for the contract to know which message is to be received), but it knows that for all messages the destinationChainID should be the current chain ID.

The interface you suggested seems totally reasonable to me. I think it's important to support possible use cases for receiving messages with arbitrary destinationChainIDs. I don't have a strong preference and would definitely defer to others to decide, but my slight preference is that I think it would be more clear for developers to still have a single interface method for receiving messages rather than having 3 with similar methods with slightly different functionality. IIUC, the same set of possible functionality is supported in either case, so ultimately does not make huge difference either way. 👍

@nytzuga nytzuga self-assigned this Sep 27, 2023
@aaronbuchwald
Copy link
Collaborator Author

Discussing an alternative to this of removing the destinationChainID and destinationAddress field completely in favor of deferring that to Teleporter instead since the only thing Teleporter or any other protocol really needs from Warp is to encode the destinationChainID and destinationAddress since a protocol built on top can encode their own payload instead.

ava-labs/avalanchego#2050

@ceyonur ceyonur added this to the v0.5.7 milestone Oct 2, 2023
@github-project-automation github-project-automation bot moved this from Backlog 🗄 to Done ✅ in EVM Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request warp
Projects
No open projects
Archived in project
Development

No branches or pull requests

5 participants