Skip to content

Commit aace5dd

Browse files
committed
refactor: Wrap validator_signer with MutableConfigValue (#11372)
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.
1 parent 69bd80a commit aace5dd

File tree

116 files changed

+822
-596
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

116 files changed

+822
-596
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

chain/chain/src/doomslug.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ pub struct Doomslug {
138138
endorsement_pending: bool,
139139
/// Information to track the timer (see `start_timer` routine in the paper)
140140
timer: DoomslugTimer,
141-
signer: Option<Arc<dyn ValidatorSigner>>,
141+
signer: Option<Arc<ValidatorSigner>>,
142142
/// How many approvals to have before producing a block. In production should be always `HalfStake`,
143143
/// but for many tests we use `NoApprovals` to invoke more forkfulness
144144
threshold_mode: DoomslugThresholdMode,
@@ -362,7 +362,7 @@ impl Doomslug {
362362
min_delay: Duration,
363363
delay_step: Duration,
364364
max_delay: Duration,
365-
signer: Option<Arc<dyn ValidatorSigner>>,
365+
signer: Option<Arc<ValidatorSigner>>,
366366
threshold_mode: DoomslugThresholdMode,
367367
) -> Self {
368368
Doomslug {

chain/chain/src/runtime/tests.rs

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ use primitive_types::U256;
4848

4949
fn stake(
5050
nonce: Nonce,
51-
signer: &dyn Signer,
52-
sender: &dyn ValidatorSigner,
51+
signer: &Signer,
52+
sender: &ValidatorSigner,
5353
stake: Balance,
5454
) -> SignedTransaction {
5555
SignedTransaction::from_actions(
@@ -438,13 +438,15 @@ fn test_validator_rotation() {
438438
let block_producers: Vec<_> =
439439
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
440440
let signer =
441-
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
441+
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
442+
.into();
442443
// test1 doubles stake and the new account stakes the same, so test2 will be kicked out.`
443444
let staking_transaction = stake(1, &signer, &block_producers[0], TESTING_INIT_STAKE * 2);
444445
let new_account = AccountId::try_from(format!("test{}", num_nodes + 1)).unwrap();
445446
let new_validator = create_test_signer(new_account.as_str());
446-
let new_signer =
447-
InMemorySigner::from_seed(new_account.clone(), KeyType::ED25519, new_account.as_ref());
447+
let new_signer: Signer =
448+
InMemorySigner::from_seed(new_account.clone(), KeyType::ED25519, new_account.as_ref())
449+
.into();
448450
let create_account_transaction = SignedTransaction::create_account(
449451
2,
450452
block_producers[0].validator_id().clone(),
@@ -462,7 +464,8 @@ fn test_validator_rotation() {
462464
validators[1].clone(),
463465
KeyType::ED25519,
464466
validators[1].as_ref(),
465-
);
467+
)
468+
.into();
466469
vec![
467470
staking_transaction,
468471
create_account_transaction,
@@ -524,7 +527,8 @@ fn test_validator_stake_change() {
524527
let block_producers: Vec<_> =
525528
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
526529
let signer =
527-
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
530+
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
531+
.into();
528532

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

567571
let staking_transaction = stake(1, &signers[0], &block_producers[0], TESTING_INIT_STAKE - 1);
@@ -656,7 +660,7 @@ fn test_stake_in_last_block_of_an_epoch() {
656660
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
657661
let signers: Vec<_> = validators
658662
.iter()
659-
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
663+
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
660664
.collect();
661665
let staking_transaction =
662666
stake(1, &signers[0], &block_producers[0], TESTING_INIT_STAKE + TESTING_INIT_STAKE / 6);
@@ -717,7 +721,8 @@ fn test_state_sync() {
717721
let block_producers: Vec<_> =
718722
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
719723
let signer =
720-
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
724+
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
725+
.into();
721726
let staking_transaction = stake(1, &signer, &block_producers[0], TESTING_INIT_STAKE + 1);
722727
env.step_default(vec![staking_transaction]);
723728
env.step_default(vec![]);
@@ -806,7 +811,8 @@ fn test_get_validator_info() {
806811
let block_producers: Vec<_> =
807812
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
808813
let signer =
809-
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref());
814+
InMemorySigner::from_seed(validators[0].clone(), KeyType::ED25519, validators[0].as_ref())
815+
.into();
810816
let staking_transaction = stake(1, &signer, &block_producers[0], 0);
811817
let mut expected_blocks = [0, 0];
812818
let mut expected_chunks = [0, 0];
@@ -1047,7 +1053,8 @@ fn test_double_sign_challenge_not_all_slashed() {
10471053
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
10481054

10491055
let signer =
1050-
InMemorySigner::from_seed(validators[2].clone(), KeyType::ED25519, validators[2].as_ref());
1056+
InMemorySigner::from_seed(validators[2].clone(), KeyType::ED25519, validators[2].as_ref())
1057+
.into();
10511058
let staking_transaction = stake(1, &signer, &block_producers[2], TESTING_INIT_STAKE / 3);
10521059
env.step(
10531060
vec![vec![staking_transaction]],
@@ -1223,7 +1230,7 @@ fn test_fishermen_stake() {
12231230
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
12241231
let signers: Vec<_> = validators
12251232
.iter()
1226-
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
1233+
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
12271234
.collect();
12281235
let fishermen_stake = 3300 * NEAR_BASE + 1;
12291236

@@ -1290,7 +1297,7 @@ fn test_fishermen_unstake() {
12901297
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
12911298
let signers: Vec<_> = validators
12921299
.iter()
1293-
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
1300+
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
12941301
.collect();
12951302
let fishermen_stake = 3300 * NEAR_BASE + 1;
12961303

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

1370-
let staking_transaction1 = stake(1, &signers[1], &block_producers[1], 0);
1377+
let staking_transaction1 = stake(1, &signers[1].clone().into(), &block_producers[1], 0);
13711378
env.step_default(vec![staking_transaction1]);
13721379
let account = env.view_account(block_producers[1].validator_id());
13731380
assert_eq!(account.amount, TESTING_INIT_BALANCE - TESTING_INIT_STAKE);
13741381
assert_eq!(account.locked, TESTING_INIT_STAKE);
13751382
for _ in 2..=5 {
13761383
env.step_default(vec![]);
13771384
}
1378-
let staking_transaction2 = stake(2, &signers[1], &block_producers[1], 1);
1385+
let staking_transaction2 = stake(2, &signers[1].clone().into(), &block_producers[1], 1);
13791386
env.step_default(vec![staking_transaction2]);
13801387
for _ in 7..=13 {
13811388
env.step_default(vec![]);
@@ -1387,7 +1394,7 @@ fn test_delete_account_after_unstake() {
13871394
4,
13881395
signers[1].account_id.clone(),
13891396
signers[1].account_id.clone(),
1390-
&signers[1] as &dyn Signer,
1397+
&signers[1].clone().into(),
13911398
vec![Action::DeleteAccount(DeleteAccountAction {
13921399
beneficiary_id: signers[0].account_id.clone(),
13931400
})],
@@ -1412,7 +1419,7 @@ fn test_proposal_deduped() {
14121419
validators.iter().map(|id| create_test_signer(id.as_str())).collect();
14131420
let signers: Vec<_> = validators
14141421
.iter()
1415-
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()))
1422+
.map(|id| InMemorySigner::from_seed(id.clone(), KeyType::ED25519, id.as_ref()).into())
14161423
.collect();
14171424

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

14391446
let staking_transaction1 = stake(1, &signers[1], &block_producers[1], 100);
@@ -1481,7 +1488,7 @@ fn test_trie_and_flat_state_equality() {
14811488
4,
14821489
signers[0].account_id.clone(),
14831490
validators[1].clone(),
1484-
&signers[0] as &dyn Signer,
1491+
&signers[0].clone().into(),
14851492
vec![Action::Transfer(TransferAction { deposit: 10 })],
14861493
// runtime does not validate block history
14871494
CryptoHash::default(),
@@ -1569,7 +1576,7 @@ fn generate_transaction_pool(
15691576
round.try_into().unwrap(),
15701577
signers[i].account_id.clone(),
15711578
signers[(i + round) % signer_count].account_id.clone(),
1572-
&signers[i] as &dyn Signer,
1579+
&signers[i].clone().into(),
15731580
round.try_into().unwrap(),
15741581
block_hash,
15751582
);

chain/chain/src/test_utils.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use near_primitives::hash::CryptoHash;
2323
use near_primitives::test_utils::create_test_signer;
2424
use near_primitives::types::{AccountId, NumBlocks, NumShards};
2525
use near_primitives::utils::MaybeValidated;
26-
use near_primitives::validator_signer::InMemoryValidatorSigner;
26+
use near_primitives::validator_signer::ValidatorSigner;
2727
use near_primitives::version::PROTOCOL_VERSION;
2828
use near_store::genesis::initialize_genesis_state;
2929
use near_store::test_utils::create_test_store;
@@ -120,15 +120,15 @@ pub fn process_block_sync(
120120
// TODO(#8190) Improve this testing API.
121121
pub fn setup(
122122
clock: Clock,
123-
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<InMemoryValidatorSigner>) {
123+
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<ValidatorSigner>) {
124124
setup_with_tx_validity_period(clock, 100, 1000)
125125
}
126126

127127
pub fn setup_with_tx_validity_period(
128128
clock: Clock,
129129
tx_validity_period: NumBlocks,
130130
epoch_length: u64,
131-
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<InMemoryValidatorSigner>) {
131+
) -> (Chain, Arc<EpochManagerHandle>, Arc<NightshadeRuntime>, Arc<ValidatorSigner>) {
132132
let store = create_test_store();
133133
let mut genesis = Genesis::test_sharded(
134134
clock.clone(),

chain/chain/src/tests/garbage_collection.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use near_primitives::merkle::PartialMerkleTree;
2020
use near_primitives::shard_layout::ShardUId;
2121
use near_primitives::test_utils::{create_test_signer, TestBlockBuilder};
2222
use near_primitives::types::{BlockHeight, NumBlocks, StateRoot};
23-
use near_primitives::validator_signer::InMemoryValidatorSigner;
23+
use near_primitives::validator_signer::ValidatorSigner;
2424
use near_store::test_utils::gen_changes;
2525
use near_store::{DBCol, ShardTries, Trie, WrappedTrieChanges};
2626

@@ -730,7 +730,7 @@ fn add_block(
730730
epoch_manager: &dyn EpochManagerAdapter,
731731
prev_block: &mut Block,
732732
blocks: &mut Vec<Block>,
733-
signer: Arc<InMemoryValidatorSigner>,
733+
signer: Arc<ValidatorSigner>,
734734
height: u64,
735735
) {
736736
let next_epoch_id = epoch_manager

chain/chain/src/validate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ mod tests {
456456
nonce,
457457
account_id,
458458
"bob".parse().unwrap(),
459-
&signer,
459+
&signer.into(),
460460
10,
461461
CryptoHash::default(),
462462
)

chain/chunks/src/chunk_cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ mod tests {
296296
CryptoHash::default(),
297297
CryptoHash::default(),
298298
vec![],
299-
&signer,
299+
&signer.into(),
300300
))
301301
}
302302

chain/chunks/src/client.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ mod tests {
229229
nonce,
230230
signer_id.clone(),
231231
receiver_id.clone(),
232-
&signer,
232+
&signer.into(),
233233
deposit,
234234
CryptoHash::default(),
235235
);

chain/chunks/src/shards_manager_actor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1977,7 +1977,7 @@ impl ShardsManagerActor {
19771977
prev_outgoing_receipts_root: CryptoHash,
19781978
tx_root: CryptoHash,
19791979
congestion_info: Option<CongestionInfo>,
1980-
signer: &dyn ValidatorSigner,
1980+
signer: &ValidatorSigner,
19811981
rs: &ReedSolomon,
19821982
protocol_version: ProtocolVersion,
19831983
) -> Result<(EncodedShardChunk, Vec<MerklePath>), Error> {

chain/client/src/chunk_distribution_network.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ mod tests {
396396
hash(&[height.to_le_bytes().as_slice(), shard_id.to_le_bytes().as_slice()].concat());
397397
let mut mock_hashes = MockHashes::new(prev_block_hash);
398398

399-
let signer = EmptyValidatorSigner::default();
399+
let signer = EmptyValidatorSigner::default().into();
400400
let header_inner = ShardChunkHeaderInner::V3(ShardChunkHeaderInnerV3 {
401401
prev_block_hash,
402402
prev_state_root: mock_hashes.next().unwrap(),

0 commit comments

Comments
 (0)