Skip to content

Commit 96016cf

Browse files
committed
use &mut instead of passing db_tx by value
1 parent ef5b141 commit 96016cf

File tree

6 files changed

+172
-246
lines changed

6 files changed

+172
-246
lines changed

wallet/src/signer/mod.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,22 @@ pub trait Signer {
117117
tx: PartiallySignedTransaction,
118118
tokens_additional_info: &TokensAdditionalInfo,
119119
key_chain: &(impl AccountKeyChains + Sync),
120-
db_tx: T,
120+
db_tx: &mut T,
121121
block_height: BlockHeight,
122-
) -> (
123-
T,
124-
SignerResult<(
125-
PartiallySignedTransaction,
126-
Vec<SignatureStatus>,
127-
Vec<SignatureStatus>,
128-
)>,
129-
);
122+
) -> SignerResult<(
123+
PartiallySignedTransaction,
124+
Vec<SignatureStatus>,
125+
Vec<SignatureStatus>,
126+
)>;
130127

131128
/// Sign an arbitrary message for a destination known to this key chain.
132129
async fn sign_challenge<T: WalletStorageReadUnlocked + Send>(
133130
&mut self,
134131
message: &[u8],
135132
destination: &Destination,
136133
key_chain: &(impl AccountKeyChains + Sync),
137-
db_tx: T,
138-
) -> (T, SignerResult<ArbitraryMessageSignature>);
134+
db_tx: &mut T,
135+
) -> SignerResult<ArbitraryMessageSignature>;
139136

140137
/// Sign a transaction intent. The number of `input_destinations` must be the same as
141138
/// the number of inputs in the transaction; all of the destinations must be known
@@ -146,8 +143,8 @@ pub trait Signer {
146143
input_destinations: &[Destination],
147144
intent: &str,
148145
key_chain: &(impl AccountKeyChains + Sync),
149-
db_tx: T,
150-
) -> (T, SignerResult<SignedTransactionIntent>);
146+
db_tx: &mut T,
147+
) -> SignerResult<SignedTransactionIntent>;
151148
}
152149

