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

client: Miscellaneous tx related fixes for eth methods #2411

Merged
merged 6 commits into from
Nov 7, 2022

Conversation

g11tech
Copy link
Contributor

@g11tech g11tech commented Nov 5, 2022

  • extend RpcTx,
  • blockOpt optional in estimateGas,
  • improved tx fields validation

some eth tx method fixes extracted from shandong

@codecov
Copy link

codecov bot commented Nov 5, 2022

Codecov Report

Merging #2411 (08d2d41) into master (fd121be) will decrease coverage by 1.68%.
The diff coverage is 92.10%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 89.42% <ø> (ø)
blockchain 90.21% <ø> (ø)
client 87.44% <92.10%> (+0.04%) ⬆️
common 98.13% <ø> (ø)
devp2p 91.65% <ø> (+0.21%) ⬆️
ethash ∅ <ø> (∅)
evm 79.72% <ø> (?)
rlp ?
statemanager 89.46% <ø> (ø)
trie 90.36% <ø> (+0.34%) ⬆️
tx 97.80% <ø> (ø)
util 83.89% <ø> (ø)
vm 85.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@acolytec3 acolytec3 mentioned this pull request Nov 6, 2022
13 tasks
@acolytec3
Copy link
Contributor

This looks good as far as it goes. I've spent the last hour trying to figure out how to run a test for a london block without success, but it would be good to add those tests to verify things work right for Typed Transactions. Will try to work on it some more and see if I can't get one that's valid.

@g11tech g11tech changed the title client: Extend RpcTx type with 1559 fields and make blockOpt optional in estimateGas client: Miscellaneous tx related fixes for eth methods (extend RpcTx, blockOpt optional in estimateGas, improved tx fields validation) Nov 6, 2022
@g11tech g11tech changed the title client: Miscellaneous tx related fixes for eth methods (extend RpcTx, blockOpt optional in estimateGas, improved tx fields validation) client: Miscellaneous tx related fixes for eth methods Nov 6, 2022
@holgerd77
Copy link
Member

Have re-labelled to "WIP" to reflect the latest comment, @acolytec3 please switch back to "Ready for review/merge" or something when you are done with the tests.

@acolytec3
Copy link
Contributor

Finally got my new tests working. We'll see if this makes code coverage happy and will merge if so.

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM now that coverage is up!

@acolytec3 acolytec3 merged commit 9d470d3 into master Nov 7, 2022
@acolytec3 acolytec3 deleted the eth-tx-fixes branch November 7, 2022 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants