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

Solve activity spam #209

Merged
merged 3 commits into from
Sep 26, 2023
Merged

Solve activity spam #209

merged 3 commits into from
Sep 26, 2023

Conversation

Duddino
Copy link
Member

@Duddino Duddino commented Sep 25, 2023

Abstract

Activity was being synced multiple times per block. This is because Network::getBlockCount resolves the promise immediately (This may have changed, i'm not sure).
Added a new event, 'new-block', and moved the console.log and the activityUpdate there
image
Example of the amount of times Activity::update was called on master, using the latest state of the art profiling tools

Testing

  • Test that the network syncs correctly
  • Test that the switch to testnet works correctly

@Duddino Duddino self-assigned this Sep 25, 2023
@Duddino Duddino added Bug This is either a bugfix (PR) or a bug (issue). Awaiting Review This PR and/or issue is awaiting reviews before continuing. labels Sep 25, 2023
Copy link
Member

@JSKitty JSKitty left a comment

Choose a reason for hiding this comment

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

Going to give a tACK 81bbf15

Minor nit: I think the reason I allowed syncing history between blocks, is because Blockbook has this little 'delay' or 'cache' of block heights: I'm sure I tested previously that the history updates quicker than the block count on Blockbook's APIs, so this PR slightly slows down the activity and balance 'reactivity', my TX took 2 minutes before showing, when it usually should take 1 minute (60s).

Copy link
Member

@panleone panleone left a comment

Choose a reason for hiding this comment

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

tACK 81bbf15
Minimizing the number of network requests is always a good thing

Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK 81bbf15

@JSKitty JSKitty merged commit 8fcdd79 into PIVX-Labs:master Sep 26, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Review This PR and/or issue is awaiting reviews before continuing. Bug This is either a bugfix (PR) or a bug (issue).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants