Skip to content

Fix new Nitro changes #98

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.

Sign up for GitHub

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

Closed
gzeoneth wants to merge 10 commits into from
Closed

Fix new Nitro changes #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8b3ded6
chore: move dependencies into dev dependencies (#70)
fredlacs Jun 20, 2022
47a4668
Merge branch 'c-nitro-stable' into c-nitro-next
yahgwai Jun 22, 2022
f0c8551
Merge branch 'c-nitro-stable' into c-nitro-next
yahgwai Jun 22, 2022
65e2825
fix: remove retryable from addr aliasing
gzeoneth Jun 24, 2022
766382b
fix: race condition causing isExpired to throw
gzeoneth Jun 24, 2022
5b19efc
fix: lint
gzeoneth Jun 24, 2022
fb4a469
fix: delayedInboxAccs
gzeoneth Jun 24, 2022
1ce22f9
chore: use newer nitro contracts
gzeoneth Jun 24, 2022
38ab2ab
fix: use Outbox.isSpent
gzeoneth Jun 24, 2022
e7c3ee2
Merge branch 'c-nitro-stable' into c-nitro-patch-1
fredlacs Jun 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,16 @@
"@ethersproject/address": "^5.0.8",
"@ethersproject/bignumber": "^5.1.1",
"@ethersproject/bytes": "^5.0.8",
"@typechain/ethers-v5": "9.0.0",
"@types/prompts": "^2.0.14",
"@types/yargs": "^17.0.9",
"dotenv": "^10.0.0",
"ethers": "^5.1.0",
"ts-node": "^10.2.1",
"typechain": "7.0.0"
"ethers": "^5.1.0"
},
"devDependencies": {
"@arbitrum/nitro-contracts": "1.0.0-beta.5",
"@arbitrum/nitro-contracts": "https://gitpkg.now.sh/OffchainLabs/nitro/contracts?expose-outbox-isspent",
Copy link
Contributor

@yahgwai yahgwai Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should release a version of the nitro contracts and include that here

Copy link
Member Author

@gzeoneth gzeoneth Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OffchainLabs/nitro#723

"@nomiclabs/hardhat-ethers": "^2.0.4",
"@typechain/ethers-v5": "9.0.0",
"@types/chai": "^4.2.11",
"@types/mocha": "^9.0.0",
"@types/prompts": "^2.0.14",
"@types/yargs": "^17.0.9",
"@typescript-eslint/eslint-plugin": "^5.14.0",
"@typescript-eslint/eslint-plugin-tslint": "^5.27.1",
"@typescript-eslint/parser": "^5.14.0",
Expand All @@ -70,6 +66,7 @@
"axios": "^0.21.3",
"chai": "^4.2.0",
"chalk": "^4.1.0",
"dotenv": "^10.0.0",
"eslint": "^7.32.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-mocha": "^9.0.0",
Expand All @@ -80,7 +77,9 @@
"prettier": "^2.3.2",
"prettier-plugin-solidity": "^1.0.0-beta.17",
"prompts": "^2.4.2",
"ts-node": "^10.2.1",
"tslint": "^6.1.3",
"typechain": "7.0.0",
"typedoc": "^0.21.5",
"typedoc-plugin-markdown": "^3.10.4",
"typescript": "^4.2.2",
Expand Down
44 changes: 28 additions & 16 deletions scripts/instantiate_bridge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,25 @@ export const instantiateBridge = (
throw new Error('need ARB_KEY var')
}

let networkID = args.networkID
if (!networkID) {
let l2NetworkID = args.networkID
if (!l2NetworkID) {
console.log(
'No networkID command line arg provided; using network',
defaultNetworkId
)

networkID = defaultNetworkId
l2NetworkID = defaultNetworkId
}
const isL1 = isDefined(l1Networks[networkID])
const isL2 = isDefined(l2Networks[networkID])
const isL1 = isDefined(l1Networks[l2NetworkID])
const isL2 = isDefined(l2Networks[l2NetworkID])
if (!isL1 && !isL2) {
throw new Error(`Unrecognized network ID: ${networkID}`)
throw new Error(`Unrecognized network ID: ${l2NetworkID}`)
}
if (!isL2) {
throw new Error(`Tests must specify an L2 network ID: ${networkID}`)
throw new Error(`Tests must specify an L2 network ID: ${l2NetworkID}`)
}

const l2Network = l2Networks[networkID]
const l2Network = l2Networks[l2NetworkID]
const l1Network = l1Networks[l2Network.partnerChainID]

if (!l1Network) {
Expand All @@ -86,14 +86,26 @@ export const instantiateBridge = (
)
}

if (!l1Network.rpcURL) {
throw new Error('L1 rpc url not set (see .env.sample or networks.ts)')
}
if (!l2Network.rpcURL) {
throw new Error('L2 rpc url not set (see .env.sample or utils/networks.ts)')
}
const ethProvider = new JsonRpcProvider(l1Network.rpcURL)
const arbProvider = new JsonRpcProvider(l2Network.rpcURL)
const l1Rpc = (() => {
if (l2NetworkID === 42161) return process.env['MAINNET_RPC'] as string
if (l2NetworkID === 421611) return process.env['RINKEBY_RPC'] as string
if (l2NetworkID === 1338) return 'http://127.0.0.1:8545/'
throw new Error(
'L1 rpc url not set (see .env.sample or networks.ts) or chain id not supported'
)
})()
const l2Rpc = (() => {
if (l2NetworkID === 42161)
return process.env['ARB_ONE_RPC'] || 'https://arb1.arbitrum.io/rpc'
if (l2NetworkID === 421611)
return process.env['RINKARBY_RPC'] || 'https://rinkeby.arbitrum.io/rpc'
throw new Error(
'L2 rpc url not set (see .env.sample or networks.ts) or chain id not supported'
)
})()

const ethProvider = new JsonRpcProvider(l1Rpc)
const arbProvider = new JsonRpcProvider(l2Rpc)

const l1Signer = (() => {
if (l1PkParam) {
Expand Down
2 changes: 0 additions & 2 deletions scripts/testSetup.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ export const getCustomNetworks = async (
isCustom: true,
name: 'EthLocal',
partnerChainIDs: [l2NetworkInfo.chainId],
rpcURL: l1Url,
}

const l2Network: Omit<L2Network, 'tokenBridge'> = {
Expand All @@ -106,7 +105,6 @@ export const getCustomNetworks = async (
isCustom: true,
name: 'ArbLocal',
partnerChainID: l1NetworkInfo.chainId,
rpcURL: l2Url,
retryableLifetimeSeconds: 7 * 24 * 60 * 60,
}
return {
Expand Down
9 changes: 0 additions & 9 deletions src/lib/dataEntities/networks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@
/* eslint-env node */
'use strict'

import dotenv from 'dotenv'
import { SignerOrProvider, SignerProviderUtils } from './signerOrProvider'
import { ArbSdkError } from '../dataEntities/errors'
import { SEVEN_DAYS_IN_SECONDS } from './constants'

dotenv.config()

export interface L1Network extends Network {
partnerChainIDs: number[]
blockTime: number //seconds
Expand All @@ -40,7 +37,6 @@ export interface Network {
chainID: number
name: string
explorerUrl: string
rpcURL: string
gif?: string
isCustom: boolean
}
Expand Down Expand Up @@ -135,7 +131,6 @@ export const l1Networks: L1Networks = {
explorerUrl: 'https://etherscan.io',
partnerChainIDs: [42161],
blockTime: 14,
rpcURL: process.env['MAINNET_RPC'] as string,
isCustom: false,
},
1338: {
Expand All @@ -144,7 +139,6 @@ export const l1Networks: L1Networks = {
explorerUrl: 'https://etherscan.io',
partnerChainIDs: [42161],
blockTime: 1,
rpcURL: 'http://127.0.0.1:8545/',
isCustom: false,
},
4: {
Expand All @@ -153,7 +147,6 @@ export const l1Networks: L1Networks = {
explorerUrl: 'https://rinkeby.etherscan.io',
partnerChainIDs: [421611],
blockTime: 15,
rpcURL: process.env['RINKEBY_RPC'] as string,
isCustom: false,
},
}
Expand All @@ -168,7 +161,6 @@ export const l2Networks: L2Networks = {
tokenBridge: mainnetTokenBridge,
ethBridge: mainnetETHBridge,
confirmPeriodBlocks: 45818,
rpcURL: process.env['ARB_ONE_RPC'] || 'https://arb1.arbitrum.io/rpc',
isCustom: false,
retryableLifetimeSeconds: SEVEN_DAYS_IN_SECONDS,
},
Expand All @@ -181,7 +173,6 @@ export const l2Networks: L2Networks = {
tokenBridge: rinkebyTokenBridge,
ethBridge: rinkebyETHBridge,
confirmPeriodBlocks: 6545, // TODO
rpcURL: process.env['RINKARBY_RPC'] || 'https://rinkeby.arbitrum.io/rpc',
isCustom: false,
retryableLifetimeSeconds: SEVEN_DAYS_IN_SECONDS,
},
Expand Down
4 changes: 3 additions & 1 deletion src/lib/inbox/inbox.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ export class InboxTools {
return null
}

const delayedAcc = await bridge.inboxAccs(eventInfo.event.messageIndex)
const delayedAcc = await bridge.delayedInboxAccs(
eventInfo.event.messageIndex
)

return { ...eventInfo, delayedAcc: delayedAcc }
}
Expand Down
20 changes: 15 additions & 5 deletions src/lib/message/L1ToL2Message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,13 @@ export abstract class L1ToL2Message {
return ethers.utils.stripZeros(value.toHexString())
}

const addressAlias = new Address(fromAddress)

const from = addressAlias.applyAlias()
Comment on lines -130 to -132
Copy link
Contributor

@fredlacs fredlacs Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure we don't need to apply an alias here anymore?
If the user is supplying the from address as their L1 address it still needs to be aliased.

The difference is that now the alias is applied int he L1 inbox instead of by arb-os.
We should clarify what's expected in the fromAddress field. If its the value from the Inbox delivered event, it doesn't need aliasing. If its the plain user address, it needs alaising

Copy link
Member Author

@gzeoneth gzeoneth Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, calculateSubmitRetryableId is public static
I think we should just add some comment regarding the input expected.

const chainId = BigNumber.from(l2ChainId)
const msgNum = BigNumber.from(messageNumber)

const fields: any[] = [
formatNumber(chainId),
zeroPad(formatNumber(msgNum), 32),
from.value,
fromAddress,
formatNumber(l1BaseFee),

formatNumber(l1Value),
Expand Down Expand Up @@ -433,7 +430,20 @@ export class L1ToL2MessageReader extends L1ToL2Message {
}

// not redeemed, has it now expired
if (await this.isExpired()) {
try {
if (await this.isExpired()) {
Copy link
Contributor

@yahgwai yahgwai Jun 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make isExpired atomic with getSuccessfulRedeem so that we always have consistent results for getSuccessfulRedeem.

We could adjust isExpired so that it has a try/catch internally, and rename it to retryableExists.

Then adjust getSuccessfulRedeem so that we call retryableExists somewhere at the start of it. If retryableExists is true then we can just return null straight away - since it hasnt been redeemed or expired. If retryableExists is false then we either successfully redeemed or we expired. We then look for a successful redeem, if we find it we return the tx receipt, if we dont we return some value that indicates an expiration.

Then we won't check isExpired/retryableExists in receiptToStatus, instead we just pass in the receipt, or the value that indicates expiration.

return L1ToL2MessageStatus.EXPIRED
}
} catch (error) {
// this can happen due to a race condition that
// the retryable is redeemed/expired in between the calls
const successfulRedeemReceiptRetry = await this.getSuccessfulRedeem()
if (
successfulRedeemReceiptRetry &&
successfulRedeemReceiptRetry.status === 1
) {
return L1ToL2MessageStatus.REDEEMED
}
return L1ToL2MessageStatus.EXPIRED
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/message/L2ToL1Message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export class L2ToL1MessageReader extends L2ToL1Message {
this.l1Provider
)

return outbox.callStatic.spent(this.event.position)
return outbox.callStatic.isSpent(this.event.position)
}

/**
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
"@openzeppelin/contracts-upgradeable" "4.5.2"
hardhat "^2.6.6"

"@arbitrum/nitro-contracts@https://gitpkg.now.sh/OffchainLabs/nitro/contracts?expose-outbox-isspent":
version "1.0.0-beta.5"
resolved "https://gitpkg.now.sh/OffchainLabs/nitro/contracts?expose-outbox-isspent#47069a151c59a26efad04764c9c56e33af22bfe1"
dependencies:
"@openzeppelin/contracts" "4.5.0"
"@openzeppelin/contracts-upgradeable" "4.5.2"
hardhat "^2.6.6"

"@babel/code-frame@7.12.11":
version "7.12.11"
resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.12.11.tgz#f4ad435aa263db935b8f10f2c552d23fb716a63f"
Expand Down