Skip to content

fix(elections): source validator snapshot from current vset#101

Merged
Keshoid merged 3 commits into
release/nodectl/v0.4.0from
feature/sma-79-validator-snapshot-uses-adnl-address-from-current-elections
Apr 17, 2026
Merged

fix(elections): source validator snapshot from current vset#101
Keshoid merged 3 commits into
release/nodectl/v0.4.0from
feature/sma-79-validator-snapshot-uses-adnl-address-from-current-elections

Conversation

@Keshoid
Copy link
Copy Markdown
Contributor

@Keshoid Keshoid commented Apr 17, 2026

Summary

  • Bug: build_validators_snapshot sourced adnl, pubkey, key_id, key_election_id, key_expires_at, is_key_active, and stake from elections/bid data instead of the active validator set — showing stale or incorrect values when a node is validating AND has submitted a new election bid
  • Fix: All vset-related snapshot fields now come from the current validator set (p34) and past_elections.frozen_map. Extended find_validator_entries to return the matched ValidatorEntry from the node's config. Fields are None when the node is not in the validator set.
  • Tests: Added test_validator_snapshot_uses_vset_data and test_validator_snapshot_none_when_not_in_vset; updated existing test_publish_snapshot_validators

Closes SMA-79

Copilot AI review requested due to automatic review settings April 17, 2026 13:42
@linear
Copy link
Copy Markdown

linear Bot commented Apr 17, 2026

…napshot-uses-adnl-address-from-current-elections
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

Fixes validator snapshot generation so vset-derived fields (pubkey/adnl/key metadata/stake) are sourced from the current validator set and past_elections.frozen_map, avoiding stale election/bid data when a node is validating while also submitting a new bid (SMA-79).

Changes:

  • Update build_validators_snapshot to populate validator-related snapshot fields exclusively from the current vset (p34) + past_elections.frozen_map, returning None when the node is not in the vset.
  • Extend find_validator_entries to return the matched ValidatorEntry (and associated election id) used to correlate node config with the vset.
  • Add/adjust tests to verify vset sourcing and None behavior when not in vset.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/node-control/elections/src/runner.rs Changes snapshot construction to use vset + frozen stake data; extends find_validator_entries return shape to carry matched config entry data.
src/node-control/elections/src/runner_tests.rs Updates existing snapshot test expectations and adds new tests covering vset-sourced snapshot fields and None fields when not in vset.

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

Comment on lines 1515 to +1519
&& let Some(idx) =
vset.list().iter().position(|item| item.public_key.as_slice() == &key)
{
current_entry = Some((u16::try_from(idx)?, vset.list()[idx].clone()));
current_entry =
Some((u16::try_from(idx)?, vset.list()[idx].clone(), *election_id, entry.clone()));
Copy link

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

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

In find_validator_entries, the exported public key is copied into a [u8; 32] using copy_from_slice earlier in the loop. If the provider returns a non-32-byte value, this will panic and crash snapshot building. Consider validating the length and returning an error (or skipping the entry with a warning) instead of panicking.

Copilot uses AI. Check for mistakes.
@Keshoid Keshoid merged commit 075074b into release/nodectl/v0.4.0 Apr 17, 2026
6 checks passed
@Keshoid Keshoid deleted the feature/sma-79-validator-snapshot-uses-adnl-address-from-current-elections branch April 17, 2026 14:53
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