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

Validator health status #3207

Open
wants to merge 3 commits into
base: albatross
Choose a base branch
from
Open

Validator health status #3207

wants to merge 3 commits into from

Conversation

viquezclaudio
Copy link
Member

What's in this pull request?

...

This fixes #___.

Pull request checklist

  • All tests pass. The project builds and runs.
  • I have resolved any merge conflicts.
  • I have resolved all clippy and rustfmt warnings.

@viquezclaudio viquezclaudio force-pushed the viquezcl/validator branch 2 times, most recently from f08b8d3 to 8108a5b Compare December 23, 2024 15:18
Track the number of blocks that are succesfully produced when changing the validator health
@viquezclaudio viquezclaudio marked this pull request as ready for review December 26, 2024 17:37
@@ -113,7 +113,7 @@ where
let (v, c) = build_validator(
peer_ids[i],
Address::from(&validator_keys[i]),
false,
true,
Copy link
Member

Choose a reason for hiding this comment

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

Make automatic_reactivate a fn param for build_validators so it doesn't influence other tests?

/// The current validator health
pub health: ValidatorHealth,
/// Number of blocks that we have produced in time(without being inactivated)
pub blk_cnt: u32,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub blk_cnt: u32,
pub block_counter: u32,

@@ -181,6 +198,8 @@ impl<TValidatorNetwork: ValidatorNetwork + 'static> NextProduceMicroBlockEvent<T
continue;
}

self.health_state.write().blk_cnt += 1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.health_state.write().blk_cnt += 1;
self.health_state.write().blk_cnt.saturating_add(1);

Prevent overflow

Comment on lines +168 to +170
log::warn!(block = block.block_number(), "Not publishing block");
let event = ProduceMicroBlockEvent::MicroBlock;
break Some(Some(event));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
log::warn!(block = block.block_number(), "Not publishing block");
let event = ProduceMicroBlockEvent::MicroBlock;
break Some(Some(event));
log::warn!(block = block.block_number(), "Not publishing block");
break Some(Some(ProduceMicroBlockEvent::MicroBlock));

pub health: ValidatorHealth,
/// Number of blocks that we have produced in time(without being inactivated)
pub blk_cnt: u32,
/// For testing/debug purposes control wether produced blocks are published by the validator
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// For testing/debug purposes control wether produced blocks are published by the validator
/// For testing/debug purposes control whether produced blocks are published by the validator

pub struct HealthState {
/// The current validator health
pub health: ValidatorHealth,
/// Number of blocks that we have produced in time(without being inactivated)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Number of blocks that we have produced in time(without being inactivated)
/// Number of subsequent blocks that we have been produced without being inactivated

good_blocks = %self.health_state.read().blk_cnt,
"Current validator health is yellow",
);
if self.health_state.read().blk_cnt >= VALIDATOR_HEALTH_THRESHOLD {
Copy link
Member

Choose a reason for hiding this comment

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

This logic doesn't seem to match with the original description: If the validator is Yellow and is not deactivated in a quarter of an epoch, we change its status to Green.

inactivated = red_block_number,
"Current validator health is red",
);
if self.health_state.read().blk_cnt >= VALIDATOR_HEALTH_THRESHOLD {
Copy link
Member

Choose a reason for hiding this comment

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

This logic doesn't seem to match with the original description: If the validator is Red and is not deactivated in one epoch, we change its status to Yellow.

Comment on lines 927 to 928
let validator_health = self.health_state.read().health;
match validator_health {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let validator_health = self.health_state.read().health;
match validator_health {
match self.health_state.read().health {

}
ValidatorHealth::Red(_) => {
log::warn!(
"The validator needs human intervention, no automatic reactivate"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"The validator needs human intervention, no automatic reactivate"
"The validator needs human intervention, no automatic reactivation"

@viquezclaudio viquezclaudio added the WIP This pull request is still work in progress label Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP This pull request is still work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants