Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

fix: waitForTransactionToBeProvable not waiting for blocks #338

Merged
merged 27 commits into from
Feb 17, 2021

Conversation

antouhou
Copy link
Member

@antouhou antouhou commented Feb 8, 2021

Issue being fixed or feature implemented

Fix BlockchainListener#waitForBlocks not waiting for blocks properly. It wasn't uncovered due to another bug in the drive that was fixed in dashevo/js-drive#457

What was done?

  • Reworked waitForBlocks method of BlockchainListener to subscribe to all needed block events at the same time and not one by one
  • Refactoring

How Has This Been Tested?

With tests

Breaking Changes

None

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@antouhou antouhou added this to the v0.18 milestone Feb 8, 2021
shuplenkov
shuplenkov previously approved these changes Feb 9, 2021
Copy link
Member

@shuplenkov shuplenkov left a comment

Choose a reason for hiding this comment

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

👍

shuplenkov
shuplenkov previously approved these changes Feb 9, 2021
Copy link
Member

@shuplenkov shuplenkov left a comment

Choose a reason for hiding this comment

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

👍

@shumkov shumkov changed the title fix: blockchainListener.waitForBlocks not waiting for blocks properly fix: BlockchainListener#waitForBlocks not waiting for blocks properly Feb 9, 2021
@shumkov shumkov changed the title fix: BlockchainListener#waitForBlocks not waiting for blocks properly fix: waitForBlocks not waiting for blocks properly Feb 9, 2021
@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 2 alerts when merging 53f5dc2 into 482617d - view on LGTM.com

new alerts:

  • 2 for Unreachable statement

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 1 alert when merging e22be10 into 482617d - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

# Conflicts:
#	lib/externalApis/tenderdash/blockchainListener/BlockchainListener.js
#	lib/grpcServer/handlers/platform/waitForStateTransitionResultHandlerFactory.js
@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 1 alert when merging 4be94d1 into 482617d - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 1 alert when merging 7bf04fe into 482617d - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 1 alert when merging 6113024 into 482617d - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Feb 16, 2021

This pull request introduces 1 alert when merging 68d082b into 482617d - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@shumkov shumkov changed the title fix: waitForBlocks not waiting for blocks properly fix: waitForTransactionToBeProvable not waiting for blocks properly Feb 16, 2021
@shumkov shumkov changed the title fix: waitForTransactionToBeProvable not waiting for blocks properly fix: waitForTransactionToBeProvable not waiting for blocks Feb 16, 2021
@antouhou antouhou merged commit 8a319d4 into v0.18-dev Feb 17, 2021
@antouhou antouhou deleted the fix-block-wait branch February 17, 2021 13:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants