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

Fetch revert reason for sendSignedTransaction #3420

Merged
merged 5 commits into from
Apr 7, 2020
Merged

Fetch revert reason for sendSignedTransaction #3420

merged 5 commits into from
Apr 7, 2020

Conversation

cgewecke
Copy link
Collaborator

@cgewecke cgewecke commented Mar 17, 2020

Description

Adds revert reason support to sendSignedTransaction (eth_sendRawTransaction).

Fetches transaction object for the failed tx and synthesizes unsigned tx params for it so it can be replayed as an eth_call whose return data is parsed as a revert reason.

Fixes #3345

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success.
  • I have executed npm run test:cov and my test cases do cover all lines and branches of the added code.
  • I ran npm run build-all and tested the resulting file/'s from dist folder in a browser.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have tested my code on the live network.

@cgewecke cgewecke added 1.x 1.0 related issues Review Needed Maintainer(s) need to review labels Mar 17, 2020
@coveralls
Copy link

coveralls commented Mar 17, 2020

Coverage Status

Coverage increased (+0.01%) to 88.596% when pulling a76c08d on issue/3345 into 0ffac4b on 1.x.

Copy link
Collaborator

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

LGTM, and looks like travis reported the test successfully 👍 sendSignedTransaction reverts with reason (53ms)

@cgewecke
Copy link
Collaborator Author

cgewecke commented Apr 7, 2020

Thanks @ryanio!

@cgewecke cgewecke merged commit 2a5c5cb into 1.x Apr 7, 2020
@cgewecke cgewecke deleted the issue/3345 branch April 7, 2020 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Review Needed Maintainer(s) need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend transaction revert handling for eth.sendSignedTransaction
3 participants