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

YAS 292: fix test provider's eth_sendTransaction behavior #103

Merged
merged 6 commits into from
Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
9 changes: 7 additions & 2 deletions packages/core-utils/src/app/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,16 +291,21 @@ export const getCurrentTime = (): number => {
return Math.round(Date.now() / 1000)
}
/**
* Encodes a transaction in RLP format
* Encodes a transaction in RLP format, using a random signature
* @param {object} Transaction object
*/
export const rlpEncodeTransaction = (transaction: object): string => {
export const rlpEncodeTransactionWithRandomSig = (
transaction: object
): string => {
return RLP.encode([
hexlify(transaction['nonce']),
hexlify(transaction['gasPrice']),
hexlify(transaction['gasLimit']),
hexlify(transaction['to']),
hexlify(transaction['value']),
transaction['data'],
'0x01', // v
'0x' + '01'.repeat(32), // r
'0x' + '01'.repeat(32), // s
])
}
26 changes: 22 additions & 4 deletions packages/rollup-full-node/src/app/test-web3-rpc-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import {
getLogger,
numberToHexString,
castToNumber,
rlpEncodeTransaction,
rlpEncodeTransactionWithRandomSig,
ZERO_ADDRESS,
} from '@eth-optimism/core-utils'
import { GAS_LIMIT } from '@eth-optimism/ovm'
import { JsonRpcProvider, Web3Provider } from 'ethers/providers'
import { utils } from 'ethers'

/* Internal Imports */
import { initializeL2Node } from './index'
import { initializeL2Node, latestBlock } from './index'
import { DefaultWeb3Handler } from './web3-rpc-handler'
import {
L2NodeContext,
Expand Down Expand Up @@ -132,8 +134,24 @@ export class TestWeb3Handler extends DefaultWeb3Handler {
*
* @param The transaction to send
*/
public async sendTransaction(ovmTx: object): Promise<string> {
return this.sendRawTransaction(rlpEncodeTransaction(ovmTx))
public async sendTransaction(ovmTx: any): Promise<string> {
if (!ovmTx.nonce) {
ovmTx.nonce = await this.getTransactionCount(ovmTx.from, latestBlock)
}
if (!ovmTx.to) {
ovmTx.to = ZERO_ADDRESS
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be slightly cleaner to set this to null instead? Like I actually don't know what clients normally do when decoding / encoding Ethereum transactions, but the way contract creations normally work is we end up decoding the tx and get a to: null in the result, I believe. I want to know exactly how Ethereum encodes EOA contract creation though....

Though after thinking about it, this approach does actually work so could also keep it. Maybe it would be good to have a test which does a contract creation too? My only thought is if we ever move away from using the weird to=ZERO_ADDRESS to signify a contract creation this could fail silently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

did this because the execution manager checks for isCreate with target===ZERO_ADDRESS, but will def add a test

}
if (!ovmTx.gasPrice) {
ovmTx.gasPrice = 0
}
if (!ovmTx.gasLimit) {
ovmTx.gasLimit = GAS_LIMIT
}
ovmTx.value = 0
return this.sendRawTransaction(
rlpEncodeTransactionWithRandomSig(ovmTx),
ovmTx.from
)
}

/**
Expand Down
14 changes: 10 additions & 4 deletions packages/rollup-full-node/src/app/web3-rpc-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ import { NoOpL2ToL1MessageSubmitter } from './message-submitter'

const log = getLogger('web3-handler')

const latestBlock: string = 'latest'
export const latestBlock: string = 'latest'

export class DefaultWeb3Handler
implements Web3Handler, FullnodeHandler, L1ToL2TransactionListener {
Expand Down Expand Up @@ -541,15 +541,22 @@ export class DefaultWeb3Handler
return response
}

public async sendRawTransaction(rawOvmTx: string): Promise<string> {
public async sendRawTransaction(
rawOvmTx: string,
fromAddressOverride?: string
): Promise<string> {
const debugTime = new Date().getTime()
const blockTimestamp = this.getTimestamp()
log.debug('Sending raw transaction with params:', rawOvmTx)

// Decode the OVM transaction -- this will be used to construct our internal transaction
const ovmTx = utils.parseTransaction(rawOvmTx)
// override the from address if in testing mode
if (!!fromAddressOverride) {
ovmTx.from = fromAddressOverride
}
log.debug(
`OVM Transaction being parsed ${rawOvmTx}, parsed: ${JSON.stringify(
`OVM Transaction being parsed ${rawOvmTx}, with from address override of [${fromAddressOverride}], parsed: ${JSON.stringify(
ovmTx
)}`
)
Expand Down Expand Up @@ -840,7 +847,6 @@ export class DefaultWeb3Handler
)
throw new Error('Non-EOA transaction detected')
}
// TODO: Make sure we lock this function with this nonce so we don't send to txs with the same nonce
// Generate the calldata which we'll use to call our internal execution manager
// First pull out the `to` field (we just need to check if it's null & if so set ovmTo to the zero address as that's how we deploy contracts)
const ovmTo = ovmTx.to === null ? ZERO_ADDRESS : ovmTx.to
Expand Down
33 changes: 25 additions & 8 deletions packages/rollup-full-node/test/app/test-web-rpc-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
} from '../../src'
import * as SimpleStorage from '../contracts/build/untranspiled/SimpleStorage.json'
import * as EmptyContract from '../contracts/build/untranspiled/EmptyContract.json'
import * as CallerStorer from '../contracts/build/transpiled/CallerStorer.json'
import { getOvmTransactionMetadata } from '@eth-optimism/ovm'

const log = getLogger('test-web3-handler', true)

Expand Down Expand Up @@ -225,7 +227,7 @@ describe('TestHandler', () => {
await testRpcServer.close()
})

it('should run the transaction', async () => {
it('should run the transaction for arbitrary from, to, and data, correctly filling optional fields including nonce', async () => {
const storageKey = add0x('01'.repeat(32))
const storageValue = add0x('02'.repeat(32))
const executionManagerAddress = await httpProvider.send(
Expand All @@ -242,23 +244,38 @@ describe('TestHandler', () => {
'setStorage'
].encode([executionManagerAddress, storageKey, storageValue])
const transaction = {
nonce: 0,
from: wallet.address,
to: simpleStorage.address,
gasPrice: 0,
gasLimit: 0,
value: 0,
data: transactionData,
}

const response = await httpProvider.send('eth_sendTransaction', [
transaction,
])
await httpProvider.send('eth_sendTransaction', [transaction])
const res = await simpleStorage.getStorage(
executionManagerAddress,
storageKey
)
res.should.equal(storageValue)
})

it('the EVM should actually see the arbitrary .from as the sender of the tx', async () => {
const factory = new ContractFactory(
CallerStorer.abi,
CallerStorer.bytecode,
wallet
)
const callerStorer = await factory.deploy()
const setData = await callerStorer.interface.functions[
'storeMsgSender'
].encode([])
const randomFromAddress = add0x('02'.repeat(20))
const transaction = {
from: randomFromAddress,
to: callerStorer.address,
data: setData,
}
await httpProvider.send('eth_sendTransaction', [transaction])
const res = await callerStorer.getLastMsgSender()
res.should.equal(randomFromAddress)
})
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pragma solidity ^0.5.0;

contract CallerStorer {
address public lastMsgSender;
function storeMsgSender() public {
lastMsgSender = msg.sender;
}
function getLastMsgSender() public view returns(address) {
return lastMsgSender;
}
}