refactor!: change many `U256` type to `U64` by KaoImin · Pull Request #1591 · axonweb3/axon

KaoImin · 2023-11-23T07:03:21Z

What this PR does / why we need it?

This PR is substitute for #1539

Some mainly changes:

Change the type of nonce from U256 to U64 according to the EIP-2681 limit the account nonce to be between 0 and 2^64-1.
Change the type of gas_limit from U256 to U64. According to the current gas cost of the most complex ethereum transaction is on the order of million, U64 is enough.
Change the type of chain_id from U256 to U64 according to metamask limit. The ChainId opcode returns a 256-bit value, so it should not larger than U256::MAX / 2 - 35.This issue does not lead to consensus in Ethereum community, however, I think limit the max chain ID to 4503599627370476 is enough currently. Many temporary L2/L3 need a unique chain ID is the demand of variable-lenght chain ID in foreseeable future. But we do not have the demand now. If the day comes, we can change the chain ID type to U256 even without hardfork.
Set the Default Max Price as 500 Gwei that is same as go-ethereum. Meanwhile, change the type of gas_price from U256 to U64.

What is the impact of this PR?

Breaking change

PR relation:

Ref refactor: change some types from U256 to U64 #1539
Fix bug: bypass restrictions through p2p message, even cause panic #1586
test code updated: axonweb3/axon-test@be8b502...df72c7e

CI Settings

CI Usage

Tip: Check the CI you want to run below, and then comment /run-ci.

CI Switch

CI Description

CI Name	Description
Web3 Compatible Test	Test the Web3 compatibility of Axon
v3 Core Test	Run the compatibility tests provided by Uniswap V3
OCT 1-5 \| 6-10 \| 11 \| 12-15 \| 16-19	Run the compatibility tests provided by OpenZeppelin

chaoticlonghair · 2023-11-27T03:27:29Z

References:

Ethereum Yellow Paper
- 4.2. The Transaction
  - nonce: a scalar value
  - gasPrice: a scalar value
  - gasLimit: a scalar value
  - value: a scalar value
  - chainId: unknown
- 4.3. The Block
  - difficulty: a scalar value
  - number: a scalar value
  - gasLimit: a scalar value
  - gasUsed: a scalar value
  - timestamp: a scalar value
  - nonce: a 64-bit value

Go Etherem:

Transaction

chainID() *big.Int
gas() uint64
gasPrice() *big.Int
gasTipCap() *big.Int
gasFeeCap() *big.Int
value() *big.Int
nonce() uint64

Block

Difficulty  *big.Int       `json:"difficulty"       gencodec:"required"`
Number      *big.Int       `json:"number"           gencodec:"required"`
GasLimit    uint64         `json:"gasLimit"         gencodec:"required"`
GasUsed     uint64         `json:"gasUsed"          gencodec:"required"`
Time        uint64         `json:"timestamp"        gencodec:"required"`
Extra       []byte         `json:"extraData"        gencodec:"required"`
MixDigest   common.Hash    `json:"mixHash"`
Nonce       BlockNonce     `json:"nonce"`

KaoImin · 2023-11-30T04:37:08Z

References:

Ethereum Yellow Paper

4.2. The Transaction

nonce: a scalar value
gasPrice: a scalar value
gasLimit: a scalar value
value: a scalar value
chainId: unknown

4.3. The Block

difficulty: a scalar value
number: a scalar value
gasLimit: a scalar value
gasUsed: a scalar value
timestamp: a scalar value
nonce: a 64-bit value

Go Etherem:

Difficulty  *big.Int       `json:"difficulty"       gencodec:"required"`
Number      *big.Int       `json:"number"           gencodec:"required"`
GasLimit    uint64         `json:"gasLimit"         gencodec:"required"`
GasUsed     uint64         `json:"gasUsed"          gencodec:"required"`
Time        uint64         `json:"timestamp"        gencodec:"required"`
Extra       []byte         `json:"extraData"        gencodec:"required"`
MixDigest   common.Hash    `json:"mixHash"`
Nonce       BlockNonce     `json:"nonce"`

