Skip to content

Commit 724654f

Browse files
OttoAllmendingerllm-git
andcommitted
feat(wasm-utxo): refactor transaction parsing with better error handling
Refactor BitGoPsbt transaction parsing methods to use functional style with map/filter operations for improved readability and error handling. Extract helper methods for summing input and output values to reduce code duplication. Issue: BTC-2652 Co-authored-by: llm-git <[email protected]>
1 parent df8d870 commit 724654f

File tree

1 file changed

+111
-82
lines changed
  • packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt

1 file changed

+111
-82
lines changed

packages/wasm-utxo/src/fixed_script_wallet/bitgo_psbt/mod.rs

Lines changed: 111 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -284,15 +284,13 @@ impl BitGoPsbt {
284284
) -> Result<(), Vec<String>> {
285285
let num_inputs = self.psbt().inputs.len();
286286

287-
let mut errors = vec![];
288-
for index in 0..num_inputs {
289-
match self.finalize_input(secp, index) {
290-
Ok(()) => {}
291-
Err(e) => {
292-
errors.push(format!("Input {}: {}", index, e));
293-
}
294-
}
295-
}
287+
let errors: Vec<String> = (0..num_inputs)
288+
.filter_map(|index| {
289+
self.finalize_input(secp, index)
290+
.err()
291+
.map(|e| format!("Input {}: {}", index, e))
292+
})
293+
.collect();
296294

297295
if errors.is_empty() {
298296
Ok(())
@@ -463,6 +461,44 @@ impl BitGoPsbt {
463461
}
464462
}
465463

464+
/// Parse inputs with wallet keys and replay protection
465+
///
466+
/// # Arguments
467+
/// - `wallet_keys`: The wallet's root keys for deriving scripts
468+
/// - `replay_protection`: Scripts that are allowed as inputs without wallet validation
469+
///
470+
/// # Returns
471+
/// - `Ok(Vec<ParsedInput>)` with parsed inputs
472+
/// - `Err(ParseTransactionError)` if input parsing fails
473+
fn parse_inputs(
474+
&self,
475+
wallet_keys: &crate::fixed_script_wallet::RootWalletKeys,
476+
replay_protection: &psbt_wallet_input::ReplayProtection,
477+
) -> Result<Vec<ParsedInput>, ParseTransactionError> {
478+
let psbt = self.psbt();
479+
let network = self.network();
480+
481+
psbt.unsigned_tx
482+
.input
483+
.iter()
484+
.zip(psbt.inputs.iter())
485+
.enumerate()
486+
.map(|(input_index, (tx_input, psbt_input))| {
487+
ParsedInput::parse(
488+
psbt_input,
489+
tx_input,
490+
wallet_keys,
491+
replay_protection,
492+
network,
493+
)
494+
.map_err(|error| ParseTransactionError::Input {
495+
index: input_index,
496+
error,
497+
})
498+
})
499+
.collect()
500+
}
501+
466502
/// Parse outputs with wallet keys to identify which outputs belong to the wallet
467503
///
468504
/// # Arguments
@@ -482,22 +518,69 @@ impl BitGoPsbt {
482518
let psbt = self.psbt();
483519
let network = self.network();
484520

485-
let mut parsed_outputs = Vec::new();
486-
487-
for (output_index, tx_output) in psbt.unsigned_tx.output.iter().enumerate() {
488-
let psbt_output = &psbt.outputs[output_index];
521+
psbt.unsigned_tx
522+
.output
523+
.iter()
524+
.zip(psbt.outputs.iter())
525+
.enumerate()
526+
.map(|(output_index, (tx_output, psbt_output))| {
527+
ParsedOutput::parse(psbt_output, tx_output, wallet_keys, network).map_err(|error| {
528+
ParseTransactionError::Output {
529+
index: output_index,
530+
error,
531+
}
532+
})
533+
})
534+
.collect()
535+
}
489536

490-
// Parse the output
491-
let parsed_output = ParsedOutput::parse(psbt_output, tx_output, wallet_keys, network)
492-
.map_err(|error| ParseTransactionError::Output {
493-
index: output_index,
494-
error,
495-
})?;
537+
/// Calculate total input value from parsed inputs
538+
///
539+
/// # Returns
540+
/// - `Ok(u64)` with total input value
541+
/// - `Err(ParseTransactionError)` if overflow occurs
542+
fn sum_input_values(parsed_inputs: &[ParsedInput]) -> Result<u64, ParseTransactionError> {
543+
parsed_inputs
544+
.iter()
545+
.enumerate()
546+
.try_fold(0u64, |total, (index, input)| {
547+
total
548+
.checked_add(input.value)
549+
.ok_or(ParseTransactionError::InputValueOverflow { index })
550+
})
551+
}
496552

497-
parsed_outputs.push(parsed_output);
498-
}
553+
/// Calculate total output value and spend amount from transaction outputs and parsed outputs
554+
///
555+
/// # Returns
556+
/// - `Ok((total_value, spend_amount))` with total output value and external spend amount
557+
/// - `Err(ParseTransactionError)` if overflow occurs
558+
fn sum_output_values(
559+
tx_outputs: &[miniscript::bitcoin::TxOut],
560+
parsed_outputs: &[ParsedOutput],
561+
) -> Result<(u64, u64), ParseTransactionError> {
562+
tx_outputs
563+
.iter()
564+
.zip(parsed_outputs.iter())
565+
.enumerate()
566+
.try_fold(
567+
(0u64, 0u64),
568+
|(total_value, spend), (index, (tx_output, parsed_output))| {
569+
let new_total = total_value
570+
.checked_add(tx_output.value.to_sat())
571+
.ok_or(ParseTransactionError::OutputValueOverflow { index })?;
572+
573+
let new_spend = if parsed_output.is_external() {
574+
spend
575+
.checked_add(tx_output.value.to_sat())
576+
.ok_or(ParseTransactionError::SpendAmountOverflow { index })?
577+
} else {
578+
spend
579+
};
499580

500-
Ok(parsed_outputs)
581+
Ok((new_total, new_spend))
582+
},
583+
)
501584
}
502585

503586
/// Parse outputs with wallet keys to identify which outputs belong to a particular wallet.
@@ -539,69 +622,15 @@ impl BitGoPsbt {
539622
replay_protection: &psbt_wallet_input::ReplayProtection,
540623
) -> Result<ParsedTransaction, ParseTransactionError> {
541624
let psbt = self.psbt();
542-
let network = self.network();
543625

544-
// Parse inputs
545-
let mut parsed_inputs = Vec::new();
546-
let mut total_input_value = 0u64;
547-
548-
for (input_index, (tx_input, psbt_input)) in psbt
549-
.unsigned_tx
550-
.input
551-
.iter()
552-
.zip(psbt.inputs.iter())
553-
.enumerate()
554-
{
555-
// Parse the input
556-
let parsed_input = ParsedInput::parse(
557-
psbt_input,
558-
tx_input,
559-
wallet_keys,
560-
replay_protection,
561-
network,
562-
)
563-
.map_err(|error| ParseTransactionError::Input {
564-
index: input_index,
565-
error,
566-
})?;
567-
568-
// Add value to total, checking for overflow
569-
total_input_value = total_input_value
570-
.checked_add(parsed_input.value)
571-
.ok_or(ParseTransactionError::InputValueOverflow { index: input_index })?;
572-
573-
parsed_inputs.push(parsed_input);
574-
}
575-
576-
// Parse outputs using the reusable method
626+
// Parse inputs and outputs
627+
let parsed_inputs = self.parse_inputs(wallet_keys, replay_protection)?;
577628
let parsed_outputs = self.parse_outputs(wallet_keys)?;
578629

579-
// Calculate totals and spend amount
580-
let mut total_output_value = 0u64;
581-
let mut spend_amount = 0u64;
582-
583-
for (output_index, (tx_output, parsed_output)) in psbt
584-
.unsigned_tx
585-
.output
586-
.iter()
587-
.zip(parsed_outputs.iter())
588-
.enumerate()
589-
{
590-
total_output_value = total_output_value
591-
.checked_add(tx_output.value.to_sat())
592-
.ok_or(ParseTransactionError::OutputValueOverflow {
593-
index: output_index,
594-
})?;
595-
596-
// If this is an external output, add to spend amount
597-
if parsed_output.is_external() {
598-
spend_amount = spend_amount.checked_add(tx_output.value.to_sat()).ok_or(
599-
ParseTransactionError::SpendAmountOverflow {
600-
index: output_index,
601-
},
602-
)?;
603-
}
604-
}
630+
// Calculate totals
631+
let total_input_value = Self::sum_input_values(&parsed_inputs)?;
632+
let (total_output_value, spend_amount) =
633+
Self::sum_output_values(&psbt.unsigned_tx.output, &parsed_outputs)?;
605634

606635
// Calculate miner fee
607636
let miner_fee = total_input_value

0 commit comments

Comments
 (0)