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

refactor!: change many U256 type to U64 #1591

Merged
merged 15 commits into from
Dec 4, 2023
Merged

refactor!: change many U256 type to U64 #1591

merged 15 commits into from
Dec 4, 2023

Conversation

KaoImin
Copy link
Contributor

@KaoImin KaoImin commented Nov 23, 2023

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:

CI Settings

CI Usage

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

CI Switch

  • Web3 Compatible Tests
  • OpenZeppelin tests
  • OCT 1-5 And 12-15
  • OCT 6-10
  • OCT 11
  • OCT 16-19
  • v3 Core Tests

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

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

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"`

@Flouse Flouse linked an issue Nov 27, 2023 that may be closed by this pull request
@Flouse Flouse mentioned this pull request Nov 27, 2023
6 tasks
@KaoImin KaoImin force-pushed the refactor-u256-cast-u64 branch 2 times, most recently from bb650b3 to 421f8f8 Compare November 29, 2023 07:33
@KaoImin KaoImin marked this pull request as ready for review November 29, 2023 15:57
@KaoImin KaoImin requested a review from a team as a code owner November 29, 2023 15:57
@KaoImin KaoImin requested review from blckngm, Simon-Tl, driftluo and Flouse and removed request for Simon-Tl November 29, 2023 15:57
@KaoImin
Copy link
Contributor Author

KaoImin commented Nov 30, 2023

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 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.

driftluo
driftluo previously approved these changes Dec 1, 2023
@KaoImin
Copy link
Contributor Author

KaoImin commented Dec 1, 2023

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

This comment was marked as outdated.

@sunchengzhu

This comment was marked as outdated.

This comment was marked as outdated.

@sunchengzhu

This comment was marked as outdated.

1 similar comment
@Flouse
Copy link
Contributor

Flouse commented Dec 1, 2023

/run-ci

This comment was marked as outdated.

Copy link

github-actions bot commented Dec 1, 2023

CI tests run on commit:

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.

@axonweb3 axonweb3 deleted a comment from KaoImin Dec 1, 2023
@axonweb3 axonweb3 deleted a comment from github-actions bot Dec 1, 2023
@axonweb3 axonweb3 deleted a comment from KaoImin Dec 1, 2023
Flouse
Flouse previously approved these changes Dec 4, 2023
driftluo
driftluo previously approved these changes Dec 4, 2023
@KaoImin KaoImin dismissed stale reviews from driftluo and Flouse via 470093b December 4, 2023 04:01
@KaoImin KaoImin requested review from Flouse and driftluo December 4, 2023 04:02
@Flouse Flouse merged commit b90612d into main Dec 4, 2023
20 checks passed
Flouse added a commit that referenced this pull request Dec 4, 2023
* 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]>
@KaoImin KaoImin deleted the refactor-u256-cast-u64 branch December 5, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants