diff --git a/.changeset/fuzzy-gorillas-accept.md b/.changeset/fuzzy-gorillas-accept.md new file mode 100644 index 0000000000000..3bede02b3aaf2 --- /dev/null +++ b/.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 diff --git a/integration-tests/test/fee-payment.spec.ts b/integration-tests/test/fee-payment.spec.ts index 2005c57513c29..5b94edb1afd55 100644 --- a/integration-tests/test/fee-payment.spec.ts +++ b/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' - ) - }) }) diff --git a/integration-tests/test/rpc.spec.ts b/integration-tests/test/rpc.spec.ts index 5dc2ef92423ad..00eef9794a693 100644 --- a/integration-tests/test/rpc.spec.ts +++ b/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' ) }) }) diff --git a/l2geth/core/state_transition.go b/l2geth/core/state_transition.go index d0106680f52c1..d5e839a757dcf 100644 --- a/l2geth/core/state_transition.go +++ b/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())) } } diff --git a/l2geth/core/state_transition_ovm.go b/l2geth/core/state_transition_ovm.go index 0f7b698d6b57a..adba0fb257dfd 100644 --- a/l2geth/core/state_transition_ovm.go +++ b/l2geth/core/state_transition_ovm.go @@ -148,7 +148,7 @@ func modMessage( from, to, msg.Nonce(), - msg.Value(), + common.Big0, gasLimit, msg.GasPrice(), data, diff --git a/l2geth/eth/api_backend.go b/l2geth/eth/api_backend.go index 45b63575df8e7..704581ed5f1f9 100644 --- a/l2geth/eth/api_backend.go +++ b/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{}) { diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol index ad7c21d223a01..74816818683d4 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol +++ b/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); + } } } } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/predeploys/OVM_SequencerEntrypoint.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/predeploys/OVM_SequencerEntrypoint.sol index 0f40b1893a1ec..0be33bd2b98a0 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/predeploys/OVM_SequencerEntrypoint.sol +++ b/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 */ diff --git a/packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts b/packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts index d2bc886aea407..c3a09d32c4ffa 100644 --- a/packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts +++ b/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.' + ) + }) }) })