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

Add eth_getTransactionByHash endpoint #90

Merged
merged 5 commits into from
Apr 17, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 7 additions & 1 deletion packages/ovm/src/app/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,12 @@ export const getOvmTransactionMetadata = (
* Converts an EVM receipt to an OVM receipt.
*
* @param internalTxReceipt The EVM tx receipt to convert to an OVM tx receipt
* @param ovmTxHash The OVM tx hash to replace the internal tx hash with.
* @returns The converted receipt
*/
export const internalTxReceiptToOvmTxReceipt = async (
internalTxReceipt: TransactionReceipt
internalTxReceipt: TransactionReceipt,
ovmTxHash?: string
): Promise<OvmTransactionReceipt> => {
const ovmTransactionMetadata = getOvmTransactionMetadata(internalTxReceipt)
// Construct a new receipt
Expand All @@ -187,6 +189,10 @@ export const internalTxReceiptToOvmTxReceipt = async (

ovmTxReceipt.status = ovmTransactionMetadata.ovmTxSucceeded ? 1 : 0

if (!!ovmTxReceipt.transactionHash && !!ovmTxHash) {
ovmTxReceipt.transactionHash = ovmTxHash
}

if (ovmTransactionMetadata.revertMessage !== undefined) {
ovmTxReceipt.revertMessage = ovmTransactionMetadata.revertMessage
}
Expand Down
41 changes: 39 additions & 2 deletions packages/rollup-full-node/src/app/web3-rpc-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ export class DefaultWeb3Handler
args = this.assertParameters(params, 1)
response = await this.getLogs([0])
break
case Web3RpcMethods.getTransactionByHash:
args = this.assertParameters(params, 1)
response = await this.getTransactionByHash(args[0])
break
case Web3RpcMethods.getTransactionCount:
args = this.assertParameters(params, 2, latestBlock)
response = await this.getTransactionCount(args[0], args[1])
Expand Down Expand Up @@ -423,6 +427,35 @@ export class DefaultWeb3Handler
return res
}

public async getTransactionByHash(ovmTxHash: string): Promise<any> {
log.debug('Getting tx for ovm tx hash:', ovmTxHash)
// First convert our ovmTxHash into an internalTxHash
const internalTxHash = await this.getInternalTxHash(ovmTxHash)

if (!internalTxHash) {
log.debug(
`There is no EVM tx hash associated with OVM tx hash [${ovmTxHash}]`
)
return null
}

log.debug(
`OVM tx hash [${ovmTxHash}] is associated with EVM tx hash [${internalTxHash}]`
)

const internalTx = await this.context.provider.send(
Web3RpcMethods.getTransactionByHash,
[internalTxHash]
)

log.debug(
`OVM tx hash [${ovmTxHash}] is for EVM tx: [${JSON.stringify(
internalTx
)}]`
)
return internalTx
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately my guess is that just returning the internal tx is probably not going to be sufficient for many use cases of this endpoint. For example, if a dev 1. manually constructs an OVM transaction, only storing the hash 2. sends it off to the chain, and then 3. attempts to retrieve it later with this endpoint, it will not get back the original transaction constructed in step 1.

}

public async getTransactionCount(
address: Address,
defaultBlock: string
Expand Down Expand Up @@ -464,7 +497,8 @@ export class DefaultWeb3Handler

// Now let's parse the internal transaction reciept
const ovmTxReceipt: OvmTransactionReceipt = await internalTxReceiptToOvmTxReceipt(
internalTxReceipt
internalTxReceipt,
ovmTxHash
)
if (ovmTxReceipt.revertMessage !== undefined && !includeRevertMessage) {
delete ovmTxReceipt.revertMessage
Expand Down Expand Up @@ -848,7 +882,10 @@ export class DefaultWeb3Handler
return []
}
} else if (params.length === expected - 1 || params.length === expected) {
return params.length === expected ? params : [...params, defaultLast]
const nonEmptyParams = params.filter((x) => !!x)
return nonEmptyParams.length === expected
? nonEmptyParams
: [...nonEmptyParams, defaultLast]
}
throw new InvalidParametersError(
`Expected ${expected} parameters but received ${
Expand Down
2 changes: 2 additions & 0 deletions packages/rollup-full-node/src/types/web3-rpc-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface Web3Handler {
getCode(address: Address, defaultBlock: string): Promise<string>
getExecutionManagerAddress()
getLogs(filter: any): Promise<any[]>
getTransactionByHash(transactionHash: string): Promise<any>
getTransactionCount(address: Address, defaultBlock: string): Promise<string>
getTransactionReceipt(txHash: string): Promise<string>
networkVersion(): Promise<string>
Expand All @@ -37,6 +38,7 @@ export enum Web3RpcMethods {
getCode = 'eth_getCode',
getExecutionManagerAddress = 'ovm_getExecutionManagerAddress',
getLogs = 'eth_getLogs',
getTransactionByHash = 'eth_getTransactionByHash',
getTransactionCount = 'eth_getTransactionCount',
getTransactionReceipt = 'eth_getTransactionReceipt',
networkVersion = 'net_version',
Expand Down
57 changes: 49 additions & 8 deletions packages/rollup-full-node/test/app/web-rpc-handler.spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
import '../setup'
/* External Imports */
import { getLogger, numberToHexString } from '@eth-optimism/core-utils'
import {
add0x,
getLogger,
keccak256,
numberToHexString,
} from '@eth-optimism/core-utils'
import { ethers, ContractFactory, Wallet, Contract } from 'ethers'
import { resolve } from 'path'
import * as rimraf from 'rimraf'
import * as fs from 'fs'
import assert from 'assert'

/* Internal Imports */
import {
FullnodeRpcServer,
DefaultWeb3Handler,
TestWeb3Handler,
} from '../../src/app'
import { FullnodeRpcServer, DefaultWeb3Handler } from '../../src/app'
import * as SimpleStorage from '../contracts/build/untranspiled/SimpleStorage.json'
import { Web3RpcMethods } from '../../src/types'

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

Expand Down Expand Up @@ -68,14 +71,14 @@ const setStorage = async (
simpleStorage: Contract,
httpProvider,
executionManagerAddress
): Promise<void> => {
): Promise<any> => {
// Set storage with our new storage elements
const tx = await simpleStorage.setStorage(
executionManagerAddress,
storageKey,
storageValue
)
await httpProvider.getTransactionReceipt(tx.hash)
return httpProvider.getTransactionReceipt(tx.hash)
}

const getAndVerifyStorage = async (
Expand Down Expand Up @@ -210,6 +213,44 @@ describe('Web3Handler', () => {
})
})

describe('the eth_getTransactionByHash endpoint', () => {
it('should return null if no tx exists', async () => {
const garbageHash = add0x(
keccak256(Buffer.from('garbage').toString('hex'))
)
const txByHash = await httpProvider.send(
Web3RpcMethods.getTransactionByHash,
[garbageHash]
)

assert(
txByHash === null,
'Should not have gotten a tx for garbage hash!'
)
})

it.only('should return a tx by OVM hash', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.only

const executionManagerAddress = await httpProvider.send(
'ovm_getExecutionManagerAddress',
[]
)
const wallet = getWallet(httpProvider)
const simpleStorage = await deploySimpleStorage(wallet)
const txReceipt = await setStorage(
simpleStorage,
httpProvider,
executionManagerAddress
)

const txByHash = await httpProvider.send(
Web3RpcMethods.getTransactionByHash,
[txReceipt.transactionHash]
)

assert(!!txByHash, 'Transaction should exist!')
})
})

describe('SimpleStorage integration test', () => {
it('should set storage & retrieve the value', async () => {
const executionManagerAddress = await httpProvider.send(
Expand Down