Skip to content
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

feat: add sanity checks in bridge sdk transfer function #1959

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Prev Previous commit
Next Next commit
update
  • Loading branch information
fionnachan committed Oct 18, 2024
commit e9effa6cfa97eb7b4e199a09a713992d2bc4f18a
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
validateSignerChainId
} from './utils'
import { depositTokenEstimateGas } from '../util/TokenDepositUtils'
import { addressIsSmartContract } from '../util/AddressUtils'

// https://github.com/OffchainLabs/arbitrum-sdk/blob/main/src/lib/message/L1ToL2MessageGasEstimator.ts#L33
export const DEFAULT_GAS_PRICE_PERCENT_INCREASE = BigNumber.from(500)
Expand Down Expand Up @@ -321,6 +322,12 @@ export class Erc20DepositStarter extends BridgeTransferStarter {

const depositToAddress = depositRequest.txRequest.to.toLowerCase()

if (!addressIsSmartContract(depositToAddress, this.sourceChainProvider)) {
throw new Error(
`Parent chain token gateway router address provided is not a smart contract address.`
)
}

const parentGatewayRouterAddressForChain =
getArbitrumNetwork(
destinationChainId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
validateSignerChainId
} from './utils'
import { getL2ConfigForTeleport } from './teleport'
import { addressIsSmartContract } from '../util/AddressUtils'

export class Erc20TeleportStarter extends BridgeTransferStarter {
public transferType: TransferType = 'erc20_teleport'
Expand Down Expand Up @@ -200,6 +201,12 @@ export class Erc20TeleportStarter extends BridgeTransferStarter {

const depositToAddress = depositRequest.txRequest.to.toLowerCase()

if (!addressIsSmartContract(depositToAddress, this.sourceChainProvider)) {
throw new Error(
`Teleporter transfer address provided is not a smart contract address.`
)
}

const l1TeleporterAddress =
l1l3Bridger.l2Network.teleporter?.l1Teleporter.toLowerCase()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ export class Erc20WithdrawalStarter extends BridgeTransferStarter {

const withdrawToAddress = request.txRequest.to.toLowerCase()

if (!addressIsSmartContract(withdrawToAddress, this.sourceChainProvider)) {
throw new Error(
`Child chain token gateway router address provided is not a smart contract address.`
)
}

const childGatewayRouterAddressForChain =
getArbitrumNetwork(
sourceChainId
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { depositEthEstimateGas } from '../util/EthDepositUtils'
import { fetchErc20Allowance } from '../util/TokenUtils'
import { isExperimentalFeatureEnabled } from '../util'
import { isCustomDestinationAddressTx } from '../state/app/utils'
import { addressIsSmartContract } from '../util/AddressUtils'

export class EthDepositStarter extends BridgeTransferStarter {
public transferType: TransferType = 'eth_deposit'
Expand Down Expand Up @@ -151,6 +152,10 @@ export class EthDepositStarter extends BridgeTransferStarter {

const depositToAddress = depositRequest.txRequest.to.toLowerCase()

if (!addressIsSmartContract(depositToAddress, this.sourceChainProvider)) {
throw new Error(`Inbox address provided is not a smart contract address.`)
}

const inboxAddressForChain =
getArbitrumNetwork(destinationChainId).ethBridge.inbox.toLowerCase()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
percentIncrease,
validateSignerChainId
} from './utils'
import { addressIsSmartContract } from '../util/AddressUtils'

export class EthTeleportStarter extends BridgeTransferStarter {
public transferType: TransferType = 'eth_teleport'
Expand Down Expand Up @@ -131,6 +132,10 @@ export class EthTeleportStarter extends BridgeTransferStarter {

const depositToAddress = depositRequest.txRequest.to.toLowerCase()

if (!addressIsSmartContract(depositToAddress, this.sourceChainProvider)) {
throw new Error(`Inbox address provided is not a smart contract address.`)
}

const inboxAddressForChain =
l1l3Bridger.l2Network.ethBridge.inbox.toLowerCase()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from './utils'
import { withdrawInitTxEstimateGas } from '../util/WithdrawalUtils'
import { isExperimentalFeatureEnabled } from '../util'
import { addressIsSmartContract } from '../util/AddressUtils'

export class EthWithdrawalStarter extends BridgeTransferStarter {
public transferType: TransferType = 'eth_withdrawal'
Expand Down Expand Up @@ -77,6 +78,12 @@ export class EthWithdrawalStarter extends BridgeTransferStarter {

const withdrawToAddress = request.txRequest.to

if (!addressIsSmartContract(withdrawToAddress, this.sourceChainProvider)) {
throw new Error(
`Native currency withdrawal request address is not a smart contract address.`
)
}

if (withdrawToAddress.toLowerCase() !== ARB_SYS_ADDRESS.toLowerCase()) {
throw new Error(
`Native currency withdrawal request address must be the ArbSys address ${ARB_SYS_ADDRESS} instead of ${withdrawToAddress}.`
Expand Down
2 changes: 1 addition & 1 deletion packages/arb-token-bridge-ui/src/token-bridge-sdk/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export async function validateSignerChainId({

if (signerChainId !== sourceChainId) {
throw new Error(
`Signer is on chain ${signerChainId} but should be on chain ${sourceChainId}.`
`Signer is on chain ${signerChainId} but should be on chain ${sourceChainId}. Please try again after connecting to the correct chain in your wallet.`
)
}
}
13 changes: 11 additions & 2 deletions packages/arb-token-bridge-ui/src/util/AddressUtils.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
import { Provider } from '@ethersproject/providers'

import { getAPIBaseUrl } from '.'
import { getProviderForChainId } from '../token-bridge-sdk/utils'

export type Address = `0x${string}`

export async function addressIsSmartContract(address: string, chainId: number) {
const provider = getProviderForChainId(chainId)
export async function addressIsSmartContract(
address: string,
chainIdOrProvider: number | Provider
) {
const provider =
typeof chainIdOrProvider === 'number'
? getProviderForChainId(chainIdOrProvider)
: chainIdOrProvider

try {
return (await provider.getCode(address)).length > 2
} catch (_) {
Expand Down