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

Jump to bottom

evm: add blobgasfee eip 7516 #3035

Merged
merged 8 commits into from
Sep 19, 2023
Merged

evm: add blobgasfee eip 7516 #3035

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f208388
evm: add blobgasfee eip 7516
g11tech Sep 18, 2023
8a25c22
Update packages/evm/src/interpreter.ts
g11tech Sep 18, 2023
2f8e578
Update packages/common/src/hardforks.ts
acolytec3 Sep 18, 2023
bcfedc8
update min hardfork
acolytec3 Sep 18, 2023
1e84dde
add to supported eips
g11tech Sep 19, 2023
7665573
Merge remote-tracking branch 'origin/master' into opcode-blobgasfee
g11tech Sep 19, 2023
7aac40c
update minimum hf to cancun
g11tech Sep 19, 2023
f8e3152
update min hf to paris same as 4844
g11tech Sep 19, 2023
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
13 changes: 13 additions & 0 deletions packages/common/src/eips.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,4 +471,17 @@ export const EIPs: EIPsDict = {
minimumHardfork: Hardfork.London,
requiredEIPs: [],
},
7516: {
comment: 'BLOBBASEFEE opcode',
url: 'https://eips.ethereum.org/EIPS/eip-7516',
status: Status.Draft,
minimumHardfork: Hardfork.Shanghai,
requiredEIPs: [4844],
gasPrices: {
blobbasefee: {
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
blobbasefee: {
blobBaseFee: {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually the opcode gas fee lookup in evm is:

      const baseFee = Number(common.param('gasPrices', opcodeBuilder[key].name.toLowerCase()))

so it has to be all lowercase i think

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right. I loked across the eips.ts file and there's no consistency here. Just a silly thought but maybe a follow-up PR is to just make all of the gasPrices fields in common/src/eips lowercase so we can drop that toLowerCase call here. That's certainly a small performance gain for every single time we get the base fee.

Copy link
Member

Choose a reason for hiding this comment

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

@acolytec3 Note that the line which @g11tech references to is in codes.ts of EVM (the opcode builder). The opcode builder expects all to be opcode names in common to be lowercase. Note: this method is only called if we get new opcodes (at EVM startup + fork transition) so this is not called each time for all opcodes if one is called.

Copy link
Member

Choose a reason for hiding this comment

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

Won't die on this hill but in most cases we do have the camel case and I do somewhat like it for readability, but it is also true that this is a lot more inconsistent than I anticipated when looking through Common naming. Atm we can't update anyhow since this would be a breaking change.

So for here I guess we can keep both ways.

v: 2,
d: 'Gas cost of the BLOBBASEFEE opcode',
},
},
},
}
4 changes: 2 additions & 2 deletions packages/common/src/hardforks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,9 +834,9 @@ export const hardforks: HardforksDict = {
cancun: {
name: 'cancun',
comment:
'Next feature hardfork after the shanghai having proto-danksharding EIP 4844 blobs (still WIP hence not for production use), transient storage opcodes, parent beacon block root availability in EVM and selfdestruct only in same transaction',
'Next feature hardfork after shanghai, includes proto-danksharding EIP 4844 blobs (still WIP hence not for production use), transient storage opcodes, parent beacon block root availability in EVM, selfdestruct only in same transaction, and blob base fee opcode',
url: 'https://github.com/ethereum/execution-specs/blob/master/network-upgrades/mainnet-upgrades/cancun.md',
status: Status.Final,
eips: [1153, 4844, 4788, 5656, 6780],
eips: [1153, 4844, 4788, 5656, 6780, 7516],
},
}
5 changes: 3 additions & 2 deletions packages/evm/src/evm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export class EVM implements EVMInterface {
// Supported EIPs
const supportedEIPs = [
1153, 1559, 2315, 2565, 2718, 2929, 2930, 3074, 3198, 3529, 3540, 3541, 3607, 3651, 3670,
3855, 3860, 4399, 4895, 4788, 4844, 5133, 5656, 6780,
3855, 3860, 4399, 4895, 4788, 4844, 5133, 5656, 6780, 7516,
]

for (const eip of this.common.eips()) {
Expand Down Expand Up @@ -966,7 +966,7 @@ export function EvmErrorResult(error: EvmError, gasUsed: bigint): ExecResult {
}
}

function defaultBlock(): Block {
export function defaultBlock(): Block {
return {
header: {
number: BigInt(0),
Expand All @@ -977,6 +977,7 @@ function defaultBlock(): Block {
prevRandao: zeros(32),
gasLimit: BigInt(0),
baseFeePerGas: undefined,
getBlobGasPrice: () => undefined,
},
}
}
12 changes: 12 additions & 0 deletions packages/evm/src/interpreter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,18 @@
return baseFee
}

/**
* Returns the Blob Base Fee of the block as proposed in [EIP-7516](https;//eips.etheruem.org/EIPS/eip-7516)
*/
getBlobBaseFee(): bigint {
const blobBaseFee = this._env.block.header.getBlobGasPrice()
if (blobBaseFee === undefined) {
// Sanity check
throw new Error('Block has no Blob Base Fee')
}

Check warning on line 771 in packages/evm/src/interpreter.ts

View check run for this annotation

Codecov / codecov/patch

packages/evm/src/interpreter.ts#L769-L771 

Added lines #L769 - L771 were not covered by tests
return blobBaseFee
}
Copy link
Member

Choose a reason for hiding this comment

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

Not so deep in the blob pricing topic: so is "blobBaseFee === blobGasPrice" here?

If so: I think we then should consider still renaming all blobGasPrice related naming in Block to blobBaseFee? (we can still do since all 4844 functionality is still non-final)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, the confusion is real- here is an actual discussion happening on it lol:

https://discord.com/channels/595666850260713488/745077610685661265/1153600913932554270

Copy link
Member

Choose a reason for hiding this comment

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

Guess we might want to wait with merging here until this is settled? 🤔 Since this is the very last question on ACD channel and still un-answered?


/**
* Returns the chain ID for current chain. Introduced for the
* CHAINID opcode proposed in [EIP-1344](https://eips.ethereum.org/EIPS/eip-1344).
Expand Down
6 changes: 6 additions & 0 deletions packages/evm/src/opcodes/codes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,12 @@ const eipOpcodes: { eip: number; opcodes: OpcodeEntry }[] = [
0x5e: { name: 'MCOPY', isAsync: false, dynamicGas: true },
},
},
{
eip: 7516,
opcodes: {
0x4a: { name: 'BLOBBASEFEE', isAsync: false, dynamicGas: false },
},
},
]

/**
Expand Down
7 changes: 7 additions & 0 deletions packages/evm/src/opcodes/functions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,13 @@ export const handlers: Map<number, OpHandler> = new Map([
}
},
],
// 0x4a: BLOBBASEFEE
[
0x4a,
function (runState) {
runState.stack.push(runState.interpreter.getBlobBaseFee())
},
],
// 0x50 range - 'storage' and execution
// 0x50: POP
[
Expand Down
1 change: 1 addition & 0 deletions packages/evm/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ export type Block = {
prevRandao: Uint8Array
gasLimit: bigint
baseFeePerGas?: bigint
getBlobGasPrice(): bigint | undefined
}
}

Expand Down
38 changes: 38 additions & 0 deletions packages/evm/test/runCall.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Account,
Address,
MAX_UINT64,
bytesToBigInt,
bytesToHex,
concatBytes,
hexToBytes,
Expand All @@ -13,6 +14,7 @@ import { keccak256 } from 'ethereum-cryptography/keccak.js'
import { assert, describe, it } from 'vitest'

import * as genesisJSON from '../../client/test/testdata/geth-genesis/eip4844.json'
import { defaultBlock } from '../src/evm.js'
import { ERROR } from '../src/exceptions.js'
import { EVM } from '../src/index.js'

Expand Down Expand Up @@ -602,6 +604,42 @@ describe('RunCall tests', () => {
)
})

it('runCall() => use BLOBBASEFEE opcode from EIP 7516', async () => {
// setup the evm
const common = Common.fromGethGenesis(genesisJSON, {
chain: 'custom',
hardfork: Hardfork.Cancun,
})
const evm = new EVM({
common,
})

const BLOBBASEFEE_OPCODE = 0x4a
assert.equal(
evm.getActiveOpcodes().get(BLOBBASEFEE_OPCODE)!.name,
'BLOBBASEFEE',
'Opcode 0x4a named BLOBBASEFEE'
)

const block = defaultBlock()
block.header.getBlobGasPrice = () => BigInt(119)

// setup the call arguments
const runCallArgs: EVMRunCallOpts = {
gasLimit: BigInt(0xffffffffff),
// calldata -- retrieves the blobgas and returns it from memory
data: hexToBytes('0x4a60005260206000F3'),
block,
}
const res = await evm.runCall(runCallArgs)
assert.equal(
bytesToBigInt(unpadBytes(res.execResult.returnValue)),
BigInt(119),
'retrieved correct gas fee'
)
assert.equal(res.execResult.executionGasUsed, BigInt(6417), 'correct blob gas fee (2) charged')
})

it('step event: ensure EVM memory and not internal memory gets reported', async () => {
const common = new Common({ chain: Chain.Mainnet, hardfork: Hardfork.Berlin })
const evm = new EVM({
Expand Down
Loading