feat(sequencer)!: add validator update with name action #1608
Conversation
github-actions
bot
added
proto
| @@ -33,6 +33,7 @@ pub enum Action { | |||
| Sequence(SequenceAction), | |||
| Transfer(TransferAction), | |||
| ValidatorUpdate(ValidatorUpdate), | |||
| ValidatorUpdateWithName(ValidatorUpdateWithName), | |||
There was a problem hiding this comment.
Open to calling this action something else if people would like. Maybe something like NamedValidatorUpdate
ethanoroshiba
requested review from
a team,
joroshiba,
SuperFluffy and
noot
as code owners
| #[instrument(skip_all)] | ||
| fn clear_validator_names(&mut self) { | ||
| self.delete(VALIDATOR_NAMES_KEY.to_string()); | ||
| } |
There was a problem hiding this comment.
Seems like this is unused - can it be removed?
| fn put_validator_names(&mut self, validator_names: BTreeMap<String, String>) -> Result<()> { | ||
| self.put_raw( | ||
| VALIDATOR_NAMES_KEY.to_string(), | ||
| serde_json::to_vec(&validator_names).wrap_err("failed to serialize validator names")?, |
There was a problem hiding this comment.
We're now using Borsh to encode everything in storage. Could you update this to follow the approach taken for other types please?
This will involve creating a new type in authority::storage like:
#[derive(Debug, BorshSerialize, BorshDeserialize)]
pub(in crate::authority) struct ValidatorName<'a>(Cow<'a, str>);See the implementation for AddressPrefix for example.
This new type should then be included as a variant in authority::storage::ValueImpl.
There was a problem hiding this comment.
Could we have a new unit test for the added putter/getter please?
| } | ||
| }; | ||
|
|
||
| match validator_names.get(&hex::encode(address.address_bytes())) { |
There was a problem hiding this comment.
Can we avoid hex-encoding the addresses in this collection?
Summary
Added
ValidatorUpdateWithNameaction which wraps aValidatorUpdateand has an additional field for the validator name to be stored app side.Background
Validator names could previously only be set in the CometBFT genesis, this is meant to give us a way to set validator names app-side.
Changes
ValidatorUpdateWithNameaction to proto, core, and sequencer.Testing
ValidatorUpdateWithNameaction.validator_name_requestABCI query.ValidatorUpdateWithNametoapp_execute_transaction_with_every_action_snapshotBreaking Changelist
Related Issues
closes #1590