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

Dynamically remove validator key #12011

Merged
merged 3 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions chain/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ impl Client {

/// Updates client's mutable validator signer.
/// It will update all validator signers that synchronize with it.
pub(crate) fn update_validator_signer(&self, signer: Arc<ValidatorSigner>) -> bool {
self.validator_signer.update(Some(signer))
pub(crate) fn update_validator_signer(&self, signer: Option<Arc<ValidatorSigner>>) -> bool {
self.validator_signer.update(signer)
}
}

Expand Down
9 changes: 4 additions & 5 deletions chain/client/src/client_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,11 +1208,10 @@ impl ClientActorInner {
);

if update_result.validator_signer_updated {
let validator_signer =
self.client.validator_signer.get().expect("Validator signer just updated");

check_validator_tracked_shards(&self.client, validator_signer.validator_id())
.expect("Could not check validator tracked shards");
if let Some(validator_signer) = self.client.validator_signer.get() {
check_validator_tracked_shards(&self.client, validator_signer.validator_id())
.expect("Could not check validator tracked shards");
}

// Request PeerManager to advertise tier1 proxies.
// It is needed to advertise that our validator key changed.
Expand Down
2 changes: 1 addition & 1 deletion chain/client/src/config_updater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl ConfigUpdater {
pub fn try_update(
&mut self,
update_client_config_fn: &dyn Fn(UpdateableClientConfig) -> bool,
update_validator_signer_fn: &dyn Fn(Arc<ValidatorSigner>) -> bool,
update_validator_signer_fn: &dyn Fn(Option<Arc<ValidatorSigner>>) -> bool,
) -> ConfigUpdaterResult {
let mut update_result = ConfigUpdaterResult::default();
while let Ok(maybe_updateable_configs) = self.rx_config_update.try_recv() {
Expand Down
4 changes: 3 additions & 1 deletion core/dyn-configs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ pub struct UpdateableConfigs {
/// Contents of the `config.json` corresponding to the mutable fields of `ClientConfig`.
pub client_config: Option<UpdateableClientConfig>,
/// Validator key hot loaded from file.
pub validator_signer: Option<Arc<ValidatorSigner>>,
/// `None` means that the validator key existence could not be determined.
/// `Some(None)` means that it was determined that the validator key does not exist.
pub validator_signer: Option<Option<Arc<ValidatorSigner>>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if it makes sense to use a custom enum here instead. Parsing Option<Option<Foo>> feels too cumbersome compared to having a enum with specific variants.

}

/// Pushes the updates to listeners.
Expand Down
11 changes: 7 additions & 4 deletions nearcore/src/dyn_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@ pub fn read_updateable_configs(
let updateable_client_config = config.as_ref().map(get_updateable_client_config);

let validator_signer = if let Some(config) = config {
read_validator_key(home_dir, &config).unwrap_or_else(|err| {
errs.push(err);
None
})
match read_validator_key(home_dir, &config) {
Ok(validator_key) => Some(validator_key),
Err(err) => {
errs.push(err);
None
}
}
} else {
None
};
Expand Down
2 changes: 2 additions & 0 deletions nightly/pytest-sanity.txt
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ pytest --timeout=240 sanity/switch_node_key.py
pytest --timeout=240 sanity/switch_node_key.py --features nightly
pytest --timeout=120 sanity/validator_switch_key_quick.py
pytest --timeout=120 sanity/validator_switch_key_quick.py --features nightly
pytest --timeout=120 sanity/validator_remove_key_quick.py
pytest --timeout=120 sanity/validator_remove_key_quick.py --features nightly
pytest --timeout=60 sanity/shadow_tracking.py
pytest --timeout=60 sanity/shadow_tracking.py --features nightly
pytest sanity/proxy_simple.py
Expand Down
8 changes: 8 additions & 0 deletions pytest/lib/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,14 @@ def reset_validator_key(self, new_key):
with open(os.path.join(self.node_dir, "validator_key.json"), 'w+') as f:
json.dump(new_key.to_json(), f)

def remove_validator_key(self):
logger.info(
f"Removing validator_key.json file for node {self.ordinal}.")
self.validator_key = None
file_path = os.path.join(self.node_dir, "validator_key.json")
if os.path.exists(file_path):
os.remove(file_path)

def reset_node_key(self, new_key):
self.node_key = new_key
with open(os.path.join(self.node_dir, "node_key.json"), 'w+') as f:
Expand Down
68 changes: 68 additions & 0 deletions pytest/tests/sanity/validator_remove_key_quick.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/usr/bin/env python3
# Starts one validating node and one non-validating node
# Dynamically remove the key from the validator node
# and make sure that the network stalls.
# Dynamically reload the key at the validator node
# and make sure that the network progress again.

import unittest
import sys
import pathlib

sys.path.append(str(pathlib.Path(__file__).resolve().parents[2] / 'lib'))

from configured_logger import logger
from cluster import start_cluster
from utils import wait_for_blocks
import state_sync_lib

EPOCH_LENGTH = 20
TIMEOUT = 100


class ValidatorRemoveKeyQuickTest(unittest.TestCase):

def test_validator_remove_key_quick(self):
logger.info("Validator remove key quick test")
validator_config, rpc_config = state_sync_lib.get_state_sync_configs_pair(
)

validator_config.update({
"tracked_shards": [0],
"store.load_mem_tries_for_tracked_shards": True,
})

rpc_config.update({
"tracked_shards": [0],
})

[validator,
rpc] = start_cluster(1, 1, 1, None,
[["epoch_length", EPOCH_LENGTH],
["block_producer_kickout_threshold", 80],
["chunk_producer_kickout_threshold", 80]], {
0: validator_config,
1: rpc_config
})

wait_for_blocks(rpc, target=EPOCH_LENGTH)
validator_key = validator.validator_key
validator.remove_validator_key()
wait_for_blocks(rpc, target=EPOCH_LENGTH * 2)
validator.reload_updateable_config()
validator.stop_checking_store()
try:
wait_for_blocks(rpc, count=5, timeout=10)
except:
pass
else:
self.fail('Blocks are not supposed to be produced')

validator.reset_validator_key(validator_key)
validator.reload_updateable_config()
validator.stop_checking_store()
wait_for_blocks(rpc, count=EPOCH_LENGTH * 2)


if __name__ == '__main__':
unittest.main()
Loading