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

[WIP] Backport upstream changes #2

Draft
wants to merge 42 commits into
base: develop
Choose a base branch
from

Conversation

maxim-inj
Copy link

@maxim-inj maxim-inj commented Sep 7, 2024

Need to evaluate and backport some or all fixes from the upstream.

  • Problem: gasWanted is not aligned for process proposal mode (#773438) ⚠️
  • Problem: no way to disable nonce checking in benchmark (#f9cd15)
  • Problem: no MaxTx set from config for mempool (#e5b222)
  • Problem: insufficient invalid value check in eth transaction (#45ac08)
  • Problem: incarnation cache not enabled (#9c959a)
  • Problem: no overflow check when add gasLimit to gasWanted (#d874df) ⚠️
  • Problem: invalid chain id for signer (#f3e62c) ⚠️
  • Problem: benchmark logic don't increase nonce (#350955)
  • Problem: cometbft not up to date (#c36b26)
  • Problem: disable of create vesting account messages are not complete (#b0022e)
  • Optimize AnteHandle method to skip checks if disabledMsgs is empty (#ff4caa)
  • Problem: vote_extensions is not disabled in test (#0e0baa)
  • Problem: no debug files uploaded when timeout in ci (#e3b813)
  • Problem: nondeterministic account set occurs with more stm workers (#7ef8af)
  • Problem: unnecessary GetAccount in ante handlers (#fe3f4f)
  • Problem: handle for integer overflow is messy (#d26ada) ⚠️
  • Problem: method eth_chainId crashed occasionally(#452b0c) ⚠️
  • Problem: hash in subscribe newHeads differs from eth_getBlockByNumber (#3fabdbe) ⚠️
  • Problem: block-stm tx executor bad worst case performance (#79bb39e)
  • Problem: pre-estimation don't run in parallel (#56f8a5)
  • Problem: tx evm raw doesn't work under offline (#dbc7eb4)
  • Problem: get unnecessary block result when only need header (#7e30762)
  • Problem: trace block gas refund change not committed (#bcac9e2) ⚠️
  • Problem: db is double closed when shutdown (#f2a562b) ⚠️
  • Problem: no time in Transaction (#9755333) ⚠️
  • Problem: multisig account failed on threshold encode after send tx #33e3cc6
  • Problem: dependencies are outdated #5acb559
  • Problem: get unnecessary block result in header related api call #b16b489
  • Problem: unsuppored sign mode SIGN_MODE_TEXTUAL in bank transfer #c4cef0f
  • Problem: validation broke after transaction conversion with raw field #617eac9
  • Problem: opBlockhash broke after sdk50 #853e1e5 ⚠️
  • Problem: default headerHashNum is too large #580ce6e ⚠️
  • Problem: no header hash from fallback historicalInfo #a2ad87c ⚠️
  • Problem: node can't quit by signal #816389c ⚠️

Additional issues

  • Problem: reverted Evm call doesn't revert Tx, other msgs in Tx still execute ⚠️
  • Problem: fee collector account failed to refund fees: the check always considers that 100% of gas is refundable
  • Problem: need to support eth_getBlockReceipts see Get receipts by block number ethereum/go-ethereum#19634 ⚠️

mmsqe and others added 15 commits June 26, 2024 20:26
* Problem: gasWanted is not aligned for process proposal mode

* check maxGasWanted
Solution:
- add unsafe option to turn off nonce checking
* Problem: no MaxTx set from config for mempool

* Update config.go

Co-authored-by: yihuang <[email protected]>
Signed-off-by: mmsqe <[email protected]>

---------

Signed-off-by: mmsqe <[email protected]>
Co-authored-by: yihuang <[email protected]>
* Problem: incarnation cache not enabled

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* cache eth sig verification result

* fix concurrent incarnations

* update deps

---------

Signed-off-by: yihuang <[email protected]>
Solution:
- keep the benchmark logic closer to real one, only removing the nonce
  check part
…506)

* Problem: disable of create vesting account messages are not complete

* Update CHANGELOG.md

Signed-off-by: mmsqe <[email protected]>

---------

Signed-off-by: mmsqe <[email protected]>
)

* Optimize AnteHandle method to skip checks if disabledMsgs is empty

* Problem: cometbft not up to date (#505)

include the fix from cometbft/cometbft#3196

* Problem: disable of create vesting account messages are not complete (#506)

* Problem: disable of create vesting account messages are not complete

* Update CHANGELOG.md

Signed-off-by: mmsqe <[email protected]>

---------

Signed-off-by: mmsqe <[email protected]>

* CHANGELOG

* Update CHANGELOG.md

Signed-off-by: mmsqe <[email protected]>

---------

Signed-off-by: mmsqe <[email protected]>
Signed-off-by: fx0x55 <[email protected]>
Co-authored-by: mmsqe <[email protected]>
Co-authored-by: mmsqe <[email protected]>
* Problem: vote_extensions is not disabled in test

* bump sdk
* Problem: no debug files uploaded when timeout in ci

30mins for marked, 1h for all in pytest

* Apply suggestions from code review

* set both
)

* rename

* pytest -v -s test_debug_traceblock.py

* follow existing practice

* update test

* update stm

* test

* add timeout

* disable

* revert

* fix

* update deps

---------

Co-authored-by: huangyi <[email protected]>
* Problem: unnecessary GetAccount in ante handlers

reuse account after verify account balance to avoid get again when check sender sequence

* rename

* less MakeSigner

* cast string

* cleanup

* fix test

* fix lint

* cleanup

* cleanup test

* rm prefix

* fix

* fix test, account created automatically

* rename

* fix comment

* Apply suggestions from code review

Signed-off-by: yihuang <[email protected]>

* cleanup

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: huangyi <[email protected]>
@maxim-inj maxim-inj self-assigned this Sep 7, 2024
Copy link

coderabbitai bot commented Sep 7, 2024

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@maxim-inj maxim-inj changed the title Backport upstream changes [WIP] Backport upstream changes Sep 7, 2024
mmsqe and others added 12 commits September 10, 2024 11:46
* Problem: method eth_chainId crashed occasionally

add fallback default config to ensure no panic in IsEIP155 check

* cleanup

---------

Co-authored-by: huangyi <[email protected]>
…#521)

* Problem: hash in subscribe newHeads differs from eth_getBlockByNumber

* fix test

* cleanup
* Problem: block-stm tx executor don't do simple static dependency
analysis

Solution:
- estimate dependencies based on tx fee payer, try to optimise worst case
  performance.

* fix build

* customize estimates instead of dependencies

* fix context

* update api

* cleanup

* changelog

* cleanup

* cleanup

* cleanup

* pre-estimate config

* fix lint
* Problem: pre-estimation don't run in parallel

* fix build

* fix race

* cleanup chunking

* keep unchanged

---------

Co-authored-by: mmsqe <[email protected]>
* Problem: trace block not accurate for dynamic fee transaction

fix

changelog

temp

* Update x/evm/keeper/state_transition.go

Signed-off-by: yihuang <[email protected]>

* avoid mutate config tracer

* fix test

* adjust test

* more test

* resolve

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: mmsqe <[email protected]>
* Problem: db is double closed when shutdown

* fix standAlone
* Problem: no time in Transaction

* update deps
)

* Problem: marshal error get overwritten by unexpected end of JSON input

* test multisig

* fix threshold encode

* pass

* update deps
* Problem: dependencies are outdated

* bump cometbft ibc

* cleanup

* fix build
mmsqe and others added 15 commits October 4, 2024 09:24
* Problem: get unnecessary block result in DoCall

* GetProof GetBalance

* HeaderByNumber

* EstimateGas

* TraceCall

* fix test
…#536)

* Problem: validation broke after transaction conversion with raw field

* cleanup

* less validate

* borrow

* Apply suggestions from code review

Signed-off-by: yihuang <[email protected]>

* rm chainid check

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: yihuang <[email protected]>
* Problem: opBlockhash broke after sdk50

* cleanup

* Apply suggestions from code review

* DeleteHeaderHash

* add header_hash_num in param

* Apply suggestions from code review

Signed-off-by: yihuang <[email protected]>

* Apply suggestions from code review

Signed-off-by: yihuang <[email protected]>

* Update x/evm/types/key.go

Signed-off-by: yihuang <[email protected]>

* Apply suggestions from code review

* fix build

* lint

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: yihuang <[email protected]>
* clean up

* Problem: default headerHashNum is too large
* Problem: no header hash from fallback historicalInfo

* test

* keep headerNum no change

* align check

* more test
* Problem: node can't quit by signal

Solution:
- cleanup graceful shutdown procedure

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix lint

* cleanup

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: mmsqe <[email protected]>
* Problem: state overwrite not work in debug trace API

* test

* fix
* Problem: check tx blocks consensus

Solution:
- try NewConnSyncLocalClientCreator

* optional

* optional in flags

* add log

* changelog
* graceful shutdown of websocket

* context for unsubscribe logs

* added a logger to the log subscription

* lint

---------

Co-authored-by: mmsqe <[email protected]>
* Problem: build fail without cgo

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* revert

---------

Signed-off-by: yihuang <[email protected]>
* Problem: rpc stream overhead even if not used

Solution:
- init the underlying subscriptions lazily.

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* fix build

* ignore pending tx

---------

Signed-off-by: yihuang <[email protected]>
* removed listener closing in gracefully shutdown json-rpc server

* just logging error from shutdown json-rpc server

---------

Co-authored-by: yihuang <[email protected]>
* Problem: check-tx don't run in parallel

Solution:
- try with experimental flag

* fix test

---------

Co-authored-by: mmsqe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants