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

Add events sending and sent to tx submission, add arg latestBlockHash to event confirmation #3550

Merged
merged 7 commits into from
Jun 2, 2020

Conversation

ryanio
Copy link
Collaborator

@ryanio ryanio commented May 29, 2020

This PR:

  1. Adds events to the send tx promievent: sending and sent.
  2. Adds third arg to confirmation: latestBlockHash.

Fixes #3438

Type of change

  • New feature (non-breaking change which adds functionality)

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 ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build-all and tested the resulting files 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.

@ryanio ryanio added Enhancement Includes improvements or optimizations Feature Request 1.x 1.0 related issues labels May 29, 2020
@ryanio ryanio marked this pull request as draft May 29, 2020 02:31
add arg `latestBlockHash` to `confirmation`
@ryanio ryanio changed the title Add events to transaction submission Add events sending and sent to transaction submission, add latestBlockHash to confirmation Jun 1, 2020
@coveralls
Copy link

coveralls commented Jun 2, 2020

Coverage Status

Coverage increased (+0.04%) to 89.779% when pulling 3cef72c on issue/3438 into 3107c98 on 1.x.

@ryanio ryanio marked this pull request as ready for review June 2, 2020 00:24
@ryanio ryanio added the Review Needed Maintainer(s) need to review label Jun 2, 2020
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

This is super cool! Just left a couple questions.

ryanio added 2 commits June 2, 2020 10:42
move 'sending' to after any intermediate requests
@ryanio ryanio changed the title Add events sending and sent to transaction submission, add latestBlockHash to confirmation Add events sending and sent to tx submission, add arg latestBlockHash to event confirmation Jun 2, 2020
Copy link
Collaborator

@cgewecke cgewecke left a comment

Choose a reason for hiding this comment

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

This looks perfect to me 🎉

The payload object seems good. The formatting's raw which is good for seeing what actually gets sent over the wire.

An example from the deploy test:

{
 "method": "eth_sendTransaction",
 "params": [
  {
   "from": "0x4f861a99cf97d053ccb97f9fd5acd5ce2f570171",
   "data": "0x6080604.....",
   "gasPrice": "0x1",
   "gas": "0x3d0900"
  }
 ]
}

once(
type: 'transactionHash',
handler: (receipt: string) => void
handler: (transactionHash: string) => void
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 Nice catch.

@ryanio ryanio merged commit 242bfd0 into 1.x Jun 2, 2020
@ryanio
Copy link
Collaborator Author

ryanio commented Jun 2, 2020

great, thanks!

@holgerd77 holgerd77 deleted the issue/3438 branch June 3, 2020 10:07
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 Enhancement Includes improvements or optimizations Feature Request Review Needed Maintainer(s) need to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add events to transaction submission in support of performance testing
3 participants