Skip to content

Commit

Permalink
Remove "unsafe" from depositTransaction (#118)
Browse files Browse the repository at this point in the history
* Remove "unsafe" from depositTransaction

* changeset
  • Loading branch information
wilsoncusack committed Sep 21, 2023
1 parent 1bc857d commit 86f8263
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 44 deletions.
5 changes: 5 additions & 0 deletions .changeset/eighty-weeks-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"op-viem": patch
---

writeUnsafeDepositTransaction -> writeDepositTransaction
4 changes: 2 additions & 2 deletions site/.vitepress/config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ export default defineConfig({
link: '/docs/actions/wallet/L1/writeSendMessage',
},
{
text: 'writeUnsafeDepositTransaction',
link: '/docs/actions/wallet/L1/writeUnsafeDepositTransaction',
text: 'writeDepositTransaction',
link: '/docs/actions/wallet/L1/writeDepositTransaction',
},
],
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
# writeUnsafeDepositTransaction
# writeDepositTransaction

Excutes a [depositTransaction](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L1/OptimismPortal.sol#L374) call to the [`OptimismPortal`](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L1/OptimismPortal.sol) contract.

::: danger
Unlike [writeSendMessage](docs/actions/wallet/L1/writeSendMessage), using this call does not offer replayability on L2 in the case the L2 tx fails. But this call has the advantage that, if the caller is an EOA, msg.sender of the L2 tx will be the caller address. Allowing users to fully tranasact on L2 from L1, which is a critical security property.

Interacting directly the portal offers no replayability. For example, if you are sending ETH and your L2 transaction fails––because the gas limit is too low or something else goes wrong––your ETH will be in the `OptimismPortal` on L1 but you'll have nothing on L2: i.e. your ETH will be stuck indefinitely. You can read more about replays here and deposit transactions [here](https://community.optimism.io/docs/protocol/deposit-flow/#replaying-messages).

:::
If the caller is not an EOA, e.g. if the caller is a smart contract wallet, msg.sender on L2 will be [alias](https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L1/OptimismPortal.sol#L407) of the caller address.

::: warning

Expand Down Expand Up @@ -42,7 +40,7 @@ const gas = await l2PublicClient.estimateGas({

args.gasLimit = gas

const hash = await opStackL1WalletClient.writeUnsafeDepositTransaction({
const hash = await opStackL1WalletClient.writeDepositTransaction({
args,
l2Chain: base,
value: 1n,
Expand Down Expand Up @@ -110,7 +108,7 @@ A [Transaction Hash](https://viem.sh/docs/glossary/terms#hash).
- The calldata of the L2 transaction

```ts
await walletClient.writeUnsafeDepositTransaction({
await walletClient.writeDepositTransaction({
args: { // [!code focus:7]
to: account.address,
value: 1n,
Expand All @@ -129,7 +127,7 @@ await walletClient.writeUnsafeDepositTransaction({
The destination L2 chain of the deposit transaction. `l2Chain.opStackConfig.l1.chainId` must match `chain.id` (from `client.chain` or `chain` passed explicitly as an arg). The address at `l2Chain.opStackConfig.l1.contracts.optimismPortal.address` will be used for the contract call. If this is argument not passed or if no such contract definition exists, [optimismPortalAddress](#optimismPortalAddress) must be passed explicitly.

```ts
await walletClient.writeUnsafeDepositTransaction({
await walletClient.writeDepositTransaction({
args,
l2Chain: base, // [!code focus:1]
})
Expand All @@ -142,7 +140,7 @@ await walletClient.writeUnsafeDepositTransaction({
The `OptimismPortal` contract where the depositTransaction call should be made.

```ts
await walletClient.writeUnsafeDepositTransaction({
await walletClient.writeDepositTransaction({
args,
optimismPortalAddress: portal, // [!code focus:1]
})
Expand All @@ -155,7 +153,7 @@ await walletClient.writeUnsafeDepositTransaction({
Value in wei sent with this transaction. This value will be credited to the balance of the caller address on L2 _before_ the L2 transaction created by this transaction is made.

```ts
await walletClient.writeUnsafeDepositTransaction({
await walletClient.writeDepositTransaction({
args,
optimismPortalAddress: portal,
value: parseEther(1), // [!code focus:1]
Expand Down
2 changes: 1 addition & 1 deletion site/docs/actions/wallet/L1/writeSendMessage.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ await walletClient.writeSendMessage({
Value in wei sent with this transaction. This value will be credited to the balance of the caller address on L2 _before_ the L2 transaction created by this transaction is made.

```ts
await walletClient.writeUnsafeDepositTransaction({
await walletClient.writeDepositTransaction({
args,
optimismPortalAddress: portal,
value: parseEther(1), // [!code focus:1]
Expand Down
4 changes: 2 additions & 2 deletions src/_test/live.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { privateKeyToAccount } from 'viem/accounts'
import { estimateGas } from 'viem/actions'
import { goerli } from 'viem/chains'
import { test } from 'vitest'
import type { DepositTransactionParameters } from '../actions/wallet/L1/writeUnsafeDepositTransaction.js'
import type { DepositTransactionParameters } from '../actions/wallet/L1/writeDepositTransaction.js'
import { baseGoerli } from '../chains/baseGoerli.js'
import { publicL1OpStackActions } from '../decorators/publicL1OpStackActions.js'
import { walletL1OpStackActions } from '../decorators/walletL1OpStackActions.js'
Expand Down Expand Up @@ -45,7 +45,7 @@ test('correctly retrieves L2 hash', async () => {

args.gasLimit = gas

const depositHash = await walletClient.writeUnsafeDepositTransaction({
const depositHash = await walletClient.writeDepositTransaction({
l2Chain: baseGoerli,
args,
value: 1n,
Expand Down
10 changes: 5 additions & 5 deletions src/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ export {
} from './public/L2/simulateWithdrawETH.js'
export { writeDepositERC20, type WriteDepositERC20Parameters } from './wallet/L1/writeDepositERC20.js'
export { writeDepositETH, type WriteDepositETHParameters } from './wallet/L1/writeDepositETH.js'
export {
type DepositTransactionParameters,
writeDepositTransaction,
type WriteDepositTransactionParameters,
} from './wallet/L1/writeDepositTransaction.js'
export {
writeFinalizeWithdrawalTranasction,
type WriteFinalizeWithdrawalTransactionParameters,
Expand All @@ -73,11 +78,6 @@ export {
writeSendMessage,
type WriteSendMessageParameters,
} from './wallet/L1/writeSendMessage.js'
export {
type DepositTransactionParameters,
writeUnsafeDepositTransaction,
type WriteUnsafeDepositTransactionParameters,
} from './wallet/L1/writeUnsafeDepositTransaction.js'
export { writeOpStackL2, type WriteOpStackL2Parameters } from './wallet/L2/writeOpStackL2.js'
export { writeWithdrawERC20, type WriteWithdrawERC20Parameters } from './wallet/L2/writeWithdrawERC20.js'
export { writeWithdrawETH, type WriteWithdrawETHParameters } from './wallet/L2/writeWithdrawETH.js'
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import { publicClient, rollupPublicClient, rollupWalletClient, testClient, walle
import { base } from '../../../chains/index.js'
import { type TransactionDepositedEvent } from '../../../types/depositTransaction.js'
import type { OpStackChain } from '../../../types/opStackChain.js'
import { type DepositTransactionParameters, writeUnsafeDepositTransaction } from './writeUnsafeDepositTransaction.js'
import { type DepositTransactionParameters, writeDepositTransaction } from './writeDepositTransaction.js'

test('default', async () => {
expect(
await writeUnsafeDepositTransaction(walletClient, {
await writeDepositTransaction(walletClient, {
args: {
to: '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb',
value: 1n,
Expand Down Expand Up @@ -44,7 +44,7 @@ test('sends transaction to correct infered address', async () => {

args.gasLimit = gas

const hash = await writeUnsafeDepositTransaction(walletClient, {
const hash = await writeDepositTransaction(walletClient, {
args,
value: 1n,
l2Chain: base,
Expand All @@ -61,7 +61,7 @@ test('sends transaction to correct infered address', async () => {

test('sends transaction to correct explicit address', async () => {
const portal: Address = '0xbEb5Fc579115071764c7423A4f12eDde41f106Ed'
const hash = await writeUnsafeDepositTransaction(walletClient, {
const hash = await writeDepositTransaction(walletClient, {
args: {
to: '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb',
value: 1n,
Expand All @@ -86,7 +86,7 @@ test('creates correct deposit transaction', async () => {
data: '0x',
isCreation: false,
}
const hash = await writeUnsafeDepositTransaction(walletClient, {
const hash = await writeDepositTransaction(walletClient, {
args,
value: args.value!,
l2Chain: base,
Expand Down Expand Up @@ -122,7 +122,7 @@ test('correctly passes arugments', async () => {
isCreation: false,
}

const hash = await writeUnsafeDepositTransaction(walletClient, {
const hash = await writeDepositTransaction(walletClient, {
args,
l2Chain: base,
account: accounts[0].address,
Expand All @@ -148,7 +148,7 @@ test('uses defaults for data, isCreation, and value', async () => {
gasLimit: 25000n,
}

const hash = await writeUnsafeDepositTransaction(walletClient, {
const hash = await writeDepositTransaction(walletClient, {
args,
l2Chain: base,
account: accounts[0].address,
Expand All @@ -170,7 +170,7 @@ test('uses defaults for data, isCreation, and value', async () => {
test('errors if l2Chain and optimismPortalAddress both not passed', async () => {
expect(() =>
// @ts-expect-error
writeUnsafeDepositTransaction(walletClient, {
writeDepositTransaction(walletClient, {
args: {
to: '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb',
gasLimit: 25000n,
Expand All @@ -193,7 +193,7 @@ test('errors if chain.id does not match l1.chainId', async () => {
} as const satisfies OpStackChain

expect(() =>
writeUnsafeDepositTransaction(walletClient, {
writeDepositTransaction(walletClient, {
args: {
to: '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb',
gasLimit: 25000n,
Expand All @@ -218,7 +218,7 @@ test('works if override chain id matches l1.id', async () => {
} as const satisfies OpStackChain

expect(
await writeUnsafeDepositTransaction(walletClient, {
await writeDepositTransaction(walletClient, {
args: {
to: '0x0c54fccd2e384b4bb6f2e405bf5cbc15a017aafb',
gasLimit: 25000n,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type DepositTransactionParameters = {
data?: Hex
}

export type WriteUnsafeDepositTransactionParameters<
export type WriteDepositTransactionParameters<
TChain extends Chain | undefined = Chain,
TAccount extends Account | undefined = Account | undefined,
TChainOverride extends Chain | undefined = Chain | undefined,
Expand All @@ -32,14 +32,21 @@ export type WriteUnsafeDepositTransactionParameters<
>

/**
* Calls depositTransaction directly on the OptimismPortal contract.
* Marked 'unsafe' becaused does not offer replayability incase the
* L2 tx fails.
* Calls depositTransaction on the OptimismPortal contract.
*
* @param parameters - {@link WriteUnsafeDepositTransactionParameters}
* Unlike writeSendMessage, does not offer replayability on L2 incase the L2 tx fails.
* But has the advantage that, if the caller is an EOA, msg.sender of the L2 tx
* will be the caller address. Allowing users to fully tranasact on L2 from L1, which
* is a critical security property.
*
* If the caller is not an EOA, e.g. if the caller is a smart contract wallet,
* msg.sender on L2 will be alias of the caller address
* https://github.com/ethereum-optimism/optimism/blob/develop/packages/contracts-bedrock/src/L1/OptimismPortal.sol#L407
*
* @param parameters - {@link WriteDepositTransactionParameters}
* @returns A [Transaction Hash](https://viem.sh/docs/glossary/terms.html#hash). {@link WriteContractReturnType}
*/
export async function writeUnsafeDepositTransaction<
export async function writeDepositTransaction<
TChain extends Chain | undefined,
TAccount extends Account | undefined,
TChainOverride extends Chain | undefined = undefined,
Expand All @@ -49,7 +56,7 @@ export async function writeUnsafeDepositTransaction<
args: { to, value = 0n, gasLimit, isCreation = false, data = '0x' },
optimismPortalAddress,
...rest
}: WriteUnsafeDepositTransactionParameters<
}: WriteDepositTransactionParameters<
TChain,
TAccount,
TChainOverride
Expand Down
14 changes: 7 additions & 7 deletions src/decorators/walletL1OpStackActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import type { Account, Chain, Transport, WriteContractReturnType } from 'viem'
import type { WalletClient } from 'viem'
import { writeDepositERC20, type WriteDepositERC20Parameters } from '../actions/wallet/L1/writeDepositERC20.js'
import { writeDepositETH, type WriteDepositETHParameters } from '../actions/wallet/L1/writeDepositETH.js'
import {
writeDepositTransaction,
type WriteDepositTransactionParameters,
} from '../actions/wallet/L1/writeDepositTransaction.js'
import {
writeFinalizeWithdrawalTranasction,
type WriteFinalizeWithdrawalTransactionParameters,
Expand All @@ -10,10 +14,6 @@ import {
writeProveWithdrawalTransaction,
type WriteProveWithdrawalTransactionParameters,
} from '../actions/wallet/L1/writeProveWithdrawalTransaction.js'
import {
writeUnsafeDepositTransaction,
type WriteUnsafeDepositTransactionParameters,
} from '../actions/wallet/L1/writeUnsafeDepositTransaction.js'

export type WalletL1OpStackActions<
TChain extends Chain | undefined = Chain | undefined,
Expand All @@ -29,10 +29,10 @@ export type WalletL1OpStackActions<
>(
args: WriteDepositERC20Parameters<TChain, TAccount, TChainOverride>,
) => Promise<WriteContractReturnType>
writeUnsafeDepositTransaction: <
writeDepositTransaction: <
TChainOverride extends Chain | undefined = Chain | undefined,
>(
args: WriteUnsafeDepositTransactionParameters<
args: WriteDepositTransactionParameters<
TChain,
TAccount,
TChainOverride
Expand Down Expand Up @@ -66,7 +66,7 @@ export function walletL1OpStackActions<
client: WalletClient<TTransport, TChain, TAccount>,
): WalletL1OpStackActions<TChain, TAccount> {
return {
writeUnsafeDepositTransaction: (args) => writeUnsafeDepositTransaction(client, args),
writeDepositTransaction: (args) => writeDepositTransaction(client, args),
writeDepositETH: (args) => writeDepositETH(client, args),
writeDepositERC20: (args) => writeDepositERC20(client, args),
writeProveWithdrawalTransaction: (args) => writeProveWithdrawalTransaction(client, args),
Expand Down

0 comments on commit 86f8263

Please sign in to comment.