Skip to content

Commit

Permalink
Merge pull request #6 from radixdlt/rectify_review_suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
CyonAlexRDX authored Aug 30, 2024
2 parents 9cf2188 + bea8ef0 commit 3ee7ed4
Show file tree
Hide file tree
Showing 24 changed files with 332 additions and 350 deletions.
12 changes: 6 additions & 6 deletions src/derivation/collector/keys_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,28 +66,28 @@ impl KeysCollector {
.interactor_for(factor_sources_of_kind.kind);
let factor_sources = factor_sources_of_kind.factor_sources();
match interactor {
KeyDerivationInteractor::Parallel(interactor) => {
KeyDerivationInteractor::PolyFactor(interactor) => {
// Prepare the request for the interactor
debug!("Creating parallel request for interactor");
debug!("Creating poly request for interactor");
let request = self.request_for_parallel_interactor(
factor_sources
.into_iter()
.map(|f| f.factor_source_id())
.collect(),
)?;
debug!("Dispatching parallel request to interactor: {:?}", request);
debug!("Dispatching poly request to interactor: {:?}", request);
let response = interactor.derive(request).await?;
self.process_batch_response(response)?;
}

KeyDerivationInteractor::Serial(interactor) => {
KeyDerivationInteractor::MonoFactor(interactor) => {
for factor_source in factor_sources {
// Prepare the request for the interactor
debug!("Creating serial request for interactor");
debug!("Creating mono request for interactor");
let request =
self.request_for_serial_interactor(&factor_source.factor_source_id())?;

debug!("Dispatching serial request to interactor: {:?}", request);
debug!("Dispatching mono request to interactor: {:?}", request);
// Produce the results from the interactor
let response = interactor.derive(request).await?;

Expand Down
14 changes: 7 additions & 7 deletions src/derivation/interactors/keys_collecting_interactors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ pub trait KeysCollectingInteractors {
fn interactor_for(&self, kind: FactorSourceKind) -> KeyDerivationInteractor;
}

/// An interactor which can derive keys - either in parallel or serially.
/// An interactor which can derive keys - either in poly or serially.
pub enum KeyDerivationInteractor {
Parallel(Arc<dyn DeriveKeyWithFactorParallelInteractor>),
Serial(Arc<dyn DeriveKeyWithFactorSerialInteractor>),
PolyFactor(Arc<dyn DeriveKeyWithFactorParallelInteractor>),
MonoFactor(Arc<dyn DeriveKeyWithFactorSerialInteractor>),
}

impl KeyDerivationInteractor {
pub fn parallel(interactor: Arc<dyn DeriveKeyWithFactorParallelInteractor>) -> Self {
Self::Parallel(interactor)
pub fn poly(interactor: Arc<dyn DeriveKeyWithFactorParallelInteractor>) -> Self {
Self::PolyFactor(interactor)
}

pub fn serial(interactor: Arc<dyn DeriveKeyWithFactorSerialInteractor>) -> Self {
Self::Serial(interactor)
pub fn mono(interactor: Arc<dyn DeriveKeyWithFactorSerialInteractor>) -> Self {
Self::MonoFactor(interactor)
}
}

Expand Down
32 changes: 16 additions & 16 deletions src/signing/collector/signatures_collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,10 @@ impl SignaturesCollector {
.interactor_for(factor_sources_of_kind.kind);
let factor_sources = factor_sources_of_kind.factor_sources();
match interactor {
// Parallel Interactor: Many Factor Sources at once
SigningInteractor::Parallel(interactor) => {
// PolyFactor Interactor: Many Factor Sources at once
SigningInteractor::PolyFactor(interactor) => {
// Prepare the request for the interactor
debug!("Creating parallel request for interactor");
debug!("Creating poly request for interactor");
let request = self.request_for_parallel_interactor(factor_sources_of_kind);
if !request.invalid_transactions_if_neglected.is_empty() {
info!(
Expand All @@ -194,20 +194,20 @@ impl SignaturesCollector {
request.invalid_transactions_if_neglected
)
}
debug!("Dispatching parallel request to interactor: {:?}", request);
debug!("Dispatching poly request to interactor: {:?}", request);
let response = interactor.sign(request).await;
debug!("Got response from parallel interactor: {:?}", response);
debug!("Got response from poly interactor: {:?}", response);
self.process_batch_response(response);
}

// Serial Interactor: One Factor Sources at a time
// MonoFactor Interactor: One Factor Sources at a time
// After each factor source we pass the result to the collector
// updating its internal state so that we state about being able
// to skip the next factor source or not.
SigningInteractor::Serial(interactor) => {
SigningInteractor::MonoFactor(interactor) => {
for factor_source in factor_sources {
// Prepare the request for the interactor
debug!("Creating serial request for interactor");
debug!("Creating mono request for interactor");
let request =
self.request_for_serial_interactor(&factor_source.factor_source_id());

Expand All @@ -219,10 +219,10 @@ impl SignaturesCollector {
)
}

debug!("Dispatching serial request to interactor: {:?}", request);
debug!("Dispatching mono request to interactor: {:?}", request);
// Produce the results from the interactor
let response = interactor.sign(request).await;
debug!("Got response from serial interactor: {:?}", response);
debug!("Got response from mono interactor: {:?}", response);

// Report the results back to the collector
self.process_batch_response(response);
Expand Down Expand Up @@ -254,7 +254,7 @@ impl SignaturesCollector {
fn input_for_interactor(
&self,
factor_source_id: &FactorSourceIDFromHash,
) -> BatchTXBatchKeySigningRequest {
) -> MonoFactorSignRequestInput {
self.state
.borrow()
.petitions
Expand All @@ -265,10 +265,10 @@ impl SignaturesCollector {
fn request_for_serial_interactor(
&self,
factor_source_id: &FactorSourceIDFromHash,
) -> SerialBatchSigningRequest {
) -> MonoFactorSignRequest {
let batch_signing_request = self.input_for_interactor(factor_source_id);

SerialBatchSigningRequest::new(
MonoFactorSignRequest::new(
batch_signing_request,
self.invalid_transactions_if_neglected_factor_sources(IndexSet::from_iter([
*factor_source_id,
Expand All @@ -281,7 +281,7 @@ impl SignaturesCollector {
fn request_for_parallel_interactor(
&self,
factor_sources_of_kind: &FactorSourcesOfKind,
) -> ParallelBatchSigningRequest {
) -> PolyFactorSignRequest {
let factor_source_ids = factor_sources_of_kind
.factor_sources()
.iter()
Expand All @@ -291,13 +291,13 @@ impl SignaturesCollector {
.clone()
.iter()
.map(|fid| (*fid, self.input_for_interactor(fid)))
.collect::<IndexMap<FactorSourceIDFromHash, BatchTXBatchKeySigningRequest>>();
.collect::<IndexMap<FactorSourceIDFromHash, MonoFactorSignRequestInput>>();

let invalid_transactions_if_neglected =
self.invalid_transactions_if_neglected_factor_sources(factor_source_ids);

// Prepare the request for the interactor
ParallelBatchSigningRequest::new(
PolyFactorSignRequest::new(
factor_sources_of_kind.kind,
per_factor_source,
invalid_transactions_if_neglected,
Expand Down
15 changes: 9 additions & 6 deletions src/signing/collector/signatures_collector_preprocessor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ impl SignaturesCollectorPreprocessor {
all_factor_sources_in_profile: IndexSet<HDFactorSource>,
) -> (Petitions, IndexSet<FactorSourcesOfKind>) {
let transactions = self.transactions;
let mut petitions_for_all_transactions = IndexMap::<IntentHash, PetitionTransaction>::new();
let mut petitions_for_all_transactions =
IndexMap::<IntentHash, PetitionForTransaction>::new();

let all_factor_sources_in_profile = all_factor_sources_in_profile
.into_iter()
Expand Down Expand Up @@ -65,7 +66,7 @@ impl SignaturesCollectorPreprocessor {

for transaction in transactions.into_iter() {
let mut petitions_for_entities =
HashMap::<AddressOfAccountOrPersona, PetitionEntity>::new();
HashMap::<AddressOfAccountOrPersona, PetitionForEntity>::new();

for entity in transaction.entities_requiring_auth() {
let address = entity.address();
Expand All @@ -82,7 +83,7 @@ impl SignaturesCollectorPreprocessor {

add(primary_role_matrix.override_factors.clone());
add(primary_role_matrix.threshold_factors.clone());
let petition = PetitionEntity::new_securified(
let petition = PetitionForEntity::new_securified(
transaction.intent_hash.clone(),
address.clone(),
primary_role_matrix,
Expand All @@ -93,7 +94,7 @@ impl SignaturesCollectorPreprocessor {
let factor_instance = uec;
let factor_source_id = factor_instance.factor_source_id;
use_factor_in_tx(&factor_source_id, &transaction.intent_hash);
let petition = PetitionEntity::new_unsecurified(
let petition = PetitionForEntity::new_unsecurified(
transaction.intent_hash.clone(),
address.clone(),
factor_instance,
Expand All @@ -103,8 +104,10 @@ impl SignaturesCollectorPreprocessor {
}
}

let petition_of_tx =
PetitionTransaction::new(transaction.intent_hash.clone(), petitions_for_entities);
let petition_of_tx = PetitionForTransaction::new(
transaction.intent_hash.clone(),
petitions_for_entities,
);

petitions_for_all_transactions.insert(transaction.intent_hash, petition_of_tx);
}
Expand Down
2 changes: 1 addition & 1 deletion src/signing/interactors/batch_signing_response.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::prelude::*;

/// The response of a batch signing request, either a Parallel or Serial signing
/// The response of a batch signing request, either a PolyFactor or MonoFactor signing
/// request, matters not, because the goal is to have signed all transactions with
/// enough keys (derivation paths) needed for it to be valid when submitted to the
/// Radix network.
Expand Down
22 changes: 12 additions & 10 deletions src/signing/interactors/mod.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
mod batch_signing_response;
mod batch_tx_batch_key_signing_request;
mod parallel_batch_signing_request;
mod serial_batch_signing_request;
mod sign_with_factor_parallel_interactor;
mod sign_with_factor_serial_interactor;
mod mono_factor_sign_interactor;
mod mono_factor_sign_request;
mod mono_factor_sign_request_input;
mod poly_factor_sign_interactor;
mod poly_factor_sign_request;
mod signature_collecting_interactors;
mod signing_interactor;
mod transaction_sign_request_input;

pub use batch_signing_response::*;
pub use batch_tx_batch_key_signing_request::*;
pub use parallel_batch_signing_request::*;
pub use serial_batch_signing_request::*;
pub use sign_with_factor_parallel_interactor::*;
pub use sign_with_factor_serial_interactor::*;
pub use mono_factor_sign_interactor::*;
pub use mono_factor_sign_request::*;
pub use mono_factor_sign_request_input::*;
pub use poly_factor_sign_interactor::*;
pub use poly_factor_sign_request::*;
pub use signature_collecting_interactors::*;
pub use signing_interactor::*;
pub use transaction_sign_request_input::*;
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ use crate::prelude::*;
/// The user might chose to SKIP the current factor source, and move on to the
/// next one.
///
/// Example of a Serial Batch Signing Driver is SecurityQuestionsFactorSource,
/// where it does not make any sense to let user in parallel answer multiple
/// Example of a MonoFactor Batch Signing Driver is SecurityQuestionsFactorSource,
/// where it does not make any sense to let user in poly answer multiple
/// questions from different security questions factor sources (in fact we
/// might not even even allow multiple SecurityQuestionsFactorSources to be used).
#[async_trait::async_trait]
pub trait SignWithFactorSerialInteractor {
async fn sign(&self, request: SerialBatchSigningRequest) -> SignWithFactorsOutcome;
pub trait MonoFactorSignInteractor {
async fn sign(&self, request: MonoFactorSignRequest) -> SignWithFactorsOutcome;
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
use crate::prelude::*;

/// A batch signing request used with a SignWithFactorSerialInteractor, containing
/// A batch signing request used with a MonoFactorSignInteractor, containing
/// a collection of transactions to sign with multiple keys (derivation paths),
/// and a collection of transactions which would be invalid if the user skips
/// signing with this factor source, or if we fail to sign.
#[derive(derive_more::Debug, Clone)]
#[debug("input: {:#?}", input)]
pub struct SerialBatchSigningRequest {
pub input: BatchTXBatchKeySigningRequest,
pub struct MonoFactorSignRequest {
pub input: MonoFactorSignRequestInput,
/// A collection of transactions which would be invalid if the user skips
/// signing with this factor source, or if we fail to sign
pub invalid_transactions_if_neglected: IndexSet<InvalidTransactionIfNeglected>,
}

impl SerialBatchSigningRequest {
impl MonoFactorSignRequest {
pub fn new(
input: BatchTXBatchKeySigningRequest,
input: MonoFactorSignRequestInput,
invalid_transactions_if_neglected: IndexSet<InvalidTransactionIfNeglected>,
) -> Self {
Self {
Expand Down
92 changes: 92 additions & 0 deletions src/signing/interactors/mono_factor_sign_request_input.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use crate::prelude::*;

/// A batch of transactions each batching over multiple keys (derivation paths)
/// to sign each transaction with.
#[derive(Clone, Debug, PartialEq, Eq, std::hash::Hash)]
pub struct MonoFactorSignRequestInput {
/// The ID of the factor source used to sign each per_transaction
pub factor_source_id: FactorSourceIDFromHash,

// The `factor_source_id` of each item must match `self.factor_source_id`.
pub per_transaction: Vec<TransactionSignRequestInput>,
}

impl MonoFactorSignRequestInput {
/// # Panics
/// Panics if `per_transaction` is empty
///
/// Also panics if `per_transaction` if the factor source id
/// of each request does not match `factor_source_id`.
pub fn new(
factor_source_id: FactorSourceIDFromHash,
per_transaction: IndexSet<TransactionSignRequestInput>,
) -> Self {
assert!(
!per_transaction.is_empty(),
"Invalid input. No transaction to sign, this is a programmer error."
);

assert!(per_transaction
.iter()
.all(|f| f.factor_source_id == factor_source_id), "Discprepancy! Input for one of the transactions has a mismatching FactorSourceID, this is a programmer error.");

Self {
factor_source_id,
per_transaction: per_transaction.into_iter().collect(),
}
}

pub fn factor_source_kind(&self) -> FactorSourceKind {
self.factor_source_id.kind
}
}

impl HasSampleValues for MonoFactorSignRequestInput {
fn sample() -> Self {
Self::new(
FactorSourceIDFromHash::sample(),
IndexSet::from_iter([TransactionSignRequestInput::sample()]),
)
}

fn sample_other() -> Self {
Self::new(
FactorSourceIDFromHash::sample_other(),
IndexSet::from_iter([TransactionSignRequestInput::sample_other()]),
)
}
}

#[cfg(test)]
mod tests {
use super::*;
type Sut = MonoFactorSignRequestInput;

#[test]
fn equality() {
assert_eq!(Sut::sample(), Sut::sample());
assert_eq!(Sut::sample_other(), Sut::sample_other());
}

#[test]
fn inequality() {
assert_ne!(Sut::sample(), Sut::sample_other());
}

#[test]
#[should_panic(expected = "Invalid input. No transaction to sign, this is a programmer error.")]
fn panics_if_per_transaction_is_empty() {
Sut::new(FactorSourceIDFromHash::sample(), IndexSet::new());
}

#[test]
#[should_panic(
expected = "Discprepancy! Input for one of the transactions has a mismatching FactorSourceID, this is a programmer error."
)]
fn panics_if_factor_source_mismatch() {
Sut::new(
FactorSourceIDFromHash::sample(),
IndexSet::from_iter([TransactionSignRequestInput::sample_other()]),
);
}
}
Loading

0 comments on commit 3ee7ed4

Please sign in to comment.