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

Optimize vote distribution between PRs and non-PRs #4766

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

gr0vity-dev
Copy link
Contributor

Currently, PRs receive votes both from flood_vote and flood_vote_pr, leading to duplicate voting, while fewer non-PR receive votes. This is particularly noticeable in small test networks.

This change ensures votes are distributed more efficiently:

  • PRs receive votes only through flood_vote_pr
  • Non-PRs receive votes through flood_vote_non_pr
  • Overall outgoing vote load remains unchanged

Here is the difference seen in a local test network:

Screenshot 2024-10-24 at 20 38 12

I included a small optimization to nano::network::list_non_pr since it's called often.

Test Configuration & Benchmark Results:
PR Count: 100
Non-PR Count: 200
Iterations: 10000

--------------------------------------------------
Current method   - Duration: -- 521 ms
Partition method - Duration: -- 354 ms
--------------------------------------------------

Target count per iteration: 29 [ceil( 2* sqrt(200))]
Actual Current method:      29
Actual Partition method:    29

gr0vity-dev added 2 commits October 24, 2024 20:19
This avoids potential duplicate votes to PRs and better confirmation times for non PRs
@gr0vity-dev-bot
Copy link

gr0vity-dev-bot commented Oct 24, 2024

Test Results for Commit 5bc04e8

Pull Request 4766: Results
Overall Status:

Test Case Results

  • 5n4pr_conf_10k_bintree: PASS (Duration: 110s)
  • 5n4pr_conf_10k_change: PASS (Duration: 212s)
  • 5n4pr_conf_change_dependant: PASS (Duration: 149s)
  • 5n4pr_conf_change_independant: PASS (Duration: 158s)
  • 5n4pr_conf_send_dependant: PASS (Duration: 136s)
  • 5n4pr_conf_send_independant: PASS (Duration: 140s)
  • 5n4pr_rocks_10k_bintree: PASS (Duration: 109s)
  • 5n4pr_rocks_10k_change: PASS (Duration: 219s)

Last updated: 2024-10-24 20:55:44 UTC

@pwojcikdev
Copy link
Contributor

Looks good. Do I understand correctly that before votes were being sent twice to PRs?

@gr0vity-dev
Copy link
Contributor Author

Looks good. Do I understand correctly that before votes were being sent twice to PRs?

Not systematically but they were sent:

  • to all PRs in flood_vote_pr
  • to 2 * sqrt of all peers (including PRs) in flood_vote

This means that some PRs might get the same votes twice

@pwojcikdev pwojcikdev merged commit e1a4fcb into nanocurrency:develop Oct 25, 2024
28 checks passed
RickiNano pushed a commit to RickiNano/nano-node that referenced this pull request Oct 25, 2024
* Publish votes to all PRs and a subset of non PRs

This avoids potential duplicate votes to PRs and better confirmation times for non PRs

* small optimisation for list_non_pr

---------

Co-authored-by: gr0vity-dev <homebot@users.noreply.github.com>
@qwahzi qwahzi added this to the V28 milestone Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress / V28.0
Development

Successfully merging this pull request may close these issues.

4 participants