hyperlane-xyz / hyperlane-monorepo

The home for Hyperlane core contracts, sdk packages, and other infrastructure
https://hyperlane.xyz
Other
311 stars 341 forks source link

LayerZero apps should be able to just transparently use Hyperlane #2185

Open nambrot opened 1 year ago

nambrot commented 1 year ago

If there is a contract/application already using LayerZero (i.e. the LayerZero Endpoint), then it should be able to easily just use Hyperlane without any code changes necessary.

A way of doing so is to build a middleware contract that acts as the LayerZero Endpoint but really just forwards the messages via the Hyperlane mailbox and then its sister contract receives the message on the destination and calls the intended recipient via lzReceive.

Prior art is in this PR (https://github.com/hyperlane-xyz/hyperlane-monorepo/pull/1411/files) which allowed applications to implement the v2 interface while still using v1 of Hyperlane.

Likely needs to be a mapping of domain ids. Recommend using the router pattern (Router.sol) to facilitate the sending of messages to the right sister contract

Also have to be mindful of relayer gas payments

PxGnome commented 1 year ago

Hello Team - Submission for this bounty here: https://github.com/hyperlane-xyz/hyperlane-monorepo/pull/2243 let me know of any adjustments thanks!

nambrot commented 1 year ago

Very cool, copying the feedback i have here:

PxGnome commented 1 year ago

Adjusted as per comments above and deployed here: https://github.com/PxGnome/hyperlane-interpreter

Just a few pointers:

nambrot commented 1 year ago

Can't access the repo 😅 but will review once you make it public.

It would be nice to have deployment scripts both ways. Foundry is nice for more advanced users, but we had run into issues with it here debugging can be hard for more novice users in a multi chain environment. Take a look at the tooling in hyperlane-deploy for an example. Specifically RouterDeployer should be helpful to you to do the enrollment of Routers for example

PxGnome commented 1 year ago

Whoops just made repo public.

If there is a real need to refactor back to hardhat can look into it and probably do it after also putting out the Axelar version so I can just do everything in foundry first

nambrot commented 1 year ago

Cool, good to take a quick look. Agree that hardhat doesn't need to be done right now, though I do think some sort of practical deployment story should exist. What is the context of MockLayerZeroRouter though? Seeing it in the deployment script looks a bit off tbh. The readme should include instructions as to how to deploy it.

Also, did you see my comment around the gas estimation? The storage-based approach looks surprising to me

PxGnome commented 1 year ago

Sure so replying on the above:

1. About MockLayerZeroRouter: Originally named it mock as I built it for testing but think it might be better to call this LayerZeroRouter as this is the main thing we are deploying since the other is an abstract contract that gets inherited.

Also the Mock version has a bunch of dummy functions that aren't supported by Hyperlane but exists in the ILayerZeroEndpoint due to the way the LayerZero team designed their system so someone might want to come in and customize those as they need.

Could also just take out the inheritances since that doesn't really do anything and just make it a LayerZeroRouter that can handle send() and passes messages to lzreceive()

2. Instructions for deployment Was planning to add a step by step on deployment after I'm sure of the implementation, thinking of a better way to do in foundry and might do it after Axelar set up so I can do all the writing at once.

3. Gas Estimation Issue So problem is that LayerZero's estimateFees() works different to the way Hyperlane's quoteGasPayment() has it afaik. Hyperlane needs a gas amount number provided whilst LayerZero gets an estimation from relayer directly for this. This is the part that is currently hardcoded in as a set up since we can't estimate how much gas is being used by the receiver necessarily (since we don't know what lzreceive() does).

So in theory user needs to:

  1. Deploy all the contracts then figure out how much gas they are using including for lzreceive()
  2. Update this gas amount into the LayerZeroRouter in setEstGasAmount()

Then as long as the user pays more gas than what we get in the LayerZeroRouter.estimateFees() then the message will process.

Happy to incorporate suggestions but this is my solution for now.

nambrot commented 1 year ago

Sure so replying on the above:

1. About MockLayerZeroRouter: Originally named it mock as I built it for testing but think it might be better to call this LayerZeroRouter as this is the main thing we are deploying since the other is an abstract contract that gets inherited.

Also the Mock version has a bunch of dummy functions that aren't supported by Hyperlane but exists in the ILayerZeroEndpoint due to the way the LayerZero team designed their system so someone might want to come in and customize those as they need.

Could also just take out the inheritances since that doesn't really do anything and just make it a LayerZeroRouter that can handle send() and passes messages to lzreceive()

I think just implementing no-op versions of the complete interface makes sense to me, don't feel strongly either way. You could implement them in a different contract and then just inherit if you want it to be cleaner? Do think not calling it Mock is the way to go tho.

2. Instructions for deployment Was planning to add a step by step on deployment after I'm sure of the implementation, thinking of a better way to do in foundry and might do it after Axelar set up so I can do all the writing at once.

Fine to do it later with Axelar

3. Gas Estimation Issue So problem is that LayerZero's estimateFees() works different to the way Hyperlane's quoteGasPayment() has it afaik. Hyperlane needs a gas amount number provided whilst LayerZero gets an estimation from relayer directly for this. This is the part that is currently hardcoded in as a set up since we can't estimate how much gas is being used by the receiver necessarily (since we don't know what lzreceive() does).

So in theory user needs to:

  1. Deploy all the contracts then figure out how much gas they are using including for lzreceive()
  2. Update this gas amount into the LayerZeroRouter in setEstGasAmount()

Then as long as the user pays more gas than what we get in the LayerZeroRouter.estimateFees() then the message will process.

Happy to incorporate suggestions but this is my solution for now.

Is that how L0's gas system works? I thought similar to Hyperlane, you have to know how much the target lzReceive consumes, how would the relayer know? IIUC, L0 just assumes 200k gas if you don't override it in the adapterParams?

PxGnome commented 1 year ago

Okay will clean things up a bit.

And regarding gas, yea double checking on:

They set default gas = 200000, but if I do this for my tests it doesn't pass hence I have larger default number since I assume you'd get refund on gas anyways if it doesn't get used.

Here they refer to a config file which I assume they have some config file set for each chain. However their docs and github isn't consistent on approach so not sure what the best reference is. 🤷

nambrot commented 1 year ago

Why doesn't it pass? This adapter should be transparently usable for LZ apps right? I.e. if they didn't specify the adapter params, then they should expect that any lzReceive on the destination which consumes less than 200k to succeed. Obviously you do have to calculate the overhead for this middleware and add that. I would also look deeper at the refund mechanism, IMO its just during estimation side for the given input, it will not refund you based on actual destination chain usage.

Which config file reference do you mean? Imo they talk about the uaConfig?

PxGnome commented 1 year ago

Fixes/Update below, let me know if there is anything else @nambrot, if this looks good I'll follow it for the Axelar bounty to complete and look to improve the documentation further when I have time.

Overall

Deployment

Gas Issue

nambrot commented 1 year ago

Hey @PxGnome sorry for the delay, just took another look again. Btw in the future feel free to make PRs if you want, that way I can leave comments on specific lines.

PxGnome commented 1 year ago

Hey @PxGnome sorry for the delay, just took another look again. Btw in the future feel free to make PRs if you want, that way I can leave comments on specific lines.

Ah do you mean I just fork the current repo and make a PR and then place the files there? If so will see if I bother since I set up all the files in the current git already 😅

  • Def agree the hardhat tasks are useful. I don't think it is strictly necessary, but I would recommend to take a look at the router deployment tooling that we use for example for deploying warp routes. That's the main thing that allows warp route deploy script to be this short while doing things like initialization and router enrollment automatically.

Hmmm okay, I guess I need to take a look at the multi-provider in the SDK.

  • Like the simplification/removal of Mock. I did just notice your comment about the nonce which is pretty important. What solutions were you considering to mimic the retry and ordering guarantees that L0 gives to applications?

For now I left it out since it isn't critical for it to work and felt that this is specific to the dApps usage in most cases and it doesn't seem like there is a way to retry messages on Hyperlane?

If necessary we could implement a blocking mechanism or some type of way to store the message being sent so that it could be retried if necessary but this would add to gas cost and I thought best to leave the currently implementation as minimal as possible.

Since having worked through this and comparing between the various bridges lately I personally see people bridging over from LayerZero (or Axelar) because:

So in this sense the use case I had in mind for this was more of a template starting point with an expectation the user would still customize

  • I'm still not entirely sure that we are on the same page around how L0 apps pay for destination chain gas. Would you mind expanding on why you say they are not being consistent. As far as I can tell, the link you set is indeed the mental model I have, which is that if I call send with adapter params where value is 300k, I can expect an lzReceive that consumes less than 300k to succeed. However, as far as I can tell, your implementation does not allow for such specification by the sender?

Can't recall exactly what I was referring to anymore at this point. For some reason I had a half finished implementation that isn't utilized for the adapterParams, I can easily fix this in the next update to do what you describe.


As a related point, I think I approached this more from a showcase solution with expectation that the user would customize and adapt areas whereas I guess you guys are looking at it as an out of the box solution?

I guess ultimately you guys want to see how to bridge people over from other apps and I was just thinking whether this middleware is the best or whether other solutions such as recreating examples of ONFT would be more helpful. Since personally as a user I don't really care too much about which cross chain messaging protocol I use as long as it gets the job done.

But yea happy to make adjustments to fit the brief just think that my own preferences may have resulted in a longer delay to me completing the bounty.

(Apologies for the rambling, been just doing work comparing messaging protocol. Anyways will update once I have time to update my code but may take a while as I have a few things more urgent).

nambrot commented 1 year ago

Hey @PxGnome sorry for the delay, just took another look again. Btw in the future feel free to make PRs if you want, that way I can leave comments on specific lines.

Ah do you mean I just fork the current repo and make a PR and then place the files there? If so will see if I bother since I set up all the files in the current git already 😅

Sorry, what I meant was to keep your repo, but when you make changes, make them on a different branch and then a PR into main, that way I could provide a review on specific change/lines. Not necessary though.

  • Like the simplification/removal of Mock. I did just notice your comment about the nonce which is pretty important. What solutions were you considering to mimic the retry and ordering guarantees that L0 gives to applications?

For now I left it out since it isn't critical for it to work and felt that this is specific to the dApps usage in most cases and it doesn't seem like there is a way to retry messages on Hyperlane?

If necessary we could implement a blocking mechanism or some type of way to store the message being sent so that it could be retried if necessary but this would add to gas cost and I thought best to leave the currently implementation as minimal as possible.

Yeah tbh I'm not super aware of what kind of expectations or assumptions LayerZero applications make, so happy to punt on it for now.

Since having worked through this and comparing between the various bridges lately I personally see people bridging over from LayerZero (or Axelar) because:

  • They want to deploy on a chain not supported by the existing messaging protocol
  • They want greater control over how the relaying works and don't want to share this infra with others

So in this sense the use case I had in mind for this was more of a template starting point with an expectation the user would still customize

Obviously I'm biased, but I would argue that security is also a big factor in this choice.

  • I'm still not entirely sure that we are on the same page around how L0 apps pay for destination chain gas. Would you mind expanding on why you say they are not being consistent. As far as I can tell, the link you set is indeed the mental model I have, which is that if I call send with adapter params where value is 300k, I can expect an lzReceive that consumes less than 300k to succeed. However, as far as I can tell, your implementation does not allow for such specification by the sender?

Can't recall exactly what I was referring to anymore at this point. For some reason I had a half finished implementation that isn't utilized for the adapterParams, I can easily fix this in the next update to do what you describe.

As a related point, I think I approached this more from a showcase solution with expectation that the user would customize and adapt areas whereas I guess you guys are looking at it as an out of the box solution?

Yeah the intention for this bounty is very much to make a migration of a LayerZero application to Hyperlane super smooth. Obviously, those applications could litereally just implement Hyperlane directly, so the point of a middleware like this is to reduce the surface area to an absolute minimum. Deploying the middleware out of the box with a single invocation of a script and then pointing to these "endpoints" is that IMO.

I guess ultimately you guys want to see how to bridge people over from other apps and I was just thinking whether this middleware is the best or whether other solutions such as recreating examples of ONFT would be more helpful. Since personally as a user I don't really care too much about which cross chain messaging protocol I use as long as it gets the job done.

Absolutely agree that most end-users right now don't care all that much, it's really the developer. And you are also right that show casing let's say Warp Routes as alternatives to OFTs are important. This middleware is more for already existing applications or deployments that already have some "sunk cost".

But yea happy to make adjustments to fit the brief just think that my own preferences may have resulted in a longer delay to me completing the bounty.

(Apologies for the rambling, been just doing work comparing messaging protocol. Anyways will update once I have time to update my code but may take a while as I have a few things more urgent).

No apologies needed! This is our first swing at bounties, and we take your feedback seriously! We'll try to set better expectations going forward as we super appreciate your engagement and patience! Hope we haven't scared you 😅 Feel free to DM me for more feedback

nambrot commented 7 months ago

At this point, Hyperlane ISMs should probably be a DVN that L0 apps can use