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

Client/StateManager: storageRangeAt() RPC call / EVMStateManager Interface Extension #2922

Merged
merged 7 commits into from
Aug 9, 2023
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
88 changes: 87 additions & 1 deletion packages/client/src/rpc/modules/debug.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { Address, TypeOutput, bigIntToHex, bytesToHex, hexToBytes, toType } from '@ethereumjs/util'
import {
Address,
TypeOutput,
bigIntToHex,
bytesToBigInt,
bytesToHex,
hexToBytes,
toType,
} from '@ethereumjs/util'

import { INTERNAL_ERROR, INVALID_PARAMS } from '../error-code'
import { getBlockByOption } from '../helpers'
Expand All @@ -8,6 +16,7 @@ import type { EthereumClient } from '../..'
import type { Chain } from '../../blockchain'
import type { FullEthereumService } from '../../service'
import type { RpcTx } from '../types'
import type { Block } from '@ethereumjs/block'
import type { VM } from '@ethereumjs/vm'

export interface tracerOpts {
Expand Down Expand Up @@ -92,6 +101,7 @@ export class Debug {
[validators.transaction()],
[validators.blockOption],
])
this.storageRangeAt = middleware(this.storageRangeAt.bind(this), 5, [[validators.hex]])
Copy link
Contributor

Choose a reason for hiding this comment

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

the validators here will only validate the first arg, reading from comments need 5 validators

}

/**
Expand Down Expand Up @@ -278,4 +288,80 @@ export class Debug {
message: err.message.toString(),
}
}

/**
* Returns a limited set of storage keys belonging to an account.
* @param params An array of 5 parameters:
* 1. The hash of the block that contains the requested account storage.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 1. The hash of the block that contains the requested account storage.
* 1. The hash of the block at which to get storage from the state.

just making it less ambiguous

* 2. The number index of the requested transaction within the block.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 2. The number index of the requested transaction within the block.
* 2. The transaction index of the requested block post which to get the storage.

* 3. The address of the account.
* 4. The smallest (hashed) storage key that will be returned. To include the entire range, pass '0x00'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 4. The smallest (hashed) storage key that will be returned. To include the entire range, pass '0x00'.
* 4. The starting (hashed) key from which storage will be returned. To include the entire range, pass '0x00'.

* 5. The maximum number of storage values that will be returned.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* 5. The maximum number of storage values that will be returned.
* 5. The maximum number of storage values that could be returned.

* @returns A {@link StorageRange} object that will contain at most `limit` entries in its `storage` field.
* The object will also contain `nextKey`, the next (hashed) storage key after the range included in `storage`.
*/
async storageRangeAt(params: [string, number, string, string, number]) {
Copy link
Contributor

@g11tech g11tech Jul 29, 2023

Choose a reason for hiding this comment

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

Suggested change
async storageRangeAt(params: [string, number, string, string, number]) {
async storageRangeAt(params: [string, string, string, string, string]) {

most like it will come as a hex encoded number ... not sure about that but do check and update/confirm

Copy link
Contributor

Choose a reason for hiding this comment

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

const [blockHash, txIndex, account, startKey, limit] = params

if (txIndex < 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we don't already have a validator for index parameters, it could be worth adding one. especially if there are other methods are doing this themselves

throw {
code: INTERNAL_ERROR,
message: 'txIndex cannot be smaller than 0.',
}
}

let address: Address
let blockHashBytes: Uint8Array
let startKeyBytes: Uint8Array
try {
address = Address.fromString(account)
blockHashBytes = hexToBytes(blockHash)
startKeyBytes = hexToBytes(startKey)
Copy link
Contributor

@g11tech g11tech Jul 29, 2023

Choose a reason for hiding this comment

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

startKey could be directly converted to bigint using a hexToBig... helper , and if limit is also coming as hex encoded it needs to be converted to number/bigint as well as per the requirement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There does not appear to exist a hexToBigInt() helper. I have however moved the function calls to the same line.

Copy link
Contributor

@ScottyPoi ScottyPoi Jul 31, 2023

Choose a reason for hiding this comment

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

if the hex is '0x' prefixed, you can just use the BigInt constructor like: BigInt('0xabcde')

} catch (err: any) {
throw {
code: INVALID_PARAMS,
message: err.message.toString(),
}
}

let block: Block
try {
block = await this.service.chain.getBlock(blockHashBytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
block = await this.service.chain.getBlock(blockHashBytes)
block = await this.chain.getBlock(blockHashBytes)

this.chain is available

} catch (err: any) {
throw {
code: INTERNAL_ERROR,
message: 'Could not get requested block hash.',
}
}

if (txIndex + 1 > block.transactions.length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (txIndex + 1 > block.transactions.length) {
if (txIndex >= block.transactions.length) {

more comprehensible about the check

throw {
code: INTERNAL_ERROR,
message: 'txIndex cannot be larger than the number of transactions in the block.',
}
}

try {
const block = await this.service.chain.getBlock(hexToBytes(blockHash))
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't the block already read above (in params check) also this.chain can be directly refereed

const parentBlock = await this.service.chain.getBlock(block.header.parentHash)

// Copy the VM and run transactions including the relevant transaction.
const vmCopy = await this.service.execution.vm.shallowCopy()
await vmCopy.stateManager.setStateRoot(parentBlock.header.stateRoot)
for (let i = 0; i <= txIndex; i++) {
await vmCopy.runTx({ tx: block.transactions[i], block })
}

return await vmCopy.stateManager.dumpStorageRange(
address,
bytesToBigInt(startKeyBytes),
limit
)
} catch (err: any) {
throw {
code: INTERNAL_ERROR,
message: err.message.toString(),
}
}
}
}
Loading