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

Fixes and improvements #59

Merged
merged 6 commits into from
Aug 18, 2023
Merged

Fixes and improvements #59

merged 6 commits into from
Aug 18, 2023

Conversation

AntonD3
Copy link
Collaborator

@AntonD3 AntonD3 commented Aug 18, 2023

What 💻

  • Increased rich wallets number to 10
  • Increased rich wallets start balance
  • Fixed eth_getCode
  • Fixed revert processing in the eth_call
  • Fixed reverts in the eth_estimateGas
  • Fixed effective_gas_price in the eth_getTransactionReceipt

Why ✋

  • Some complex tests require more wallets, for example, DEFI, where needed to be separate accounts for the liquidity providers, traders, owners, etc.
  • A big amount of ether can be used when you want to write tests with real-world amounts
  • Previously eth_code was returning codehash, not the code, it was fixed
  • The error in case of transaction revert during eth_call should be returned to be compatible with the testing tools
  • The error data returned from eth_estimateGas was updated, to be compatible as well
  • The wrong constant value for the effective_gas_price was used

Evidence 📷

Screenshot 2023-08-18 at 10 25 49

Here is an example where error data is used by a tooling

Here is - revert word in the error body/message

BEFORE and AFTER of eth_code request:
image

Before:
image

After:
image

Notes 📝

@AntonD3 AntonD3 marked this pull request as ready for review August 18, 2023 06:50
Copy link
Collaborator

@MexicanAce MexicanAce left a comment

Choose a reason for hiding this comment

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

Please add Verification Evidence to the PR description for all changes. Also, I think this resolved 2 of the open GitHub Issues, so let's tag them in the Notes section of the PR description as well.

src/main.rs Outdated Show resolved Hide resolved
src/node.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@MexicanAce MexicanAce left a comment

Choose a reason for hiding this comment

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

LGTM

@MexicanAce MexicanAce merged commit 30d7dc3 into main Aug 18, 2023
8 checks passed
@MexicanAce MexicanAce deleted the ad-fixes-improvements branch August 18, 2023 09:01
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.

2 participants