feat: Devnet - Githubissues

shotaronowhere commented 1 year ago

This PR includes a few extra comments to the contracts, as well as exposing a utility function to the testnet operator for the devnets.

Summary

devnet contract updates
devnet contract deployment
devnet automation scripts
devnet relayer scripts
gnosis deployment scripts

netlify[bot] commented 1 year ago

Deploy Preview for veascan ready!

Name	Link
Latest commit	97db33eed22be78901bfff08e0abc20a6d521615
Latest deploy log	https://app.netlify.com/sites/veascan/deploys/646d18deec20ed00081b5e7e
Deploy Preview	https://deploy-preview-176--veascan.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

shotaronowhere commented 1 year ago

This recent commit implements changes according to Danil's review.

there was a bug in the escape hatch deposit withdrawal method, mistakenly the deposit was sent to address(0) instead of refunded
comments improved
Danil pointed out a return type on the IAMB interface is incorrect. I updated all of the interfaces with comments pointing to a particular repo and commit for the interface source and I pruned the interface for the essential function stubs

Note I made a subtle change in the L2->L1 message authentication which I want to make sure we are in agreement about.

instead of storing an immutable inbox address and authenticating the l2 message sender by checking

IOutbox(inbox.bridge().activeOutbox()).l2tol1sender()

we simply store an immutable bridge address directly and authenticate

IOutbox(bridge.activeOutbox()).l2tol1sender()

This is the same as the OpenZeppelin implementation.

Rationale:

there are multiple inboxes and potentially multiple outboxes, however there is a single bridge contract. The bridge contract is the contract which holds > 1 million eth.

other implementations like MakerDao's token gateway use the inbox address as a source of truth.

It makes more sense to use the bridge as a source of truth since the bridge contract is unique. Moreover, in the MakerDao context, the inbox is used to also send messages to L2. In our case, we don't use the inbox for anything other than L2->L1 message authentication. Meaning the only reason we keep track of the inbox is to de-reference the inbox.bridge() address value. This doesn't make any sense since the inbox doesn't serve a function in vea for arbitrum -> eth or arbitrum -> gnosis message passing. Keeping track of the bridge directly is the simplest approach, and the same way Open Zeppelin implements the l2->l1 message sender authentication.

I also want to note that the IInbox, IOutbox, and other canonical Arbitrum contract interfaces were outdated from before nitro. I included direct references to specific repos and commits where the full interfaces can be found, but I conclude the simplest approach is to prune the interface to only keep the function stubs we actually use for simplicity and clarity for reviewers.

If we are on the same page about this, I can post the contracts fully in smart contract reviews.