Skip to content

Commit

Permalink
refactor: Wrap validator_signer with MutableConfigValue (#11372)
Browse files Browse the repository at this point in the history
Part of: #11264
This PR should be no-op:
- convert `Signer` and `ValidatorSigner` traits into an enum
- wrap `ValidatorSigner` with `MutableConfigValue`

`MutableConfigValue` requires to implement `PartialEq` and `Clone`
traits. These traits are not object safe, thus cannot be derived for
`ValidatorSigner` trait. We need to convert `ValidatorSigner` trait into
an enum, similarly `Signer` trait.
That's also the solution recommended by Rust:
rust-lang/rust#80194

Unfortunately, this change requires a change in enormous number of
places, because the existing code mostly used
`InMemory(Validator)Signer` directly instead of `dyn (Validator)Signer`.
To minimize changes, I added traits like
`From<InMemoryValidatorSigner> for ValidatorSigner` so that it suffices
to add `.into()` in most cases.
  • Loading branch information
staffik committed Jun 10, 2024
1 parent 1e39d94 commit b8f08d9
Show file tree
Hide file tree
Showing 116 changed files with 822 additions and 596 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions chain/chain/src/doomslug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub struct Doomslug {
endorsement_pending: bool,
/// Information to track the timer (see `start_timer` routine in the paper)
timer: DoomslugTimer,
signer: Option<Arc<dyn ValidatorSigner>>,
signer: Option<Arc<ValidatorSigner>>,
/// How many approvals to have before producing a block. In production should be always `HalfStake`,
/// but for many tests we use `NoApprovals` to invoke more forkfulness
threshold_mode: DoomslugThresholdMode,
Expand Down Expand Up @@ -362,7 +362,7 @@ impl Doomslug {
min_delay: Duration,
delay_step: Duration,
max_delay: Duration,
signer: Option<Arc<dyn ValidatorSigner>>,
signer: Option<Arc<ValidatorSigner>>,
threshold_mode: DoomslugThresholdMode,
) -> Self {
Doomslug {
Expand Down
49 changes: 28 additions & 21 deletions chain/chain/src/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ use primitive_types::U256;

fn stake(
nonce: Nonce,
signer: &dyn Signer,
sender: &dyn ValidatorSigner,
signer: &Signer,
sender: &ValidatorSigner,
stake: Balance,
) -> SignedTransaction {
SignedTransaction::from_actions(
Expand Down Expand Up @@ -438,13 +438,15 @@ fn test_validator_rotation() {
let block_producers: Vec<_> =
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signer =
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
.into();
// test1 doubles stake and the new account stakes the same, so test2 will be kicked out.`
let staking_transaction = stake(1, &signer, &block_producers[0], TESTING_INIT_STAKE * 2);
let new_account = AccountId::try_from(format!("test{}", num_nodes + 1)).unwrap();
let new_validator = create_test_signer(new_account.as_str());
let new_signer =
InMemorySigner::from_seed(new_account.clone(), KeyType::ED25519, new_account.as_ref());
let new_signer: Signer =
InMemorySigner::from_seed(new_account.clone(), KeyType::ED25519, new_account.as_ref())
.into();
let create_account_transaction = SignedTransaction::create_account(
2,
block_producers[0].validator_id().clone(),
Expand All @@ -462,7 +464,8 @@ fn test_validator_rotation() {
validators[1].clone(),
KeyType::ED25519,
validators[1].as_ref(),
);
)
.into();
vec![
staking_transaction,
create_account_transaction,
Expand Down Expand Up @@ -524,7 +527,8 @@ fn test_validator_stake_change() {
let block_producers: Vec<_> =
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signer =
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
.into();

let desired_stake = 2 * TESTING_INIT_STAKE / 3;
let staking_transaction = stake(1, &signer, &block_producers[0], desired_stake);
Expand Down Expand Up @@ -561,7 +565,7 @@ fn test_validator_stake_change_multiple_times() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();

let staking_transaction = stake(1, &signers[0], &block_producers[0], TESTING_INIT_STAKE - 1);
Expand Down Expand Up @@ -656,7 +660,7 @@ fn test_stake_in_last_block_of_an_epoch() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();
let staking_transaction =
stake(1, &signers[0], &block_producers[0], TESTING_INIT_STAKE + TESTING_INIT_STAKE / 6);
Expand Down Expand Up @@ -717,7 +721,8 @@ fn test_state_sync() {
let block_producers: Vec<_> =
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signer =
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
.into();
let staking_transaction = stake(1, &signer, &block_producers[0], TESTING_INIT_STAKE + 1);
env.step_default(vec![staking_transaction]);
env.step_default(vec![]);
Expand Down Expand Up @@ -806,7 +811,8 @@ fn test_get_validator_info() {
let block_producers: Vec<_> =
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signer =
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
.into();
let staking_transaction = stake(1, &signer, &block_producers[0], 0);
let mut expected_blocks = [0, 0];
let mut expected_chunks = [0, 0];
Expand Down Expand Up @@ -1047,7 +1053,8 @@ fn test_double_sign_challenge_not_all_slashed() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();

let signer =
InMemorySigner::from_seed(validators[2].clone(), KeyType::ED25519, validators[2].as_ref());
InMemorySigner::from_seed(validators[2].clone(), KeyType::ED25519, validators[2].as_ref())
.into();
let staking_transaction = stake(1, &signer, &block_producers[2], TESTING_INIT_STAKE / 3);
env.step(
vec![vec![staking_transaction]],
Expand Down Expand Up @@ -1223,7 +1230,7 @@ fn test_fishermen_stake() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();
let fishermen_stake = 3300 * NEAR_BASE + 1;

Expand Down Expand Up @@ -1290,7 +1297,7 @@ fn test_fishermen_unstake() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();
let fishermen_stake = 3300 * NEAR_BASE + 1;

Expand Down Expand Up @@ -1367,15 +1374,15 @@ fn test_delete_account_after_unstake() {
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.collect();

let staking_transaction1 = stake(1, &signers[1], &block_producers[1], 0);
let staking_transaction1 = stake(1, &signers[1].clone().into(), &block_producers[1], 0);
env.step_default(vec![staking_transaction1]);
let account = env.view_account(block_producers[1].validator_id());
assert_eq!(account.amount, TESTING_INIT_BALANCE - TESTING_INIT_STAKE);
assert_eq!(account.locked, TESTING_INIT_STAKE);
for _ in 2..=5 {
env.step_default(vec![]);
}
let staking_transaction2 = stake(2, &signers[1], &block_producers[1], 1);
let staking_transaction2 = stake(2, &signers[1].clone().into(), &block_producers[1], 1);
env.step_default(vec![staking_transaction2]);
for _ in 7..=13 {
env.step_default(vec![]);
Expand All @@ -1387,7 +1394,7 @@ fn test_delete_account_after_unstake() {
4,
signers[1].account_id.clone(),
signers[1].account_id.clone(),
&signers[1] as &dyn Signer,
&signers[1].clone().into(),
vec![Action::DeleteAccount(DeleteAccountAction {
beneficiary_id: signers[0].account_id.clone(),
})],
Expand All @@ -1412,7 +1419,7 @@ fn test_proposal_deduped() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();

let staking_transaction1 = stake(1, &signers[1], &block_producers[1], TESTING_INIT_STAKE - 100);
Expand All @@ -1433,7 +1440,7 @@ fn test_insufficient_stake() {
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
let signers: Vec<_> = validators
.iter()
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
.collect();

let staking_transaction1 = stake(1, &signers[1], &block_producers[1], 100);
Expand Down Expand Up @@ -1481,7 +1488,7 @@ fn test_trie_and_flat_state_equality() {
4,
signers[0].account_id.clone(),
validators[1].clone(),
&signers[0] as &dyn Signer,
&signers[0].clone().into(),
vec![Action::Transfer(TransferAction { deposit: 10 })],
// runtime does not validate block history
CryptoHash::default(),
Expand Down Expand Up @@ -1569,7 +1576,7 @@ fn generate_transaction_pool(
round.try_into().unwrap(),
signers[i].account_id.clone(),
signers[(i + round) % signer_count].account_id.clone(),
&signers[i] as &dyn Signer,
&signers[i].clone().into(),
round.try_into().unwrap(),
block_hash,
);
Expand Down
6 changes: 3 additions & 3 deletions chain/chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use near_primitives::hash::CryptoHash;
use near_primitives::test_utils::create_test_signer;
use near_primitives::types::{AccountId, NumBlocks, NumShards};
use near_primitives::utils::MaybeValidated;
use near_primitives::validator_signer::InMemoryValidatorSigner;
use near_primitives::validator_signer::ValidatorSigner;
use near_primitives::version::PROTOCOL_VERSION;
use near_store::genesis::initialize_genesis_state;
use near_store::test_utils::create_test_store;
Expand Down Expand Up @@ -120,15 +120,15 @@ pub fn process_block_sync(
// TODO(#8190) Improve this testing API.
pub fn setup(
clock: Clock,
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<InMemoryValidatorSigner>) {
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<ValidatorSigner>) {
setup_with_tx_validity_period(clock, 100, 1000)
}

pub fn setup_with_tx_validity_period(
clock: Clock,
tx_validity_period: NumBlocks,
epoch_length: u64,
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<InMemoryValidatorSigner>) {
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<ValidatorSigner>) {
let store = create_test_store();
let mut genesis = Genesis::test_sharded(
clock.clone(),
Expand Down
4 changes: 2 additions & 2 deletions chain/chain/src/tests/garbage_collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use near_primitives::merkle::PartialMerkleTree;
use near_primitives::shard_layout::ShardUId;
use near_primitives::test_utils::{create_test_signer, TestBlockBuilder};
use near_primitives::types::{BlockHeight, NumBlocks, StateRoot};
use near_primitives::validator_signer::InMemoryValidatorSigner;
use near_primitives::validator_signer::ValidatorSigner;
use near_store::test_utils::gen_changes;
use near_store::{DBCol, ShardTries, Trie, WrappedTrieChanges};

Expand Down Expand Up @@ -730,7 +730,7 @@ fn add_block(
epoch_manager: &dyn EpochManagerAdapter,
prev_block: &mut Block,
blocks: &mut Vec<Block>,
signer: Arc<InMemoryValidatorSigner>,
signer: Arc<ValidatorSigner>,
height: u64,
) {
let next_epoch_id = epoch_manager
Expand Down
2 changes: 1 addition & 1 deletion chain/chain/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ mod tests {
nonce,
account_id,
"bob".parse().unwrap(),
&signer,
&signer.into(),
10,
CryptoHash::default(),
)
Expand Down
2 changes: 1 addition & 1 deletion chain/chunks/src/chunk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ mod tests {
CryptoHash::default(),
CryptoHash::default(),
vec![],
&signer,
&signer.into(),
))
}

Expand Down
2 changes: 1 addition & 1 deletion chain/chunks/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ mod tests {
nonce,
signer_id.clone(),
receiver_id.clone(),
&signer,
&signer.into(),
deposit,
CryptoHash::default(),
);
Expand Down
2 changes: 1 addition & 1 deletion chain/chunks/src/shards_manager_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1977,7 +1977,7 @@ impl ShardsManagerActor {
prev_outgoing_receipts_root: CryptoHash,
tx_root: CryptoHash,
congestion_info: Option<CongestionInfo>,
signer: &dyn ValidatorSigner,
signer: &ValidatorSigner,
rs: &ReedSolomon,
protocol_version: ProtocolVersion,
) -> Result<(EncodedShardChunk, Vec<MerklePath>), Error> {
Expand Down
2 changes: 1 addition & 1 deletion chain/client/src/chunk_distribution_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ mod tests {
hash(&[height.to_le_bytes().as_slice(), shard_id.to_le_bytes().as_slice()].concat());
let mut mock_hashes = MockHashes::new(prev_block_hash);

let signer = EmptyValidatorSigner::default();
let signer = EmptyValidatorSigner::default().into();
let header_inner = ShardChunkHeaderInner::V3(ShardChunkHeaderInnerV3 {
prev_block_hash,
prev_state_root: mock_hashes.next().unwrap(),
Expand Down
Loading

0 comments on commit b8f08d9

Please sign in to comment.