feat[contracts]: Add value transfer support to ECDSAContractAccount

.changeset/fuzzy-gorillas-accept.md

@@ -0,0 +1,7 @@
+---
+"@eth-optimism/integration-tests": patch
+"@eth-optimism/l2geth": patch
+"@eth-optimism/contracts": patch
+---
+
+Add value transfer support to ECDSAContractAccount

integration-tests/test/fee-payment.spec.ts

@@ -25,13 +25,4 @@ describe('Fee Payment Integration Tests', async () => {
       tx.gasPrice.mul(tx.gasLimit).add(amount)
     )
   })
-
-  it('sequencer rejects transaction with a non-multiple-of-1M gasPrice', async () => {
-    const gasPrice = BigNumber.from(1_000_000 - 1)
-    await expect(
-      env.ovmEth.transfer(other, 0, { gasPrice })
-    ).to.be.eventually.rejectedWith(
-      'Gas price must be a multiple of 1,000,000 wei'
-    )
-  })
 })

integration-tests/test/rpc.spec.ts

@@ -1,5 +1,5 @@
 import { injectL2Context } from '@eth-optimism/core-utils'
-import { Wallet, BigNumber } from 'ethers'
+import { Wallet, BigNumber, ethers } from 'ethers'
 import chai, { expect } from 'chai'
 import { sleep, l2Provider, GWEI } from './shared/utils'
 import chaiAsPromised from 'chai-as-promised'
@@ -59,16 +59,35 @@ describe('Basic RPC tests', () => {
       ).to.be.rejectedWith('Cannot submit unprotected transaction')
     })
 
-    it('should not accept a transaction with a value', async () => {
+    it('should accept a transaction with a value', async () => {
       const tx = {
         ...DEFAULT_TRANSACTION,
-        chainId: await wallet.getChainId(),
-        value: 100,
+        chainId: await env.l2Wallet.getChainId(),
+        data: '0x',
+        value: ethers.utils.parseEther('5'),
       }
-      await expect(
-        provider.sendTransaction(await wallet.signTransaction(tx))
-      ).to.be.rejectedWith(
-        'Cannot send transaction with non-zero value. Use WETH.transfer()'
+
+      const balanceBefore = await provider.getBalance(env.l2Wallet.address)
+      const result = await env.l2Wallet.sendTransaction(tx)
+      const receipt = await result.wait()
+      expect(receipt.status).to.deep.equal(1)
+
+      expect(await provider.getBalance(env.l2Wallet.address)).to.deep.equal(
+        balanceBefore.sub(ethers.utils.parseEther('5'))
+      )
+    })
+
+    it('should reject a transaction with higher value than user balance', async () => {
+      const balance = await env.l2Wallet.getBalance()
+      const tx = {
+        ...DEFAULT_TRANSACTION,
+        chainId: await env.l2Wallet.getChainId(),
+        data: '0x',
+        value: balance.add(ethers.utils.parseEther('1')),
+      }
+
+      await expect(env.l2Wallet.sendTransaction(tx)).to.be.rejectedWith(
+        'invalid transaction: insufficient funds for gas * price + value'
       )
     })
   })

l2geth/core/state_transition.go

@@ -252,7 +252,7 @@ func (st *StateTransition) TransitionDb() (ret []byte, usedGas uint64, failed bo
 			l1MessageSender = msg.L1MessageSender().Hex()
 		}
 		if st.evm.EthCallSender == nil {
-			log.Debug("Applying transaction", "ID", st.evm.Id, "from", sender.Address().Hex(), "to", to, "nonce", msg.Nonce(), "gasPrice", msg.GasPrice().Uint64(), "gasLimit", msg.Gas(), "l1MessageSender", l1MessageSender, "data", hexutil.Encode(msg.Data()))
+			log.Debug("Applying transaction", "ID", st.evm.Id, "from", sender.Address().Hex(), "to", to, "nonce", msg.Nonce(), "gasPrice", msg.GasPrice().Uint64(), "gasLimit", msg.Gas(), "value", msg.Value().Uint64(), "l1MessageSender", l1MessageSender, "data", hexutil.Encode(msg.Data()))
 		}
 	}
 

l2geth/core/state_transition_ovm.go

@@ -148,7 +148,7 @@ func modMessage(
 		from,
 		to,
 		msg.Nonce(),
-		msg.Value(),
+		common.Big0,
 		gasLimit,
 		msg.GasPrice(),
 		data,

l2geth/eth/api_backend.go

@@ -297,11 +297,6 @@ func (b *EthAPIBackend) SubscribeLogsEvent(ch chan<- []*types.Log) event.Subscri
 // a lock can be used around the remotes for when the sequencer is reorganizing.
 func (b *EthAPIBackend) SendTx(ctx context.Context, signedTx *types.Transaction) error {
 	if b.UsingOVM {
-		// The value field is not rolled up so it must be set to 0
-		if signedTx.Value().Cmp(new(big.Int)) != 0 {
-			return fmt.Errorf("Cannot send transaction with non-zero value. Use WETH.transfer()")
-		}
-
 		to := signedTx.To()
 		if to != nil {
 			if *to == (common.Address{}) {

packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol

@@ -102,6 +102,12 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
 
         // Contract creations are signalled by sending a transaction to the zero address.
         if (transaction.isCreate) {
+            // TEMPORARY: Disable value transfer for contract creations.
+            require(
+                transaction.value == 0,
+                "Value transfer in contract creation not supported."
+            );
+
             (address created, bytes memory revertdata) = Lib_ExecutionManagerWrapper.ovmCREATE(
                 transaction.data
             );
@@ -118,7 +124,25 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
             // cases, but since this is a contract we'd end up bumping the nonce twice.
             Lib_ExecutionManagerWrapper.ovmINCREMENTNONCE();
 
-            return transaction.to.call(transaction.data);
+            // Value transfer currently only supported for CALL but not for CREATE.
+            if (transaction.value > 0) {
+                // TEMPORARY: Block value transfer if the transaction has input data.
+                require(
+                    transaction.data.length == 0,
+                    "Value is nonzero but input data was provided."
+                );
+
+                require(
+                    ovmETH.transfer(
+                        transaction.to,
+                        transaction.value
+                    ),
+                    "Value could not be transferred to recipient."
+                );
+                return (true, bytes(""));
+            } else {
+                return transaction.to.call(transaction.data);
+            }
         }
     }
 }

packages/contracts/contracts/optimistic-ethereum/OVM/predeploys/OVM_SequencerEntrypoint.sol

@@ -11,9 +11,6 @@ import { Lib_ExecutionManagerWrapper } from "../../libraries/wrappers/Lib_Execut
  * @dev The Sequencer Entrypoint is a predeploy which, despite its name, can in fact be called by
  * any account. It accepts a more efficient compressed calldata format, which it decompresses and
  * encodes to the standard EIP155 transaction format.
- * This contract is the implementation referenced by the Proxy Sequencer Entrypoint, thus enabling
- * the Optimism team to upgrade the decompression of calldata from the Sequencer.
- *
  * Compiler used: optimistic-solc
  * Runtime target: OVM
  */

packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts

@@ -12,7 +12,11 @@ import {
   DEFAULT_EIP155_TX,
   decodeSolidityError,
 } from '../../../helpers'
-import { getContractFactory, predeploys } from '../../../../src'
+import {
+  getContractFactory,
+  getContractInterface,
+  predeploys,
+} from '../../../../src'
 
 const callPredeploy = async (
   Helper_PredeployCaller: Contract,
@@ -37,6 +41,8 @@ const callPredeploy = async (
   )
 }
 
+const iOVM_ETH = getContractInterface('OVM_ETH')
+
 describe('OVM_ECDSAContractAccount', () => {
   let wallet: Wallet
   let badWallet: Wallet
@@ -262,5 +268,109 @@ describe('OVM_ECDSAContractAccount', () => {
         'Fee was not transferred to relayer.'
       )
     })
+
+    it(`should transfer value if value is greater than 0`, async () => {
+      const transaction = { ...DEFAULT_EIP155_TX, value: 1234, data: '0x' }
+      const encodedTransaction = await wallet.signTransaction(transaction)
+
+      await callPredeploy(
+        Helper_PredeployCaller,
+        OVM_ECDSAContractAccount,
+        'execute',
+        [encodedTransaction],
+        40000000
+      )
+
+      // First call transfers fee, second transfers value (since value > 0).
+      const ovmCALL: any = Mock__OVM_ExecutionManager.smocked.ovmCALL.calls[1]
+      expect(ovmCALL._address).to.equal(predeploys.OVM_ETH)
+      expect(ovmCALL._calldata).to.equal(
+        iOVM_ETH.encodeFunctionData('transfer', [
+          transaction.to,
+          transaction.value,
+        ])
+      )
+    })
+
+    it(`should revert if the value is not transferred to the recipient`, async () => {
+      const transaction = { ...DEFAULT_EIP155_TX, value: 1234, data: '0x' }
+      const encodedTransaction = await wallet.signTransaction(transaction)
+
+      Mock__OVM_ExecutionManager.smocked.ovmCALL.will.return.with(
+        (gasLimit, target, data) => {
+          if (target === predeploys.OVM_ETH) {
+            const [recipient, amount] = iOVM_ETH.decodeFunctionData(
+              'transfer',
+              data
+            )
+            if (recipient === transaction.to) {
+              return [
+                true,
+                '0x0000000000000000000000000000000000000000000000000000000000000000',
+              ]
+            } else {
+              return [
+                true,
+                '0x0000000000000000000000000000000000000000000000000000000000000001',
+              ]
+            }
+          } else {
+            return [true, '0x']
+          }
+        }
+      )
+
+      await callPredeploy(
+        Helper_PredeployCaller,
+        OVM_ECDSAContractAccount,
+        'execute',
+        [encodedTransaction],
+        40000000
+      )
+
+      const ovmREVERT: any =
+        Mock__OVM_ExecutionManager.smocked.ovmREVERT.calls[0]
+      expect(decodeSolidityError(ovmREVERT._data)).to.equal(
+        'Value could not be transferred to recipient.'
+      )
+    })
+
+    it(`should revert if trying to send value with a contract creation`, async () => {
+      const transaction = { ...DEFAULT_EIP155_TX, value: 1234, to: '' }
+      const encodedTransaction = await wallet.signTransaction(transaction)
+
+      await callPredeploy(
+        Helper_PredeployCaller,
+        OVM_ECDSAContractAccount,
+        'execute',
+        [encodedTransaction],
+        40000000
+      )
+
+      const ovmREVERT: any =
+        Mock__OVM_ExecutionManager.smocked.ovmREVERT.calls[0]
+      expect(decodeSolidityError(ovmREVERT._data)).to.equal(
+        'Value transfer in contract creation not supported.'
+      )
+    })
+
+    it(`should revert if trying to send value with non-empty transaction data`, async () => {
+      const transaction = { ...DEFAULT_EIP155_TX, value: 1234, to: '' }
+      const encodedTransaction = await wallet.signTransaction(transaction)
+
+      await callPredeploy(
+        Helper_PredeployCaller,
+        OVM_ECDSAContractAccount,
+        'execute',
+        [encodedTransaction],
+        40000000
+      )
+
+      const ovmREVERT: any =
+        Mock__OVM_ExecutionManager.smocked.ovmREVERT.calls[0]
+      expect(decodeSolidityError(ovmREVERT._data)).to.equal(
+        'Value transfer in contract creation not supported.'
+      )
+    })
   })
 })