chainID() *big.Int
gas() uint64
gasPrice() *big.Int
gasTipCap() *big.Int
gasFeeCap() *big.Int
value() *big.Int
nonce() uint64

According to the PR description, some types such as nonce, gasPrice, gasLimit, gasUsed have a specific range limitation, it is a waste of performance to use a variable-length BigUint. The Ethereum timestamp is accurate to the second, so u64 is enough. The only one that using variable-length scalar is chainId. The explanation of using U64 is written in the description.

KaoImin · 2023-12-01T03:29:20Z

Wait for @sunchengzhu upgrade Web3 Compatible Tests workflow. cc @Flouse

Flouse · 2023-12-01T08:48:34Z

/run-ci

github-actions · 2023-12-01T08:48:49Z

CI tests run on commit:

commit id: 3d507e5

CI test list:

openzeppelin_test_1_5_and_12_15.yml
openzeppelin_test_6_10.yml
openzeppelin_test_16_19.yml
openzeppelin_test_11.yml
v3_core_test.yml
web3_compatible.yml

Please check ci test results later.

Co-authored-by: Flouse <[email protected]>

* refactor: change many U256 type to U64 * change some safe as_u64 to low_u64 & cargo fmt * refactor the gas price and limit check * refactor prepay gas calculation * revert max gas limit * fix e2e test * Update eth_getBalance.test.js * remove useless code This PR is substitute for #1539 Some mainly changes: * Change the type of `nonce` from `U256` to `U64` according to the [EIP-2681](https://eips.ethereum.org/EIPS/eip-2681) limit the account nonce to be between `0` and `2^64-1`. * Change the type of `gas_limit` from `U256` to `U64`. According to the current gas cost of the most complex ethereum transaction is on the order of million, `U64` is enough. * Change the type of `chain_id` from `U256` to `U64` according to [metamask limit](https://gist.github.com/rekmarks/a47bd5f2525936c4b8eee31a16345553). The [`ChainId` opcode](https://eips.ethereum.org/EIPS/eip-1344) returns a 256-bit value, so it should not larger than `U256::MAX / 2 - 35`.This issue does not lead to consensus in Ethereum community, however, I think limit the max chain ID to `4503599627370476` is enough currently. Many temporary L2/L3 need a unique chain ID is the demand of variable-lenght chain ID in foreseeable future. But we do not have the demand now. If the day comes, we can change the chain ID type to `U256` even without hardfork. * Set the [`Default Max Price`](https://github.com/ethereum/go-ethereum/blob/be65b47/eth/gasprice/gasprice.go#L38) as `500` Gwei that is same as go-ethereum. Meanwhile, change the type of `gas_price` from `U256` to `U64`. --------- Co-authored-by: sunchengzhu <[email protected]> Co-authored-by: Flouse <[email protected]>

github-actions Bot added the refactor label Nov 23, 2023

KaoImin mentioned this pull request Nov 23, 2023

refactor: change some types from U256 to U64 #1539

Closed

6 tasks

KaoImin force-pushed the refactor-u256-cast-u64 branch 2 times, most recently from 93a966b to 463cfb6 Compare November 24, 2023 09:17

Flouse linked an issue Nov 27, 2023 that may be closed by this pull request

Duplicated calculation in eth_estimateGas. #1584

Closed

Flouse mentioned this pull request Nov 27, 2023

Release Axon v0.3.0-beta #1596

Closed

6 tasks

KaoImin force-pushed the refactor-u256-cast-u64 branch 2 times, most recently from bb650b3 to 421f8f8 Compare November 29, 2023 07:33

Flouse linked an issue Nov 29, 2023 that may be closed by this pull request

Reduce the size for many fixed-size types, such as gas_limit, and so on. #1553

Closed

KaoImin marked this pull request as ready for review November 29, 2023 15:57

KaoImin requested a review from a team as a code owner November 29, 2023 15:57

KaoImin requested review from Flouse, Simon-Tl, blckngm and driftluo and removed request for Simon-Tl November 29, 2023 15:57

KaoImin requested a review from chaoticlonghair November 30, 2023 04:37

KaoImin force-pushed the refactor-u256-cast-u64 branch from 7cb85b0 to e59e269 Compare November 30, 2023 15:35

This comment was marked as outdated.

Sign in to view

driftluo previously approved these changes Dec 1, 2023

View reviewed changes

KaoImin added 7 commits December 1, 2023 14:52

refactor: change many U256 type to U64

96ba83c

change some safe as_u64 to low_u64 & cargo fmt

2f20d0c

cargo fmt

eb0576f

refactor the gas price and limit check

21ac601

cargo fmt

c207b99

refactor prepay gas calculation

519450c

revert max gas limit

e9a77c5

This comment was marked as outdated.

Sign in to view

Flouse reviewed Dec 1, 2023

View reviewed changes

Comment thread protocol/src/constants/configs.rs Outdated

sunchengzhu force-pushed the refactor-u256-cast-u64 branch from e05798d to 41e8f18 Compare December 1, 2023 08:37

sunchengzhu mentioned this pull request Dec 1, 2023

chore(CI): update axon-test version #1612

Merged

3 tasks

refactor: fix Gwei

3d507e5

This comment was marked as outdated.

Sign in to view

axonweb3 deleted a comment from KaoImin Dec 1, 2023

axonweb3 deleted a comment from github-actions Bot Dec 1, 2023

axonweb3 deleted a comment from KaoImin Dec 1, 2023

Flouse reviewed Dec 1, 2023

View reviewed changes

Comment thread protocol/src/types/primitive.rs

sunchengzhu mentioned this pull request Dec 1, 2023

chore(CI): update axon-test version 2nd #1613

Merged

3 tasks

KaoImin requested a review from Flouse December 1, 2023 10:28

remove useless code

bc1b91b

Flouse previously approved these changes Dec 4, 2023

View reviewed changes

driftluo previously approved these changes Dec 4, 2023

View reviewed changes

Flouse reviewed Dec 4, 2023

View reviewed changes

Comment thread core/executor/src/tests/mod.rs

Update core/executor/src/tests/mod.rs

470093b

Co-authored-by: Flouse <[email protected]>

KaoImin dismissed stale reviews from driftluo and Flouse via 470093b December 4, 2023 04:01

KaoImin requested review from Flouse and driftluo December 4, 2023 04:02

Flouse approved these changes Dec 4, 2023

View reviewed changes

Flouse merged commit b90612d into main Dec 4, 2023

KaoImin deleted the refactor-u256-cast-u64 branch December 5, 2023 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: change many `U256` type to `U64`#1591

refactor!: change many `U256` type to `U64`#1591
Flouse merged 15 commits intomainfrom
refactor-u256-cast-u64

KaoImin commented Nov 23, 2023 •

edited by Flouse

Loading

Uh oh!

chaoticlonghair commented Nov 27, 2023

Uh oh!

KaoImin commented Nov 30, 2023

Uh oh!

This comment was marked as outdated.

KaoImin commented Dec 1, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Flouse commented Dec 1, 2023

Uh oh!

This comment was marked as outdated.

github-actions Bot commented Dec 1, 2023

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

KaoImin commented Nov 23, 2023 • edited by Flouse Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it?

What is the impact of this PR?

CI Usage

CI Description

Uh oh!

chaoticlonghair commented Nov 27, 2023

Uh oh!

KaoImin commented Nov 30, 2023

Uh oh!

This comment was marked as outdated.

KaoImin commented Dec 1, 2023

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Flouse commented Dec 1, 2023

Uh oh!

This comment was marked as outdated.

github-actions Bot commented Dec 1, 2023

CI tests run on commit:

CI test list:

Please check ci test results later.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KaoImin commented Nov 23, 2023 •

edited by Flouse

Loading