Skip to content

Refactor wallets::representatives to use nano::locked#5018

Merged
pwojcikdev merged 2 commits intonanocurrency:developfrom
pwojcikdev:wallet-representatives-mutex-refactor
Jan 29, 2026
Merged

Refactor wallets::representatives to use nano::locked#5018
pwojcikdev merged 2 commits intonanocurrency:developfrom
pwojcikdev:wallet-representatives-mutex-refactor

Conversation

@pwojcikdev
Copy link
Copy Markdown
Contributor

No description provided.

@gr0vity-dev-bot
Copy link
Copy Markdown

Test Results for Commit 44caafb

Pull Request 5018: Results
Overall Status:

Test Case Results

  • 5n4pr_conf_10k_bintree: PASS (Duration: 120s)
  • 5n4pr_conf_10k_change: PASS (Duration: 131s)
  • 5n4pr_conf_change_dependant: PASS (Duration: 132s)
  • 5n4pr_conf_change_independant: PASS (Duration: 118s)
  • 5n4pr_conf_send_dependant: PASS (Duration: 128s)
  • 5n4pr_conf_send_independant: PASS (Duration: 133s)
  • 5n4pr_rocks_10k_bintree: PASS (Duration: 126s)
  • 5n4pr_rocks_10k_change: PASS (Duration: 223s)

Last updated: 2026-01-29 17:56:40 UTC

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors wallet representative tracking to use nano::locked instead of manually managed mutex+container pairs, and simplifies representative qualification checks.

Changes:

  • Replace wallet and wallets representative containers/mutexes with nano::locked wrappers.
  • Simplify wallets::check_rep API and move the shared logic into check_rep_impl.
  • Update unit tests to use the new wallet::reps() accessor instead of direct mutex/container access.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
nano/node/wallet.hpp Switch representative members to nano::locked; add wallet::reps(); update wallets::check_rep signature and add check_rep_impl.
nano/node/wallet.cpp Update representative insertion/scanning logic to use nano::locked; refactor check_rep into check_rep + check_rep_impl.
nano/core_test/wallets.cpp Adjust tests to use wallet::reps() instead of accessing internal mutex/container.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1802 to 1807
bool nano::wallets::check_rep (nano::account const & account)
{
auto half_principal_weight = node.minimum_principal_weight () / 2;
auto representatives_locked = representatives.lock ();
return check_rep_impl (*representatives_locked, account, half_principal_weight);
}
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

wallets::check_rep() acquires the representatives lock before calling ledger.weight() inside check_rep_impl(). This increases lock hold time and contention compared to the prior pattern where the weight/vote_minimum check happened before locking. Consider computing weight (and early-returning for non-reps) before taking the representatives lock, and only locking for the insert / counter / half_principal update (e.g., pass weight into check_rep_impl, or split into a no-lock precheck + locked update).

Copilot uses AI. Check for mistakes.
@pwojcikdev pwojcikdev merged commit a995599 into nanocurrency:develop Jan 29, 2026
33 of 34 checks passed
@pwojcikdev pwojcikdev deleted the wallet-representatives-mutex-refactor branch January 29, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants