hyperlane-xyz / hyperlane-monorepo

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

Warp Routes should be deployable from a single chain/UI #3498

Open nambrot opened 7 months ago

nambrot commented 7 months ago

Problem

Deploying a warp route requires the download of the CLI, but more critically requires native gas funds on every chain to deploy the contracts.

Additional context Warp Route contracts are pretty large right now, so using a factory approach with metaproxies would also be very helpful

Solution

The ideal developer experience is to submit a much smaller set of txs from only a single chain. One can use something like https://github.com/hyperlane-xyz/hyperlane-monorepo/issues/2232, and effectively use the Hyperlane relayer to deploy contracts from a single chain. You could also consider using factories that are deployed on every chain and then call into those from a single origin chain. An open question is how developers would interact with this deployment logic. Ideally, this modifies the existing hyperlane deploy warp command where developers can input the same warp route config shape.

One thing to consider is that warp route contracts have to be aware of each other as a child of the Router contract. Thus there needs to be at least two transactions with Hyperlane messages to all chains: 1) to deploy and initialize the contracts and 2) to observe all the contract addresses and then call enrollRemoteRouters on all of them.

Nice to Have

Optional Extension: UI support Part of the difficulty of having a UI for warp route deployments was the requirement for having multiple native gas tokens and the fact that several transactions would have to be submitted on different chains which is difficult to do with the existing dapp/wallet APIs. With this, one could actually imagine a possible UX which has a constant/limited set of transactions that have to be submitted on a UI which makes Warp Route Deployment way more accessible. It could even suggest a Vercel deployment directly from the UI.

Optional Extension: Proxied contracts Deployers likely want to proxy the implementation contracts of Warp Routes so that future changes do not require a token migration. Instead of deploying a metaproxy (whose implementation can't change), one can consider full proxies where an admin can upgrade the implementation.

Optional Extension: ICA Ownership Warp Routes are ownable (so that other routers can be enrolled)

Romulobiscoito commented 6 months ago

goo

lmcorbalan commented 1 month ago

Hi @nambrot, from @BootNodeDev we would like to give this a try.

nambrot commented 1 month ago

lets goo!

nambrot commented 1 month ago

Just noticing right now that there is another variant of this bounty where you would be able to use the existing deployer logic in the SDK and just run it on a UI where users paste in a private key (that's ok since the ownership will be transferred to owner immediately at the end of the deploy process). This likely is much faster development time, but we would still classify it as a large because its valuable

lmcorbalan commented 1 month ago

Hey @nambrot sorry the delayed updates on this one

I was able to deploy 3 warp routes, on Sepolia, Optimism Sepolia and Arbitrum Sepolia, by executing some transactions only from Sepolia My idea was to use things that already existed, to prove if it was possible or not, learn more about the protocol and what could be improved if any.

What I did was:

1 - Deploy a HyperERC20 contract on the 3 chains which would be used as implementation of some TransparentUpgradeableProxy. I made a change on the contract for receiving the receiver address for the initial supply

2 - Deploy a TransparentUpgradeableProxy on Sepolia using the HyperERC20 implementation

3 - Using the InterchainAccountRouter sent some messages from Sepolia to Optimism Sepolia and Arbitrum Sepolia routers using callRemote, where the message contained 2 calls:

Links:

As next step I would like trying to make all this on a single transaction, I would probably need to update some contracts o create a new ones

nambrot commented 1 month ago

Thanks for the update here @lmcorbalan, exciting to see your progress. I kind didn't think of using precomputed addresses great idea. fwiw, if it simplifies things, doing two txs is also fine. i.e. first tx deploys all the contracts and then 2nd tx enrolls all the routers.

lmcorbalan commented 1 month ago

Hey @nambrot What do you think about adding to the InterchainAccountRouter a function for calling multiple remotes? This way I would be able to bundle the deployment of all the remote warp routes on a single tx.

yorhodes commented 1 month ago

Hey @nambrot What do you think about adding to the InterchainAccountRouter a function for calling multiple remotes? This way I would be able to bundle the deployment of all the remote warp routes on a single tx.

hey @lmcorbalan , its totally possible but this can also be done with a multicall outside of the ICA router is that sufficient for you?

lmcorbalan commented 1 month ago

Hey @nambrot What do you think about adding to the InterchainAccountRouter a function for calling multiple remotes? This way I would be able to bundle the deployment of all the remote warp routes on a single tx.

hey @lmcorbalan , its totally possible but this can also be done with a multicall outside of the ICA router is that sufficient for you?

Hi @yorhodes, I think it could work but the multicall should be owned by the address calling remotes, since the senderInterchainAccountRouter.callRemote caller ends being the owner of the remotes ICA. right?

yorhodes commented 1 month ago

Hi @yorhodes, I think it could work but the multicall should be owned by the address calling remotes, since the senderInterchainAccountRouter.callRemote caller ends being the owner of the remotes ICA. right?

thats a good point, I guess you could add a transferOwnership call at the end?

lmcorbalan commented 1 month ago

Hi @yorhodes, I think it could work but the multicall should be owned by the address calling remotes, since the senderInterchainAccountRouter.callRemote caller ends being the owner of the remotes ICA. right?

thats a good point, I guess you could add a transferOwnership call at the end?

I think it won't work because when handling messages the ICA address is determined using the owner encoded in the message which is the msg.sender. But I think I could transferOwnership of the deployed warp route to the precomputed ICA of the sender

yorhodes commented 1 month ago

But I think I could transferOwnership of the deployed warp route to the precomputed ICA of the sender

yes, either that or using the owner specified in the warp route config for that chain

lmcorbalan commented 1 month ago

Hi @nambrot @yorhodes. More updates. I was able to deploy 3 warp routes (Base Sepolia, Optimism Sepolia and Arbitrum Sepolia) on a single transaction executed on Base Sepolia.

The tx was this one resulting in the following warp routes:

I managed to bundle all the transaction on a single one by using the multicall approach discussed with @yorhodes.

I created a MulticallFactory smart contract that deploys an OwnableMulticall, similar to the one used by the InterchainAccountRouter for creating ICAs, the function that deploys the OwnableMulticall receives an array of Call which are forwarder to the OwnableMulticall after it gets deployed, so in that array of calls I included a call for deploying the local warp route and enrolling the remote routers and a call to the local InterchainAccountRouter that includes the same calls from the previous example.

You can find the forge script that I made for this POC here

nambrot commented 1 month ago

To add a bit more context here, IMO the artifact required to complete this bounty is to be able to take an existing warp config (the same that is used on the CLI) and get the warp route deployed. I.e. it should be API-compatible with https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/aaae6b5921ccb19b07f75ae221e19c9545702383/typescript/sdk/src/token/deploy.ts#L40.

Let me know if that makes sense

lmcorbalan commented 3 weeks ago

To add a bit more context here, IMO the artifact required to complete this bounty is to be able to take an existing warp config (the same that is used on the CLI) and get the warp route deployed. I.e. it should be API-compatible with

https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/aaae6b5921ccb19b07f75ae221e19c9545702383/typescript/sdk/src/token/deploy.ts#L40

. Let me know if that makes sense

Hey @nambrot what do you think about this https://github.com/BootNodeDev/hyperlane-wrid ?

nambrot commented 3 weeks ago

Taking a look (might be easier to make a PR again to track diffs/changes):

In the readme, you write

If you are going to deploy some synthetic routes you need to add decimals, name, symbol and totalSupply attributes in the configuration file since Hyperlane CLI doesn't do it.

Your warp-deploy task should be able to do that no?

There are some required params you need: PROXY_ADMIN_ADDRESS The warp routes are deployed using TransparentUpgradeableProxy so you need to set its admin address.

Are you saying that users would have to deploy their own proxy admin? Or that they just have to specify the owner of the proxy admin that would get deployed? In the latter, that should be part of the warp config (or default to msg.sender). For the former, I would expect that part of this scope there to be a proxy admin factory that would instantiate a proxy admin for you.

Maybe related to that last point:

It only supports the following testnets

Base Sepolia Optimism Sepolia Arbitrum Sepolia Sepolia

What are the cause of this limitation? If its the existence of factories and createx, i would expect part of this scope to be a way for folks to deploy these themselves, and then leverage them in the task.

lmcorbalan commented 3 weeks ago

@nambrot

nambrot commented 2 weeks ago

Approved 1, commented on 2, and for 3 lets figure out the import issue

lmcorbalan commented 2 weeks ago

Approved 1, commented on 2, and for 3 lets figure out the import issue

Do you mean the workaround for importing the hyperlane registry?

nambrot commented 1 week ago

I mean this comment https://github.com/BootNodeDev/hyperlane-wrid/pull/3/files#diff-5a4ef88b4466b7396ba3eb6a1f7fea9dfa701468fbd0af7351ad42587efe8786R38