chain-impl-mockchain: use more idiomatic builder-style APIs

eugene-babichenko · eugene-babichenko · commit 40fa6ea17fb8 · 2021-08-05T10:18:08.000+03:00
diff --git a/chain-impl-mockchain/benches/tally.rs b/chain-impl-mockchain/benches/tally.rs
@@ -1,5 +1,4 @@
 use chain_crypto::testing::TestCryptoRng;
-use chain_impl_mockchain::testing::scenario::template::WalletTemplateBuilder;
 use chain_impl_mockchain::{
     certificate::{
         DecryptedPrivateTally, DecryptedPrivateTallyProposal, EncryptedVoteTally, VoteCast,
@@ -41,18 +40,15 @@ fn tally_benchmark(
 ) {
     let mut rng = TestCryptoRng::seed_from_u64(0);
 
-    // All wallets that are needed to be initialized in the genesis block
-    // TODO the underlying ledger constructor is not using this &mut. This should be a plain
-    // Vec<WalletTemplateBuilder>, which will greatly simplify this code.
-    let mut wallets: Vec<&mut WalletTemplateBuilder> = Vec::new();
+    let mut wallets = Vec::new();
 
     // Stake pool owner
-    let mut alice_wallet_builder = wallet(ALICE);
-    alice_wallet_builder
+    let alice_wallet_builder = wallet(ALICE)
         .with(1_000)
         .owns(STAKE_POOL)
         .committee_member();
-    wallets.push(&mut alice_wallet_builder);
+
+    wallets.push(alice_wallet_builder);
 
     // generate the required number of wallets from the distribution
     let voters_aliases: Vec<_> = (1..=voters_count)
@@ -63,17 +59,17 @@ fn tally_benchmark(
         .take(voters_count)
         .collect();
     let total_votes = voting_powers.iter().sum();
-    let mut voters_wallets: Vec<_> = voters_aliases
+    {
+        let mut voters_wallets: Vec<_> = voters_aliases
         .iter()
         .zip(voting_powers.iter())
         .map(|(alias, voting_power)| {
-            let mut wallet_builder = WalletTemplateBuilder::new(alias);
-            wallet_builder.with(*voting_power);
-            wallet_builder
+            wallet(alias).with(*voting_power)
         })
         .collect();
 
-    wallets.append(&mut voters_wallets.iter_mut().collect());
+        wallets.append(&mut voters_wallets);
+    }
 
     // Prepare committee members keys
     let members = CommitteeMembersManager::new(&mut rng, CRS_SEED, THRESHOLD, MEMBERS_NO);
@@ -84,16 +80,14 @@ fn tally_benchmark(
         .collect();
 
     // Build the vote plan
-    let mut vote_plan_builder = vote_plan(VOTE_PLAN);
-    vote_plan_builder
+    let mut vote_plan_builder = vote_plan(VOTE_PLAN)
         .owner(ALICE)
         .consecutive_epoch_dates()
         .payload_type(PayloadType::Private)
         .committee_keys(committee_keys);
     for _ in 0..n_proposals {
-        let mut proposal_builder = proposal(VoteTestGen::external_proposal_id());
-        proposal_builder.options(3).action_off_chain();
-        vote_plan_builder.with_proposal(&mut proposal_builder);
+        let proposal_builder = proposal(VoteTestGen::external_proposal_id()).options(3).action_off_chain();
+        vote_plan_builder = vote_plan_builder.with_proposal(proposal_builder);
     }
 
     // Initialize ledger
@@ -104,7 +98,7 @@ fn tally_benchmark(
                 .with_rewards(Value(1000)),
         )
         .with_initials(wallets)
-        .with_vote_plans(vec![&mut vote_plan_builder])
+        .with_vote_plans(vec![vote_plan_builder])
         .build()
         .unwrap();
 
diff --git a/chain-impl-mockchain/src/testing/builders/block_builder.rs b/chain-impl-mockchain/src/testing/builders/block_builder.rs
@@ -32,41 +32,39 @@ impl GenesisPraosBlockBuilder {
         }
     }
 
-    pub fn with_parent(&mut self, parent: &Header) -> &mut Self {
-        self.with_parent_id(parent.hash());
-        self.with_date(parent.block_date());
-        self.with_chain_length(parent.chain_length());
-        self
+    pub fn with_parent(self, parent: &Header) -> Self {
+        self.with_parent_id(parent.hash())
+            .with_date(parent.block_date())
+            .with_chain_length(parent.chain_length())
     }
 
-    pub fn with_parent_id(&mut self, parent_id: Hash) -> &mut Self {
+    pub fn with_parent_id(mut self, parent_id: Hash) -> Self {
         self.parent_id = Some(parent_id);
         self
     }
 
-    pub fn with_date(&mut self, date: BlockDate) -> &mut Self {
+    pub fn with_date(mut self, date: BlockDate) -> Self {
         self.date = Some(date);
         self
     }
 
-    pub fn with_chain_length(&mut self, chain_length: ChainLength) -> &mut Self {
+    pub fn with_chain_length(mut self, chain_length: ChainLength) -> Self {
         self.chain_length = Some(chain_length);
         self
     }
 
-    pub fn with_fragment(&mut self, fragment: Fragment) -> &mut Self {
+    pub fn with_fragment(mut self, fragment: Fragment) -> Self {
         self.contents_builder.push(fragment);
         self
     }
 
-    pub fn with_fragments(&mut self, fragments: Vec<Fragment>) -> &mut Self {
-        for fragment in fragments {
-            self.with_fragment(fragment);
-        }
-        self
+    pub fn with_fragments(self, fragments: Vec<Fragment>) -> Self {
+        fragments
+            .into_iter()
+            .fold(self, |builder, fragment| builder.with_fragment(fragment))
     }
 
-    pub fn build(&self, stake_pool: &StakePool, time_era: &TimeEra) -> Block {
+    pub fn build(self, stake_pool: &StakePool, time_era: &TimeEra) -> Block {
         if self.date.is_none() || self.chain_length.is_none() || self.parent_id.is_none() {
             panic!("date,chain_length or hash is not set");
         }
diff --git a/chain-impl-mockchain/src/testing/builders/stake_pool_builder.rs b/chain-impl-mockchain/src/testing/builders/stake_pool_builder.rs
@@ -45,37 +45,37 @@ impl StakePoolBuilder {
         }
     }
 
-    pub fn with_owners(&mut self, owners: Vec<PublicKey<Ed25519>>) -> &mut Self {
+    pub fn with_owners(mut self, owners: Vec<PublicKey<Ed25519>>) -> Self {
         self.owners.extend(owners);
         self
     }
 
-    pub fn with_alias(&mut self, alias: &str) -> &mut Self {
+    pub fn with_alias(mut self, alias: &str) -> Self {
         self.alias = alias.to_owned();
         self
     }
 
-    pub fn with_operators(&mut self, operators: Vec<PublicKey<Ed25519>>) -> &mut Self {
+    pub fn with_operators(mut self, operators: Vec<PublicKey<Ed25519>>) -> Self {
         self.operators.extend(operators);
         self
     }
 
-    pub fn with_pool_permissions(&mut self, permissions: PoolPermissions) -> &mut Self {
+    pub fn with_pool_permissions(mut self, permissions: PoolPermissions) -> Self {
         self.pool_permissions = Some(permissions);
         self
     }
 
-    pub fn with_reward_account(&mut self, reward_account: bool) -> &mut Self {
+    pub fn with_reward_account(mut self, reward_account: bool) -> Self {
         self.reward_account = reward_account;
         self
     }
 
     pub fn with_ratio_tax_type(
-        &mut self,
+        self,
         numerator: u64,
         denominator: u64,
         max_limit: Option<u64>,
-    ) -> &mut Self {
+    ) -> Self {
         self.with_tax_type(TaxType {
             fixed: Value(0),
             ratio: Ratio {
@@ -86,12 +86,12 @@ impl StakePoolBuilder {
         })
     }
 
-    pub fn with_tax_type(&mut self, tax_type: TaxType) -> &mut Self {
+    pub fn with_tax_type(mut self, tax_type: TaxType) -> Self {
         self.tax_type = tax_type;
         self
     }
 
-    pub fn build(&self) -> StakePool {
+    pub fn build(self) -> StakePool {
         let mut rng = rand_core::OsRng;
 
         let pool_vrf: KeyPair<RistrettoGroup2HashDh> = KeyPair::generate(&mut rng);
diff --git a/chain-impl-mockchain/src/testing/builders/update_builder.rs b/chain-impl-mockchain/src/testing/builders/update_builder.rs
@@ -24,19 +24,19 @@ impl ProposalBuilder {
             config_params: ConfigParams::new(),
         }
     }
-    pub fn with_proposal_changes(&mut self, changes: Vec<ConfigParam>) -> &mut Self {
-        for change in changes {
-            self.with_proposal_change(change);
-        }
-        self
+
+    pub fn with_proposal_changes(self, changes: Vec<ConfigParam>) -> Self {
+        changes
+            .into_iter()
+            .fold(self, |builder, change| builder.with_proposal_change(change))
     }
 
-    pub fn with_proposal_change(&mut self, change: ConfigParam) -> &mut Self {
+    pub fn with_proposal_change(mut self, change: ConfigParam) -> Self {
         self.config_params.push(change);
         self
     }
 
-    pub fn build(&self) -> UpdateProposal {
+    pub fn build(self) -> UpdateProposal {
         let mut update_proposal = UpdateProposal::new();
         for config_param in self.config_params.iter().cloned() {
             update_proposal.changes.push(config_param);
@@ -59,17 +59,17 @@ impl SignedProposalBuilder {
         }
     }
 
-    pub fn with_proposer_id(&mut self, proposer_id: BftLeaderId) -> &mut Self {
+    pub fn with_proposer_id(mut self, proposer_id: BftLeaderId) -> Self {
         self.proposer_id = Some(proposer_id);
         self
     }
 
-    pub fn with_proposal_update(&mut self, update_proposal: UpdateProposal) -> &mut Self {
+    pub fn with_proposal_update(mut self, update_proposal: UpdateProposal) -> Self {
         self.update_proposal = Some(update_proposal);
         self
     }
 
-    pub fn build(&self) -> SignedUpdateProposal {
+    pub fn build(self) -> SignedUpdateProposal {
         SignedUpdateProposal {
             proposal: UpdateProposalWithProposer {
                 proposal: self.update_proposal.clone().unwrap(),
@@ -93,17 +93,17 @@ impl UpdateVoteBuilder {
         }
     }
 
-    pub fn with_proposal_id(&mut self, proposal_id: UpdateProposalId) -> &mut Self {
+    pub fn with_proposal_id(mut self, proposal_id: UpdateProposalId) -> Self {
         self.proposal_id = Some(proposal_id);
         self
     }
 
-    pub fn with_voter_id(&mut self, voter_id: BftLeaderId) -> &mut Self {
+    pub fn with_voter_id(mut self, voter_id: BftLeaderId) -> Self {
         self.voter_id = Some(voter_id);
         self
     }
 
-    pub fn build(&self) -> SignedUpdateVote {
+    pub fn build(self) -> SignedUpdateVote {
         let update_vote = UpdateVote {
             proposal_id: self.proposal_id.unwrap(),
             voter_id: self.voter_id.clone().unwrap(),
diff --git a/chain-impl-mockchain/src/testing/e2e/rewards/tax.rs b/chain-impl-mockchain/src/testing/e2e/rewards/tax.rs
@@ -90,7 +90,7 @@ pub fn no_tax() {
 
 fn verify_distribute_rewards(
     total_reward: u64,
-    stake_pool_builder: &mut StakePoolDefBuilder,
+    stake_pool_builder: StakePoolDefBuilder,
     expected_stake_pool_reward: u64,
 ) {
     let (mut ledger, controller) = prepare_scenario()
diff --git a/chain-impl-mockchain/src/testing/scenario/scenario_builder.rs b/chain-impl-mockchain/src/testing/scenario/scenario_builder.rs
@@ -42,7 +42,7 @@ pub struct ScenarioBuilder {
     config: ConfigBuilder,
     initials: Option<Vec<WalletTemplateBuilder>>,
     stake_pools: Option<Vec<StakePoolDefBuilder>>,
-    vote_plans: Vec<VotePlanDefBuilder>,
+    vote_plans: Option<Vec<VotePlanDefBuilder>>,
 }
 
 pub fn prepare_scenario() -> ScenarioBuilder {
@@ -54,32 +54,32 @@ pub fn prepare_scenario() -> ScenarioBuilder {
         config: default_config_builder,
         initials: None,
         stake_pools: None,
-        vote_plans: Vec::new(),
+        vote_plans: None,
     }
 }
 
 impl ScenarioBuilder {
-    pub fn with_config(&mut self, config: ConfigBuilder) -> &mut Self {
+    pub fn with_config(mut self, config: ConfigBuilder) -> Self {
         self.config = config;
         self
     }
 
-    pub fn with_initials(&mut self, initials: Vec<&mut WalletTemplateBuilder>) -> &mut Self {
-        self.initials = Some(initials.iter().map(|x| (**x).clone()).collect());
+    pub fn with_initials(mut self, initials: Vec<WalletTemplateBuilder>) -> Self {
+        self.initials = Some(initials);
         self
     }
 
-    pub fn with_vote_plans(&mut self, vote_plans: Vec<&mut VotePlanDefBuilder>) -> &mut Self {
-        self.vote_plans = vote_plans.iter().map(|x| (**x).clone()).collect();
+    pub fn with_vote_plans(mut self, vote_plans: Vec<VotePlanDefBuilder>) -> Self {
+        self.vote_plans = Some(vote_plans);
         self
     }
 
-    pub fn with_stake_pools(&mut self, stake_pools: Vec<&mut StakePoolDefBuilder>) -> &mut Self {
-        self.stake_pools = Some(stake_pools.iter().map(|x| (**x).clone()).collect());
+    pub fn with_stake_pools(mut self, stake_pools: Vec<StakePoolDefBuilder>) -> Self {
+        self.stake_pools = Some(stake_pools);
         self
     }
 
-    pub fn build(&self) -> Result<(TestLedger, Controller), ScenarioBuilderError> {
+    pub fn build(self) -> Result<(TestLedger, Controller), ScenarioBuilderError> {
         if self.initials.is_none() {
             return Err(ScenarioBuilderError::UndefinedInitials);
         }
@@ -107,14 +107,17 @@ impl ScenarioBuilder {
         let faucets: Vec<AddressDataValue> =
             wallets.iter().cloned().map(|x| x.as_account()).collect();
 
-        let vote_plan_defs: Vec<VotePlanDef> =
-            self.vote_plans.iter().map(|x| x.clone().build()).collect();
-        let vote_plan_fragments: Vec<Fragment> = self
+        let vote_plan_defs: Vec<VotePlanDef> = self
             .vote_plans
+            .iter()
+            .map(|builders| builders.into_iter())
+            .flatten()
+            .map(|x| x.clone().build())
+            .collect();
+        let vote_plan_fragments: Vec<Fragment> = vote_plan_defs
             .iter()
             .cloned()
-            .map(|x| {
-                let vote_plan_def = x.build();
+            .map(|vote_plan_def| {
                 let owner = wallets
                     .iter()
                     .cloned()
@@ -211,9 +214,9 @@ impl ScenarioBuilder {
     }
 
     fn build_stake_pool(&self, template: StakePoolTemplate) -> StakePool {
-        let mut builder = StakePoolBuilder::new();
-        builder.with_owners(template.owners());
-        builder.with_alias(&template.alias());
+        let mut builder = StakePoolBuilder::new()
+            .with_owners(template.owners())
+            .with_alias(&template.alias());
 
         if let Some(stake_pools) = &self.stake_pools {
             let stake_pool_def_opt = stake_pools
@@ -224,12 +227,12 @@ impl ScenarioBuilder {
 
             if let Some(stake_pool_def) = stake_pool_def_opt {
                 if let Some(pool_permission) = stake_pool_def.pool_permission() {
-                    builder.with_pool_permissions(pool_permission);
+                    builder = builder.with_pool_permissions(pool_permission);
                 }
                 if let Some(tax_type) = stake_pool_def.tax_type {
-                    builder.with_tax_type(tax_type);
+                    builder = builder.with_tax_type(tax_type);
                 }
-                builder.with_reward_account(stake_pool_def.has_reward_account);
+                builder = builder.with_reward_account(stake_pool_def.has_reward_account);
             }
         }
         builder.build()
diff --git a/chain-impl-mockchain/src/testing/scenario/template/builders.rs b/chain-impl-mockchain/src/testing/scenario/template/builders.rs

Original file line number	Diff line number	Diff line change
`@@ -32,41 +32,39 @@ impl GenesisPraosBlockBuilder {`
`32`	`32`	`}`
`33`	`33`	`}`
`34`	`34`
`35`		`- pub fn with_parent(&mut self, parent: &Header) -> &mut Self {`
`36`		`- self.with_parent_id(parent.hash());`
`37`		`- self.with_date(parent.block_date());`
`38`		`- self.with_chain_length(parent.chain_length());`
`39`		`- self`
	`35`	`+ pub fn with_parent(self, parent: &Header) -> Self {`
	`36`	`+ self.with_parent_id(parent.hash())`
	`37`	`+ .with_date(parent.block_date())`
	`38`	`+ .with_chain_length(parent.chain_length())`
`40`	`39`	`}`
`41`	`40`
`42`		`- pub fn with_parent_id(&mut self, parent_id: Hash) -> &mut Self {`
	`41`	`+ pub fn with_parent_id(mut self, parent_id: Hash) -> Self {`
`43`	`42`	`self.parent_id = Some(parent_id);`
`44`	`43`	`self`
`45`	`44`	`}`
`46`	`45`
`47`		`- pub fn with_date(&mut self, date: BlockDate) -> &mut Self {`
	`46`	`+ pub fn with_date(mut self, date: BlockDate) -> Self {`
`48`	`47`	`self.date = Some(date);`
`49`	`48`	`self`
`50`	`49`	`}`
`51`	`50`
`52`		`- pub fn with_chain_length(&mut self, chain_length: ChainLength) -> &mut Self {`
	`51`	`+ pub fn with_chain_length(mut self, chain_length: ChainLength) -> Self {`
`53`	`52`	`self.chain_length = Some(chain_length);`
`54`	`53`	`self`
`55`	`54`	`}`
`56`	`55`
`57`		`- pub fn with_fragment(&mut self, fragment: Fragment) -> &mut Self {`
	`56`	`+ pub fn with_fragment(mut self, fragment: Fragment) -> Self {`
`58`	`57`	`self.contents_builder.push(fragment);`
`59`	`58`	`self`
`60`	`59`	`}`
`61`	`60`
`62`		`- pub fn with_fragments(&mut self, fragments: Vec<Fragment>) -> &mut Self {`
`63`		`- for fragment in fragments {`
`64`		`- self.with_fragment(fragment);`
`65`		`- }`
`66`		`- self`
	`61`	`+ pub fn with_fragments(self, fragments: Vec<Fragment>) -> Self {`
	`62`	`+ fragments`
	`63`	`+ .into_iter()`
	`64`	`+ .fold(self, \|builder, fragment\| builder.with_fragment(fragment))`
`67`	`65`	`}`
`68`	`66`
`69`		`- pub fn build(&self, stake_pool: &StakePool, time_era: &TimeEra) -> Block {`
	`67`	`+ pub fn build(self, stake_pool: &StakePool, time_era: &TimeEra) -> Block {`
`70`	`68`	`if self.date.is_none() \|\| self.chain_length.is_none() \|\| self.parent_id.is_none() {`
`71`	`69`	`panic!("date,chain_length or hash is not set");`
`72`	`70`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,19 +24,19 @@ impl ProposalBuilder {`
`24`	`24`	`config_params: ConfigParams::new(),`
`25`	`25`	`}`
`26`	`26`	`}`
`27`		`- pub fn with_proposal_changes(&mut self, changes: Vec<ConfigParam>) -> &mut Self {`
`28`		`- for change in changes {`
`29`		`- self.with_proposal_change(change);`
`30`		`- }`
`31`		`- self`
	`27`	`+`
	`28`	`+ pub fn with_proposal_changes(self, changes: Vec<ConfigParam>) -> Self {`
	`29`	`+ changes`
	`30`	`+ .into_iter()`
	`31`	`+ .fold(self, \|builder, change\| builder.with_proposal_change(change))`
`32`	`32`	`}`
`33`	`33`
`34`		`- pub fn with_proposal_change(&mut self, change: ConfigParam) -> &mut Self {`
	`34`	`+ pub fn with_proposal_change(mut self, change: ConfigParam) -> Self {`
`35`	`35`	`self.config_params.push(change);`
`36`	`36`	`self`
`37`	`37`	`}`
`38`	`38`
`39`		`- pub fn build(&self) -> UpdateProposal {`
	`39`	`+ pub fn build(self) -> UpdateProposal {`
`40`	`40`	`let mut update_proposal = UpdateProposal::new();`
`41`	`41`	`for config_param in self.config_params.iter().cloned() {`
`42`	`42`	`update_proposal.changes.push(config_param);`
`@@ -59,17 +59,17 @@ impl SignedProposalBuilder {`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`
`62`		`- pub fn with_proposer_id(&mut self, proposer_id: BftLeaderId) -> &mut Self {`
	`62`	`+ pub fn with_proposer_id(mut self, proposer_id: BftLeaderId) -> Self {`
`63`	`63`	`self.proposer_id = Some(proposer_id);`
`64`	`64`	`self`
`65`	`65`	`}`
`66`	`66`
`67`		`- pub fn with_proposal_update(&mut self, update_proposal: UpdateProposal) -> &mut Self {`
	`67`	`+ pub fn with_proposal_update(mut self, update_proposal: UpdateProposal) -> Self {`
`68`	`68`	`self.update_proposal = Some(update_proposal);`
`69`	`69`	`self`
`70`	`70`	`}`
`71`	`71`
`72`		`- pub fn build(&self) -> SignedUpdateProposal {`
	`72`	`+ pub fn build(self) -> SignedUpdateProposal {`
`73`	`73`	`SignedUpdateProposal {`
`74`	`74`	`proposal: UpdateProposalWithProposer {`
`75`	`75`	`proposal: self.update_proposal.clone().unwrap(),`
`@@ -93,17 +93,17 @@ impl UpdateVoteBuilder {`
`93`	`93`	`}`
`94`	`94`	`}`
`95`	`95`
`96`		`- pub fn with_proposal_id(&mut self, proposal_id: UpdateProposalId) -> &mut Self {`
	`96`	`+ pub fn with_proposal_id(mut self, proposal_id: UpdateProposalId) -> Self {`
`97`	`97`	`self.proposal_id = Some(proposal_id);`
`98`	`98`	`self`
`99`	`99`	`}`
`100`	`100`
`101`		`- pub fn with_voter_id(&mut self, voter_id: BftLeaderId) -> &mut Self {`
	`101`	`+ pub fn with_voter_id(mut self, voter_id: BftLeaderId) -> Self {`
`102`	`102`	`self.voter_id = Some(voter_id);`
`103`	`103`	`self`
`104`	`104`	`}`
`105`	`105`
`106`		`- pub fn build(&self) -> SignedUpdateVote {`
	`106`	`+ pub fn build(self) -> SignedUpdateVote {`
`107`	`107`	`let update_vote = UpdateVote {`
`108`	`108`	`proposal_id: self.proposal_id.unwrap(),`
`109`	`109`	`voter_id: self.voter_id.clone().unwrap(),`