153150
pub trait SignerProvider {

wallet/src/signer/software_signer/mod.rs

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -408,46 +408,34 @@ impl Signer for SoftwareSigner {
408408
ptx: PartiallySignedTransaction,
409409
_tokens_additional_info: &TokensAdditionalInfo,
410410
key_chain: &(impl AccountKeyChains + Sync),
411-
db_tx: T,
411+
db_tx: &mut T,
412412
block_height: BlockHeight,
413-
) -> (
414-
T,
415-
SignerResult<(
416-
PartiallySignedTransaction,
417-
Vec<SignatureStatus>,
418-
Vec<SignatureStatus>,
419-
)>,
420-
) {
421-
let res = self.sign_tx_impl(ptx, key_chain, &db_tx, block_height);
422-
(db_tx, res)
413+
) -> SignerResult<(
414+
PartiallySignedTransaction,
415+
Vec<SignatureStatus>,
416+
Vec<SignatureStatus>,
417+
)> {
418+
self.sign_tx_impl(ptx, key_chain, db_tx, block_height)
423419
}
424420

425421
async fn sign_challenge<T: WalletStorageReadUnlocked + Send>(
426422
&mut self,
427423
message: &[u8],
428424
destination: &Destination,
429425
key_chain: &(impl AccountKeyChains + Sync),
430-
db_tx: T,
431-
) -> (T, SignerResult<ArbitraryMessageSignature>) {
432-
let private_key = match self.get_private_key_for_destination(destination, key_chain, &db_tx)
433-
{
434-
Ok(pk) => pk,
435-
Err(e) => return (db_tx, Err(e)),
436-
};
437-
438-
let private_key = match private_key.ok_or(SignerError::DestinationNotFromThisWallet) {
439-
Ok(pk) => pk,
440-
Err(e) => return (db_tx, Err(e)),
441-
};
426+
db_tx: &mut T,
427+
) -> SignerResult<ArbitraryMessageSignature> {
428+
let private_key = self
429+
.get_private_key_for_destination(destination, key_chain, db_tx)?
430+
.ok_or(SignerError::DestinationNotFromThisWallet)?;
442431

443-
let sig = ArbitraryMessageSignature::produce_uniparty_signature(
432+
ArbitraryMessageSignature::produce_uniparty_signature(
444433
&private_key,
445434
destination,
446435
message,
447436
self.sig_aux_data_provider.lock().expect("poisoned mutex").as_mut(),
448-
);
449-
450-
(db_tx, sig.map_err(Into::into))
437+
)
438+
.map_err(Into::into)
451439
}
452440

453441
async fn sign_transaction_intent<T: WalletStorageReadUnlocked + Send>(
@@ -456,20 +444,18 @@ impl Signer for SoftwareSigner {
456444
input_destinations: &[Destination],
457445
intent: &str,
458446
key_chain: &(impl AccountKeyChains + Sync),
459-
db_tx: T,
460-
) -> (T, SignerResult<SignedTransactionIntent>) {
461-
let res = SignedTransactionIntent::produce_from_transaction(
447+
db_tx: &mut T,
448+
) -> SignerResult<SignedTransactionIntent> {
449+
SignedTransactionIntent::produce_from_transaction(
462450
transaction,
463451
input_destinations,
464452
intent,
465453
|dest| {
466-
self.get_private_key_for_destination(dest, key_chain, &db_tx)?
454+
self.get_private_key_for_destination(dest, key_chain, db_tx)?
467455
.ok_or(SignerError::DestinationNotFromThisWallet)
468456
},
469457
self.sig_aux_data_provider.lock().expect("poisoned mutex").as_mut(),
470-
);
471-
472-
(db_tx, res)
458+
)
473459
}
474460
}
475461

wallet/src/signer/tests/generic_fixed_signature_tests.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -373,16 +373,16 @@ pub async fn test_fixed_signatures_generic<MkS, S>(
373373
let orig_ptx = req.into_partially_signed_tx(ptx_additional_info).unwrap();
374374

375375
let mut signer = make_signer(chain_config.clone(), account.account_index());
376-
let (db_tx, res) = signer
376+
let (ptx, _, _) = signer
377377
.sign_tx(
378378
orig_ptx,
379379
&tokens_additional_info,
380380
account.key_chain(),
381-
db_tx,
381+
&mut db_tx,
382382
tx_block_height,
383383
)
384-
.await;
385-
let (ptx, _, _) = res.unwrap();
384+
.await
385+
.unwrap();
386386
db_tx.commit().unwrap();
387387
assert!(ptx.all_signatures_available());
388388

@@ -918,30 +918,30 @@ pub async fn test_fixed_signatures_generic2<MkS, S>(
918918
.collect_vec();
919919

920920
let mut signer = make_signer(chain_config.clone(), account1.account_index());
921-
let (db_tx, res) = signer
921+
let (ptx, _, _) = signer
922922
.sign_tx(
923923
ptx,
924924
&tokens_additional_info,
925925
account1.key_chain(),
926-
db_tx,
926+
&mut db_tx,
927927
tx_block_height,
928928
)
929-
.await;
930-
let (ptx, _, _) = res.unwrap();
929+
.await
930+
.unwrap();
931931
assert!(ptx.all_signatures_available());
932932

933933
// Fully sign multisig inputs.
934934
let mut signer = make_signer(chain_config.clone(), account2.account_index());
935-
let (db_tx, res) = signer
935+
let (ptx, _, _) = signer
936936
.sign_tx(
937937
ptx,
938938
&tokens_additional_info,
939939
account2.key_chain(),
940-
db_tx,
940+
&mut db_tx,
941941
tx_block_height,
942942
)
943-
.await;
944-
let (ptx, _, _) = res.unwrap();
943+
.await
944+
.unwrap();
945945
db_tx.commit().unwrap();
946946
assert!(ptx.all_signatures_available());
947947

wallet/src/signer/tests/generic_tests.rs

Lines changed: 43 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -141,22 +141,23 @@ pub async fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
141141
let message_challenge = produce_message_challenge(&message);
142142

143143
let mut signer = make_signer(chain_config.clone(), account.account_index());
144-
let db_tx = db.transaction_ro_unlocked().await.unwrap();
145-
let (db_tx, res) =
146-
signer.sign_challenge(&message, &destination, account.key_chain(), db_tx).await;
144+
let mut db_tx = db.transaction_ro_unlocked().await.unwrap();
145+
let res = signer
146+
.sign_challenge(&message, &destination, account.key_chain(), &mut db_tx)
147+
.await
148+
.unwrap();
147149

148-
let res = res.unwrap();
149150
res.verify_signature(&chain_config, &destination, &message_challenge).unwrap();
150151

151152
if let Some(make_another_signer) = &make_another_signer {
152153
let mut another_signer =
153154
make_another_signer(chain_config.clone(), account.account_index());
154155

155-
let (_db_tx, another_res) = another_signer
156-
.sign_challenge(&message, &destination, account.key_chain(), db_tx)
157-
.await;
156+
let another_res = another_signer
157+
.sign_challenge(&message, &destination, account.key_chain(), &mut db_tx)
158+
.await
159+
.unwrap();
158160

159-
let another_res = another_res.unwrap();
160161
another_res
161162
.verify_signature(&chain_config, &destination, &message_challenge)
162163
.unwrap();
@@ -172,9 +173,14 @@ pub async fn test_sign_message_generic<MkS1, MkS2, S1, S2>(
172173
let mut signer = make_signer(chain_config.clone(), account.account_index());
173174

174175
let message = make_message();
175-
let db_tx = db.transaction_ro_unlocked().await.unwrap();
176-
let (_db_tx, err) = signer
177-
.sign_challenge(&message, &random_pk_destination, account.key_chain(), db_tx)
176+
let mut db_tx = db.transaction_ro_unlocked().await.unwrap();
177+
let err = signer
178+
.sign_challenge(
179+
&message,
180+
&random_pk_destination,
181+
account.key_chain(),
182+
&mut db_tx,
183+
)
178184
.await;
179185

180186
assert_eq!(err.unwrap_err(), SignerError::DestinationNotFromThisWallet);
@@ -252,33 +258,32 @@ pub async fn test_sign_transaction_intent_generic<MkS1, MkS2, S1, S2>(
252258
SignedTransactionIntent::get_message_to_sign(&intent, &tx.get_id());
253259

254260
let mut signer = make_signer(chain_config.clone(), account.account_index());
255-
let (mut db_tx, res) = signer
261+
let res = signer
256262
.sign_transaction_intent(
257263
&tx,
258264
&input_destinations,
259265
&intent,
260266
account.key_chain(),
261-
db_tx,
267+
&mut db_tx,
262268
)
263-
.await;
264-
let res = res.unwrap();
269+
.await
270+
.unwrap();
265271
res.verify(&chain_config, &input_destinations, &expected_signed_message)
266272
.unwrap();
267273

268274
if let Some(make_another_signer) = &make_another_signer {
269275
let mut another_signer = make_another_signer(chain_config.clone(), account.account_index());
270-
let (db_tx2, another_res) = another_signer
276+
let another_res = another_signer
271277
.sign_transaction_intent(
272278
&tx,
273279
&input_destinations,
274280
&intent,
275281
account.key_chain(),
276-
db_tx,
282+
&mut db_tx,
277283
)
278-
.await;
279-
db_tx = db_tx2;
284+
.await
285+
.unwrap();
280286

281-
let another_res = another_res.unwrap();
282287
another_res
283288
.verify(&chain_config, &input_destinations, &expected_signed_message)
284289
.unwrap();
@@ -291,13 +296,13 @@ pub async fn test_sign_transaction_intent_generic<MkS1, MkS2, S1, S2>(
291296
let random_pk_destination = Destination::PublicKey(random_pk);
292297
input_destinations[rng.gen_range(0..num_inputs)] = random_pk_destination;
293298

294-
let (_db_tx, err) = signer
299+
let err = signer
295300
.sign_transaction_intent(
296301
&tx,
297302
&input_destinations,
298303
&intent,
299304
account.key_chain(),
300-
db_tx,
305+
&mut db_tx,
301306
)
302307
.await;
303308

@@ -729,32 +734,31 @@ pub async fn test_sign_transaction_generic<MkS1, MkS2, S1, S2>(
729734
let orig_ptx = req.into_partially_signed_tx(ptx_additional_info).unwrap();
730735

731736
let mut signer = make_signer(chain_config.clone(), account.account_index());
732-
let (mut db_tx, res) = signer
737+
let (ptx, _, _) = signer
733738
.sign_tx(
734739
orig_ptx.clone(),
735740
&tokens_additional_info,
736741
account.key_chain(),
737-
db_tx,
742+
&mut db_tx,
738743
tx_block_height,
739744
)
740-
.await;
741-
let (ptx, _, _) = res.unwrap();
745+
.await
746+
.unwrap();
742747

743748
assert!(ptx.all_signatures_available());
744749

745750
if let Some(make_another_signer) = &make_another_signer {
746751
let mut another_signer = make_another_signer(chain_config.clone(), account.account_index());
747-
let (db_tx2, res) = another_signer
752+
let (another_ptx, _, _) = another_signer
748753
.sign_tx(
749754
orig_ptx,
750755
&tokens_additional_info,
751756
account.key_chain(),
752-
db_tx,
757+
&mut db_tx,
753758
tx_block_height,
754759
)
755-
.await;
756-
db_tx = db_tx2;
757-
let (another_ptx, _, _) = res.unwrap();
760+
.await
761+
.unwrap();
758762
assert!(another_ptx.all_signatures_available());
759763

760764
assert_eq!(ptx, another_ptx);
@@ -811,32 +815,31 @@ pub async fn test_sign_transaction_generic<MkS1, MkS2, S1, S2>(
811815
let orig_ptx = ptx;
812816
// fully sign the remaining key in the multisig address
813817
let mut signer = make_signer(chain_config.clone(), account2.account_index());
814-
let (mut db_tx, res) = signer
818+
let (ptx, _, _) = signer
815819
.sign_tx(
816820
orig_ptx.clone(),
817821
&tokens_additional_info,
818822
account2.key_chain(),
819-
db_tx,
823+
&mut db_tx,
820824
tx_block_height,
821825
)
822-
.await;
823-
let (ptx, _, _) = res.unwrap();
826+
.await
827+
.unwrap();
824828
assert!(ptx.all_signatures_available());
825829

826830
if let Some(make_another_signer) = &make_another_signer {
827831
let mut another_signer =
828832
make_another_signer(chain_config.clone(), account2.account_index());
829-
let (db_tx2, res) = another_signer
833+
let (another_ptx, _, _) = another_signer
830834
.sign_tx(
831835
orig_ptx,
832836
&tokens_additional_info,
833837
account2.key_chain(),
834-
db_tx,
838+
&mut db_tx,
835839
tx_block_height,
836840
)
837-
.await;
838-
db_tx = db_tx2;
839-
let (another_ptx, _, _) = res.unwrap();
841+
.await
842+
.unwrap();
840843
assert!(another_ptx.all_signatures_available());
841844

842845
assert_eq!(ptx, another_ptx);

0 commit comments

Comments
 (0)