Breaking/Deprecation Tracking Issue

[holgerd77]

Tracking issue for breaking changes and deprecation tasks for an eventual round of breaking releases (roughly August/September 2024). A release round would follow the v7 releases from #2945 (Summer 2023).

Note: We have done so many additional breaking changes since this write-up (which are already completed) that it's not worth to continue to maintain or update this list. There are some remaining tasks to be extracted (in some updated form), otherwise treat this TODO list as rather outdated.

General

•  Revisit new/option methods/properties in exposed interfaces (StateManger,...) and see if to make things mandatory, if design is generally optimal or things should move,...
• Look into BigInt <-> Bytes conversion for certain edge cases Improve BigInt<>Bytes conversion helpers test coverage & address edge case issues #3180

Browser / Bundle Size / Performance

• Remove last Node polyfill needs (event emitter, other?), see e.g. Replace native events with eventemitter3 #3330
• Integrate Tree Shaking Optimizations, see Improve tree shaking for smaller bundle sizes #3446 overview and sub tasks

@ethereumjs/util

• Generally look through the bytes module and think what consistent (and eventually more length-strict) strategy we want to persue there, also regarding to the new length-specific helpers newly introduced by @gabrocheleau
• Go through the bytes module, and related functions that accept both PrefixedHexString | string as types, and restrict the type to only accept PrefixedHexString. Addressed by refactor: restrict PrefixedHexString types #3510

@ethereumjs/common

•  Get rid of networkId (superseeded by chainId), see Why is networkId explicitly required now? #3046 (might need validation) Common/Monorepo: Remove NetworkId #3513
• Common: consistent GasPrices naming #3037
• Make the StateManager interface's getAppliedKey method introduced by client: save preimages feature #3143 non-optional.

@ethereumjs/trie

•  Make trie option in genWithdrawalsTrieRoot()and genTransactionTrieRoot() mandatory to always have the eventual WASM crypto passed to the trie, see https://github.com/ethereumjs/ethereumjs-monorepo/pull/3192/files#r1448613272 for context
• Readable stream analysis refactor #3231 remove old stream method, eventually rename to generic, short name and remove dependency (remember to close Trie: Analyze readable-steam Dependency Removal/Replacement Options #3218 once the old stream method is deprecated) Stream dependency replacement with walk all value nodes #3519
•  Deal with deprecated methods - particularly around proofs - and align remaining inconsistent naming (like some names with createFromProof() vs fromProof() (likely better), also have a look at StateManager in this context)
  - [ ] Try to revive Web Streams API implementation (needed to be removed for browser compatibility reasons) in some alternative way (external scripts + generally remove all hard-wired stream functionality? Needs to be decided/discussed), also affects SM, see Readable stream analysis refactor #3231 and Remove nodejs import from trie.ts #3280 for context not applicable with Stream dependency replacement with walk all value nodes #3519
• Clean up utility methods - at a minimum unify nibblesToBytes and nibblestoBytes Breaking Trie/Statemanager -> Consolidate nibbles <-> bytes utility methods #3521

@ethereumjs/statemanager

• Make getContractCode not throw Make getContractStorage not throw if the account does not exist #3479

@ethereumjs/tx

• Finally refactor this +/- 27 insanity for v in ecsign (somewhere else?) in a clean way!!!, see also e.g. Add a method wrapper to directly extract the recovery bit value from ecsign. #1607, Util: Add support for EIP-1559 in ecsign() method #1597, Util: Signature ecsign / ecrecover functionality #1961, Util: Remove signature module in favor of direct ethereum-cryptography usage (needs analysis) #1845
•  Rename internal field representations to match the JSON RPC names: "coinbase field should be miner, uncleHash should be sha3Uncles, transactionsTrie should be transactionsRoot and receiptTrie should be receiptsRoot" as discussed with "tomorrowforsure.eth" in the chat (eventually look at VM.runBuild(), VM.runTx() receipt names as well, also eventually analog for Block properties (so at least a check))?
• Rename tx.getBaseFee() to something which does not use the term "base fee". (Also change the error in VM). See: Breaking/Deprecation Tracking Issue #3216 (comment)

@ethereumjs/block

•  Rename validation-hiding getValidationErrors() (and the like) in tx, block to something like isValidWithErrors()
• Fix order of deposits in main block constructors (move before opts) - see e.g. EIP-6110: Supply Validator Deposits on Chain #3303 (executionWitness can/should likely remain where it is until Verkle is integrated)
• Maybe we get the calcDifficultyFromHeader and cliqueSigner options out of the BlockOptions or at least better hide/encapsule this legacy stuff? Not sure.

@ethereumjs/verkle

•  See if we can come up with a generic tree interface for both Trie and Verkle which we can put in Common (as we do for StateManager e.g.) which abstracts away to a greater extend the need for a Verkle/Trie distinction (at least for some places/usages, might also turn out to be only a subset of the functionality)

[jochem-brouwer]

To align the JSON RPC names we should consider the official (?) spec for the names: https://github.com/ethereum/execution-apis/tree/main/src/eth. (And while we are doing it, check all, so also block, or eth_call methods)

[jochem-brouwer]

There is something which I keep noticing, and keep getting confused about:

ethereumjs-monorepo/packages/vm/src/runTx.ts

Lines 264 to 267 in d8f7544

	gasLimit -= txBaseFee
	if (this.DEBUG) {
	debugGas(`Subtracting base fee (${txBaseFee}) from gasLimit (-> ${gasLimit})`)
	}

This is NOT the base fee as in EIP-1559, but it is the "base fee" of the transaction. This is the cost of the tx (21000) plus the data fee (depends on calldata) and extra fee if this is a CREATE transaction. However we also have this tx.getBaseFee() method (which returns this). We should definitely rename this to something else, because this is super confusing. (Will add to tx)

[jochem-brouwer]

Linking #3479 (make getContractStorage not throw for non-existing accounts)

[acolytec3]

There is something which I keep noticing, and keep getting confused about:

ethereumjs-monorepo/packages/vm/src/runTx.ts

Lines 264 to 267 in d8f7544

	gasLimit -= txBaseFee
	if (this.DEBUG) {
	debugGas(`Subtracting base fee (${txBaseFee}) from gasLimit (-> ${gasLimit})`)
	}

This is NOT the base fee as in EIP-1559, but it is the "base fee" of the transaction. This is the cost of the tx (21000) plus the data fee (depends on calldata) and extra fee if this is a CREATE transaction. However we also have this tx.getBaseFee() method (which returns this). We should definitely rename this to something else, because this is super confusing. (Will add to tx)

I think this is what EIPs usually refer to as intrinsic gas or something like that. That name isn't any more obvious but at least if we use that term, it would align us with broader core dev usage.

[jochem-brouwer]

Ok, I will open a PR to make these terms more clear :) I'll check the EIPs what terms they use but I think I indeed remember instrinsic gas.