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

Support "pending" parameter for Web3 getTransactionCount #696

Merged
merged 3 commits into from
Nov 8, 2018

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Nov 6, 2018

Notice

It is not allowed to submit your PR to the master branch directly, please submit your PR to the master-pre-merge branch.

Description

The ethereum Web3 allows most functions that take a block number as parameter to also take the special keywords "pending", "latest", or "earliest". The current Web3 implementation of getTransactionCount errors out if it is called with "pending".

Note: It might be important to go through the other Web3 functions that take a block number as a parameter and make sure we support these 3 special parameters.

Fixes Issue #695.

Type of change

Insert x into the following checkboxes to confirm (eg. [x]):

  • Bug fix.
  • New feature.
  • Enhancement.
  • Unit test.
  • Breaking change (a fix or feature that causes existing functionality to not work as expected).
  • Requires documentation update.

Testing

Manually tested that the Web3 API now correctly responds to requests such as

eth.getTransactionCount("address", "pending").then(console.log)

Verification

Insert x into the following checkboxes to confirm (eg. [x]):

  • I have self-reviewed my own code and conformed to the style guidelines of this project.
  • New and existing tests pass locally with my changes.
  • I have added tests for my fix or feature.
  • I have made appropriate changes to the corresponding documentation.
  • My code generates no new warnings.
  • Any dependent changes have been made.

Copy link
Collaborator

@AionJayT AionJayT left a comment

Choose a reason for hiding this comment

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

can you write a testcase for it?

@arajasek
Copy link
Contributor Author

arajasek commented Nov 6, 2018

Added a testcase. This test should be moved into the new file created by PR #691 when that is merged.

@AlexandraRoatis AlexandraRoatis added this to the 0.3.2 milestone Nov 6, 2018
@AlexandraRoatis AlexandraRoatis added the enhancement New feature or request label Nov 6, 2018
@AionJayT AionJayT merged commit cc46a2c into master-pre-merge Nov 8, 2018
@arajasek arajasek deleted the getTxsCountPending branch November 8, 2018 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants