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
Arbitrum L1 to L2 messaging proposal #13516
Changes from 6 commits
f0cbaf2
85376b1
247edf0
2f0608f
b9c3467
20939ee
506fdc6
09aada2
6cd79c2
cd41637
80b6fe6
14273ca
6709e11
a6942e4
b54ffa4
347d6bf
98935ee
e98703b
6c9f33a
3e817b7
39b4e79
24d81c3
eabfe4d
cbdfb95
fa63c3d
0f7ec36
ce0e970
c1fd7d7
7bced2f
0a993b8
c9a28c1
4d42172
c33c977
79327c5
ace2d13
794914d
a82a7c0
debbbdf
6687533
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
cache | ||
contracts | ||
artifacts | ||
.env | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here |
||
|
||
# zksync | ||
zk-artifacts | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ | |
"description": "Scripts for the management of the Unlock Protocol", | ||
"private": true, | ||
"dependencies": { | ||
"@arbitrum/nitro-contracts": "v1.0.2", | ||
"@arbitrum/sdk": "^v3.1.9", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use fixed version numbers (no |
||
"@matterlabs/hardhat-zksync-deploy": "1.1.2", | ||
"@matterlabs/hardhat-zksync-solc": "1.1.0", | ||
"@matterlabs/hardhat-zksync-upgradable": "1.2.4", | ||
|
@@ -21,8 +23,9 @@ | |
"@unlock-protocol/hardhat-helpers": "workspace:^", | ||
"@unlock-protocol/hardhat-plugin": "workspace:^", | ||
"@unlock-protocol/networks": "workspace:./packages/networks", | ||
"arb-shared-dependencies": "^1.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here! |
||
"eslint": "8.54.0", | ||
"ethers": "6.10.0", | ||
"ethers": "6.11.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that upgrade needed for the PR to work ? If not could you remove the change in version (better to have it in a separate PR as it may break other scripts on the repo). |
||
"fs-extra": "11.2.0", | ||
"hardhat": "2.20.1", | ||
"solhint": "4.1.1", | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,225 @@ | ||||||
const ethers = require('ethers') | ||||||
const { | ||||||
L1ToL2MessageGasEstimator, | ||||||
} = require('@arbitrum/sdk/dist/lib/message/L1ToL2MessageGasEstimator') | ||||||
const { arbLog } = require('arb-shared-dependencies') | ||||||
const { | ||||||
EthBridger, | ||||||
getL2Network, | ||||||
addDefaultLocalNetwork, | ||||||
} = require('@arbitrum/sdk') | ||||||
const { getBaseFee } = require('@arbitrum/sdk/dist/lib/utils/lib') | ||||||
|
||||||
const ERC20_ABI = [ | ||||||
{ | ||||||
inputs: [ | ||||||
{ | ||||||
internalType: 'address', | ||||||
name: 'account', | ||||||
type: 'address', | ||||||
}, | ||||||
], | ||||||
name: 'balanceOf', | ||||||
outputs: [ | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: '', | ||||||
type: 'uint256', | ||||||
}, | ||||||
], | ||||||
stateMutability: 'view', | ||||||
type: 'function', | ||||||
}, | ||||||
{ | ||||||
inputs: [ | ||||||
{ | ||||||
internalType: 'address', | ||||||
name: 'to', | ||||||
type: 'address', | ||||||
}, | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: 'amount', | ||||||
type: 'uint256', | ||||||
}, | ||||||
], | ||||||
name: 'transfer', | ||||||
outputs: [ | ||||||
{ | ||||||
internalType: 'bool', | ||||||
name: '', | ||||||
type: 'bool', | ||||||
}, | ||||||
], | ||||||
stateMutability: 'nonpayable', | ||||||
type: 'function', | ||||||
}, | ||||||
] | ||||||
clemsos marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const INBOX_ABI = [ | ||||||
{ | ||||||
inputs: [ | ||||||
{ | ||||||
internalType: 'address', | ||||||
name: 'to', | ||||||
type: 'address', | ||||||
}, | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: 'l2CallValue', | ||||||
type: 'uint256', | ||||||
}, | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: 'maxSubmissionCost', | ||||||
type: 'uint256', | ||||||
}, | ||||||
{ | ||||||
internalType: 'address', | ||||||
name: 'excessFeeRefundAddress', | ||||||
type: 'address', | ||||||
}, | ||||||
{ | ||||||
internalType: 'address', | ||||||
name: 'callValueRefundAddress', | ||||||
type: 'address', | ||||||
}, | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: 'gasLimit', | ||||||
type: 'uint256', | ||||||
}, | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: 'maxFeePerGas', | ||||||
type: 'uint256', | ||||||
}, | ||||||
{ | ||||||
internalType: 'bytes', | ||||||
name: 'data', | ||||||
type: 'bytes', | ||||||
}, | ||||||
], | ||||||
name: 'createRetryableTicket', | ||||||
outputs: [ | ||||||
{ | ||||||
internalType: 'uint256', | ||||||
name: '', | ||||||
type: 'uint256', | ||||||
}, | ||||||
], | ||||||
stateMutability: 'payable', | ||||||
type: 'function', | ||||||
}, | ||||||
] | ||||||
|
||||||
/** | ||||||
* Set up: instantiate L1 / L2 wallets connected to providers | ||||||
*/ | ||||||
const walletPrivateKey = 'Your_Private_key' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use env variables here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @clemsos earlier informed me .env is not supported, how else would I use env variables? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Process.env.PRIVATE_KEY for example |
||||||
const L1RPC = 'https://mainnet.infura.io/v3/<your_infura_api_key>' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it |
||||||
const L2RPC = 'https://arbitrum-mainnet.infura.io/v3/<your_infura_api_key>' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same, please use the unlock RPC |
||||||
const l1Provider = new ethers.JsonRpcProvider(L1RPC) | ||||||
const l2Provider = new ethers.JsonRpcProvider(L2RPC) | ||||||
// const l1Wallet = new ethers.Wallet(walletPrivateKey, l1Provider) | ||||||
const l2Wallet = new ethers.Wallet(walletPrivateKey, l2Provider) | ||||||
|
||||||
module.exports = async () => { | ||||||
await arbLog('Cross-chain Proposer') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It came with the arbitrum sdk, its more or less a branding/description of the script. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just use a regular |
||||||
addDefaultLocalNetwork() | ||||||
|
||||||
const l2Network = await getL2Network(l2Provider) | ||||||
const ethBridger = new EthBridger(l2Network) | ||||||
const inboxAddress = ethBridger.l2Network.ethBridge.inbox | ||||||
const ARBTokenAddressOnL2 = '0x912CE59144191C1204E64559FE8253a0e49E6548' // ARB TOKEN ADDRESS ON ARBITRUM ONE | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can yoiu please use the address (or add it there) in the |
||||||
const grantsContractAddress = '0x00D5E0d31d37cc13C645D86410aB4cB7Cb428ccA' | ||||||
const timelockL2Alias = '0x28ffDfB0A6e6E06E95B3A1f928Dc4024240bD87c' // Timelock Alias Address on L2 | ||||||
const L1TimelockContract = '0x17EEDFb0a6E6e06E95B3A1F928dc4024240BC76B' // Timelock Address mainnet | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably move that to a config file in this folder. |
||||||
|
||||||
const L2TokenContract = new ethers.Contract( | ||||||
ARBTokenAddressOnL2, | ||||||
ERC20_ABI, | ||||||
l2Wallet | ||||||
).connect(l2Wallet) | ||||||
|
||||||
const balanceOf = await L2TokenContract.balanceOf(timelockL2Alias) | ||||||
const tokenAmount = ethers.parseEther('1') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please let's use the real anount. It does not make sense to send 1 ARB if this has been tested correctly (as I am sure you have done by now!) Also, please use |
||||||
|
||||||
// Create an instance of the Interface from the ABIs | ||||||
const iface_erc20 = new ethers.Interface(ERC20_ABI) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please respect the styleguide if you can:
Suggested change
|
||||||
const iface_inbox = new ethers.Interface(INBOX_ABI) | ||||||
|
||||||
// Encode the ERC20 Token transfer calldata | ||||||
const transfer_calldata = iface_erc20.encodeFunctionData('transfer', [ | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
grantsContractAddress, | ||||||
tokenAmount, | ||||||
]) | ||||||
/** | ||||||
* Now we can query the required gas params using the estimateAll method in Arbitrum SDK | ||||||
*/ | ||||||
const l1ToL2MessageGasEstimate = new L1ToL2MessageGasEstimator(l2Provider) | ||||||
|
||||||
/** | ||||||
* The estimateAll method gives us the following values for sending an L1->L2 message | ||||||
* (1) maxSubmissionCost: The maximum cost to be paid for submitting the transaction | ||||||
* (2) gasLimit: The L2 gas limit | ||||||
* (3) deposit: The total amount to deposit on L1 to cover L2 gas and L2 call value | ||||||
*/ | ||||||
const L1ToL2MessageGasParams = await l1ToL2MessageGasEstimate.estimateAll( | ||||||
{ | ||||||
from: L1TimelockContract, | ||||||
to: ARBTokenAddressOnL2, | ||||||
l2CallValue: 0, | ||||||
excessFeeRefundAddress: l2Wallet.address, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's following the format from arbitrum's example, but I didn't need to change it because the L1 and L2 wallets are the same. Ideally you want to specify the wallet address you want the excess eth refund to be sent to if it's different. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is the excess ETH coming from? An funds should go back to the DAO and not to some external address. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, it has now been changed to the DAO's address. The reason for having it that way in the first place is due to the nature of the transaction. The |
||||||
callValueRefundAddress: l2Wallet.address, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same reason as before |
||||||
data: transfer_calldata, | ||||||
}, | ||||||
await getBaseFee(l1Provider), | ||||||
l1Provider | ||||||
) | ||||||
const gasPriceBid = await l2Provider.getGasPrice() | ||||||
|
||||||
const inbox_calldata = iface_inbox.encodeFunctionData( | ||||||
'createRetryableTicket', | ||||||
[ | ||||||
ARBTokenAddressOnL2, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add comments in front of each of these params to provide their names |
||||||
0, | ||||||
L1ToL2MessageGasParams.maxSubmissionCost, | ||||||
l2Wallet.address, | ||||||
l2Wallet.address, | ||||||
L1ToL2MessageGasParams.gasLimit, | ||||||
gasPriceBid, | ||||||
transfer_calldata, | ||||||
] | ||||||
) | ||||||
|
||||||
const proposalName = `# Test Transaction before 7k ARB Transfer To Fund Unlock Protocol’s Ecosystem via Grants Stack This proposal requests to use 1 ARB from the tokens given to Unlock Protocol DAO by ArbitrumDAO to run a test transaction to de-risk the transfer of 7k ARB tokens to fund the retroQF round on Grants Stack.` | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please format this correctly. This would be Test Transaction before 7k ARB Transfer To Fund Unlock Protocol’s Ecosystem via Grants Stack This proposal requests to use 1 ARB from the tokens given to Unlock Protocol DAO by ArbitrumDAO to run a test transaction to de-risk the transfer of 7k ARB tokens to fund the retroQF round on Grants Stack.Make sure there is a title, one (or more) distinct paragraphs, as well as links to provide clarity (links to the snapshot, to the UI on which people are voting... etc) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it |
||||||
// Proposal ARGS i.e Call Governor.propose() directly with these values | ||||||
const targets = [inboxAddress] | ||||||
const values = [L1ToL2MessageGasParams.deposit.toNumber() * 10] // I Multiply by 10 to add extra in case gas changes | ||||||
const calldatas = [inbox_calldata] | ||||||
const description = proposalName | ||||||
|
||||||
const calls = [ | ||||||
{ | ||||||
contractNameOrAbi: INBOX_ABI, | ||||||
contractAddress: inboxAddress, | ||||||
functionName: 'createRetryableTicket', | ||||||
functionArgs: [ | ||||||
ARBTokenAddressOnL2, | ||||||
0, | ||||||
L1ToL2MessageGasParams.maxSubmissionCost, | ||||||
l2Wallet.address, | ||||||
l2Wallet.address, | ||||||
L1ToL2MessageGasParams.gasLimit, | ||||||
gasPriceBid, | ||||||
transfer_calldata, | ||||||
], | ||||||
value: L1ToL2MessageGasParams.deposit.toNumber() * 10, // I Multiply by 10 to add extra in case gas changes due to proposal delay | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure you need to pass the value again here? It is already declared above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, from my understanding of the existing format, and correct me if I'm wrong but this value property is responsible for passing the eth amount to a payable function, and in this case that's the eth deposit to be sent to arbitrum's inbox contract. |
||||||
}, | ||||||
] | ||||||
|
||||||
return { | ||||||
proposalName, | ||||||
calls, | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remeove these changes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay got it