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.

Sign up for GitHub

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

vm: remove isTruthy and isFalsy #2248

Merged
merged 2 commits into from
Aug 30, 2022
Merged

vm: remove isTruthy and isFalsy #2248

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28490fb
vm: remove isTruthy and isFalsy
gabrocheleau Aug 30, 2022
5e51a02
Merge branch 'master' into vm/remove-isTruthy-isFalsy-2
gabrocheleau Aug 30, 2022
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: 3 additions & 5 deletions packages/vm/src/bloom/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isTruthy, zeros } from '@ethereumjs/util'
import { zeros } from '@ethereumjs/util'
import { keccak256 } from 'ethereum-cryptography/keccak'

const BYTE_SIZE = 256
Expand Down Expand Up @@ -67,10 +67,8 @@ export class Bloom {
* Bitwise or blooms together.
*/
or(bloom: Bloom) {
if (isTruthy(bloom)) {
for (let i = 0; i <= BYTE_SIZE; i++) {
this.bitvector[i] = this.bitvector[i] | bloom.bitvector[i]
}
for (let i = 0; i <= BYTE_SIZE; i++) {
this.bitvector[i] = this.bitvector[i] | bloom.bitvector[i]
}
}
}
9 changes: 5 additions & 4 deletions packages/vm/src/buildBlock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Block } from '@ethereumjs/block'
import { ConsensusType } from '@ethereumjs/common'
import { RLP } from '@ethereumjs/rlp'
import { Trie } from '@ethereumjs/trie'
import { Address, TypeOutput, isTruthy, toBuffer, toType } from '@ethereumjs/util'
import { Address, TypeOutput, toBuffer, toType } from '@ethereumjs/util'

import { Bloom } from './bloom'
import { calculateMinerReward, encodeReceipt, rewardAccount } from './runBlock'
Expand Down Expand Up @@ -104,9 +104,10 @@ export class BlockBuilder {
private async rewardMiner() {
const minerReward = this.vm._common.param('pow', 'minerReward')
const reward = calculateMinerReward(minerReward, 0)
const coinbase = isTruthy(this.headerData.coinbase)
? new Address(toBuffer(this.headerData.coinbase))
: Address.zero()
const coinbase =
this.headerData.coinbase !== undefined
? new Address(toBuffer(this.headerData.coinbase))
: Address.zero()
await rewardAccount(this.vm.eei, coinbase, reward)
}

Expand Down
10 changes: 5 additions & 5 deletions packages/vm/src/eei/vmState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Chain, Common, Hardfork } from '@ethereumjs/common'
import { ripemdPrecompileAddress } from '@ethereumjs/evm/dist/precompiles'
import { Account, Address, isTruthy, toBuffer } from '@ethereumjs/util'
import { Account, Address, toBuffer } from '@ethereumjs/util'
import { debug as createDebugLogger } from 'debug'

import type { EVMStateAccess } from '@ethereumjs/evm/dist/types'
Expand Down Expand Up @@ -210,12 +210,12 @@ export class VmState implements EVMStateAccess {
* Merges a storage map into the last item of the accessed storage stack
*/
private _accessedStorageMerge(
storageList: Map<string, Set<string>>[],
storageList: Map<string, Set<string> | undefined>[],
storageMap: Map<string, Set<string>>
) {
const mapTarget = storageList[storageList.length - 1]

if (isTruthy(mapTarget)) {
if (mapTarget !== undefined) {
// Note: storageMap is always defined here per definition (TypeScript cannot infer this)
for (const [addressString, slotSet] of storageMap) {
const addressExists = mapTarget.get(addressString)
Expand Down Expand Up @@ -255,10 +255,10 @@ export class VmState implements EVMStateAccess {
const [balance, code, storage] = state
const account = Account.fromAccountData({ balance })
await this.putAccount(addr, account)
if (isTruthy(code)) {
if (code !== undefined) {
await this.putContractCode(addr, toBuffer(code))
}
if (isTruthy(storage)) {
if (storage !== undefined) {
for (const [key, value] of storage) {
await this.putContractStorage(addr, toBuffer(key), toBuffer(value))
}
Expand Down
14 changes: 3 additions & 11 deletions packages/vm/src/runBlock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,7 @@ import { Block } from '@ethereumjs/block'
import { ConsensusType, Hardfork } from '@ethereumjs/common'
import { RLP } from '@ethereumjs/rlp'
import { Trie } from '@ethereumjs/trie'
import {
Account,
Address,
bigIntToBuffer,
bufArrToArr,
intToBuffer,
isTruthy,
short,
} from '@ethereumjs/util'
import { Account, Address, bigIntToBuffer, bufArrToArr, intToBuffer, short } from '@ethereumjs/util'
import { debug as createDebugLogger } from 'debug'

import { Bloom } from './bloom'
Expand Down Expand Up @@ -53,8 +45,8 @@ export async function runBlock(this: VM, opts: RunBlockOpts): Promise<RunBlockRe

if (
this._hardforkByBlockNumber ||
isTruthy(this._hardforkByTTD) ||
isTruthy(opts.hardforkByTTD)
this._hardforkByTTD !== undefined ||
Copy link
Member

Choose a reason for hiding this comment

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

In vm.ts this field is marked as optional, but it looks to me that it is always being set? I think we can improve here by making that field non-optional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried making this.hardforkByTTD non-optional (which ends up removing that if altogether) but ended up with a couple failing API tests. I've created an issue for it in case we want to investigate that further (#2260)

Copy link
Member

@holgerd77 holgerd77 Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

No no, just to add here: there are a lot of development use cases where this is not wanted (that HFs align with the block numbers in regard to a certain chain configuration). So these hardforkBy* options should always be present and available to set.

Copy link
Member

Choose a reason for hiding this comment

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

The type of hardforkByTTD is undefined | bigint, and with optional I meant that it should be of type bigint (so it should never be undefined), but this ruins the logic of setting the hardforks (I realized this when checking #2260). I'm not sure what you mean "should always be available to set", if you want to set the hardforkByBlockNumber from false to true or vice-versa you have to either hack it in, or you have to create a new VM since it is readonly.

Copy link
Member

Choose a reason for hiding this comment

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

Plainly meant this should remain an option (in the case this would be in question). 🙂

opts.hardforkByTTD !== undefined
) {
this._common.setHardforkByBlockNumber(
block.header.number,
Expand Down
7 changes: 1 addition & 6 deletions packages/vm/src/runTx.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Block } from '@ethereumjs/block'
import { ConsensusType, Hardfork } from '@ethereumjs/common'
import { Capability } from '@ethereumjs/tx'
import { Address, KECCAK256_NULL, isFalsy, short, toBuffer } from '@ethereumjs/util'
import { Address, KECCAK256_NULL, short, toBuffer } from '@ethereumjs/util'
import { debug as createDebugLogger } from 'debug'

import { Bloom } from './bloom'
Expand Down Expand Up @@ -30,11 +30,6 @@ const debugGas = createDebugLogger('vm:tx:gas')
* @ignore
*/
export async function runTx(this: VM, opts: RunTxOpts): Promise<RunTxResult> {
// tx is required
if (isFalsy(opts.tx)) {
throw new Error('invalid input, tx is required')
}

// create a reasonable default if no block is given
opts.block = opts.block ?? Block.fromBlockData({}, { common: opts.tx.common })

Expand Down
4 changes: 2 additions & 2 deletions packages/vm/src/vm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Blockchain } from '@ethereumjs/blockchain'
import { Chain, Common } from '@ethereumjs/common'
import { EVM, getActivePrecompiles } from '@ethereumjs/evm'
import { DefaultStateManager } from '@ethereumjs/statemanager'
import { Account, Address, TypeOutput, isTruthy, toType } from '@ethereumjs/util'
import { Account, Address, TypeOutput, toType } from '@ethereumjs/util'
import AsyncEventEmitter = require('async-eventemitter')
import { promisify } from 'util'

Expand Down Expand Up @@ -255,7 +255,7 @@ export class VM {
common: (eeiCopy as any)._common,
evm: evmCopy,
hardforkByBlockNumber: this._hardforkByBlockNumber ? true : undefined,
hardforkByTTD: isTruthy(this._hardforkByTTD) ? this._hardforkByTTD : undefined,
hardforkByTTD: this._hardforkByTTD,
})
}

Expand Down
34 changes: 7 additions & 27 deletions packages/vm/tests/api/EIPs/eip-3074-authcall.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
bigIntToBuffer,
bufferToBigInt,
ecsign,
isTruthy,
privateToAddress,
setLengthLeft,
toBuffer,
Expand Down Expand Up @@ -170,33 +169,14 @@ function MSTORE(position: Buffer, value: Buffer) {
* @returns - The bytecode to execute AUTHCALL
*/
function getAuthCallCode(data: AuthcallData) {
const ZEROS32 = zeros(32)
const gasLimitBuffer = setLengthLeft(
isTruthy(data.gasLimit) ? bigIntToBuffer(data.gasLimit) : ZEROS32,
32
)
const gasLimitBuffer = setLengthLeft(bigIntToBuffer(data.gasLimit ?? BigInt(0)), 32)
const addressBuffer = setLengthLeft(data.address.buf, 32)
const valueBuffer = setLengthLeft(isTruthy(data.value) ? bigIntToBuffer(data.value) : ZEROS32, 32)
const valueExtBuffer = setLengthLeft(
isTruthy(data.valueExt) ? bigIntToBuffer(data.valueExt) : ZEROS32,
32
)
const argsOffsetBuffer = setLengthLeft(
isTruthy(data.argsOffset) ? bigIntToBuffer(data.argsOffset) : ZEROS32,
32
)
const argsLengthBuffer = setLengthLeft(
isTruthy(data.argsLength) ? bigIntToBuffer(data.argsLength) : ZEROS32,
32
)
const retOffsetBuffer = setLengthLeft(
isTruthy(data.retOffset) ? bigIntToBuffer(data.retOffset) : ZEROS32,
32
)
const retLengthBuffer = setLengthLeft(
isTruthy(data.retLength) ? bigIntToBuffer(data.retLength) : ZEROS32,
32
)
const valueBuffer = setLengthLeft(bigIntToBuffer(data.value ?? BigInt(0)), 32)
const valueExtBuffer = setLengthLeft(bigIntToBuffer(data.valueExt ?? BigInt(0)), 32)
const argsOffsetBuffer = setLengthLeft(bigIntToBuffer(data.argsOffset ?? BigInt(0)), 32)
const argsLengthBuffer = setLengthLeft(bigIntToBuffer(data.argsLength ?? BigInt(0)), 32)
const retOffsetBuffer = setLengthLeft(bigIntToBuffer(data.retOffset ?? BigInt(0)), 32)
const retLengthBuffer = setLengthLeft(bigIntToBuffer(data.retLength ?? BigInt(0)), 32)
const PUSH32 = Buffer.from('7f', 'hex')
const AUTHCALL = Buffer.from('f7', 'hex')
const order = [
Expand Down
4 changes: 2 additions & 2 deletions packages/vm/tests/api/istanbul/eip-1884.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Chain, Common, Hardfork } from '@ethereumjs/common'
import { ERROR } from '@ethereumjs/evm/dist/exceptions'
import { Address, bufferToBigInt, isTruthy } from '@ethereumjs/util'
import { Address, bufferToBigInt } from '@ethereumjs/util'
import * as tape from 'tape'

import { VM } from '../../../src/vm'
Expand All @@ -27,7 +27,7 @@ tape('Istanbul: EIP-1884', async (t) => {
const common = new Common({ chain, hardfork })
const vm = await VM.create({ common })

const balance = isTruthy(testCase.selfbalance) ? BigInt(testCase.selfbalance) : undefined
const balance = testCase.selfbalance !== undefined ? BigInt(testCase.selfbalance) : undefined
const account = createAccount(BigInt(0), balance)

await vm.stateManager.putAccount(addr, account)
Expand Down
18 changes: 7 additions & 11 deletions packages/vm/tests/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import {
bigIntToBuffer,
bufferToBigInt,
bufferToHex,
isFalsy,
isHexPrefixed,
isTruthy,
setLengthLeft,
stripHexPrefix,
toBuffer,
Expand Down Expand Up @@ -118,15 +116,15 @@ export function makeTx(
opts?: TxOptions
): FeeMarketEIP1559Transaction | AccessListEIP2930Transaction | Transaction {
let tx
if (isTruthy(txData.maxFeePerGas)) {
if (txData.maxFeePerGas !== undefined) {
tx = FeeMarketEIP1559Transaction.fromTxData(txData, opts)
} else if (isTruthy(txData.accessLists)) {
} else if (txData.accessLists !== undefined) {
tx = AccessListEIP2930Transaction.fromTxData(txData, opts)
} else {
tx = Transaction.fromTxData(txData, opts)
}

if (isTruthy(txData.secretKey)) {
if (txData.secretKey !== undefined) {
const privKey = toBuffer(txData.secretKey)
return tx.sign(privKey)
}
Expand Down Expand Up @@ -157,7 +155,7 @@ export async function verifyPostConditions(state: any, testData: any, t: tape.Te
const address = keyMap[key]
delete keyMap[key]

if (isTruthy(testData)) {
if (testData !== undefined) {
const promise = verifyAccountPostConditions(state, address, account, testData, t)
queue.push(promise)
} else {
Expand Down Expand Up @@ -223,9 +221,7 @@ export function verifyAccountPostConditions(

if (key === '0x') {
key = '0x00'
acctData.storage['0x00'] = isTruthy(acctData.storage['0x00'])
? acctData.storage['0x00']
: acctData.storage['0x']
acctData.storage['0x00'] = acctData.storage['0x00'] ?? acctData.storage['0x']
delete acctData.storage['0x']
}

Expand Down Expand Up @@ -259,9 +255,9 @@ export function verifyAccountPostConditions(
*/
export function verifyGas(results: any, testData: any, t: tape.Test) {
const coinbaseAddr = testData.env.currentCoinbase
const preBal = isTruthy(testData.pre[coinbaseAddr]) ? testData.pre[coinbaseAddr].balance : 0
const preBal = testData.pre[coinbaseAddr] !== undefined ? testData.pre[coinbaseAddr].balance : 0

if (isFalsy(testData.post[coinbaseAddr])) {
if (testData.post[coinbaseAddr] === undefined) {
return
}

Expand Down