Skip to content

Commit

Permalink
Lock inputs only when submitting to allow preparing txs multiple times (
Browse files Browse the repository at this point in the history
#2085)

* Lock inputs only when submitting to allow preparing txs multiple times

* Add extra check to not sign txs with same inputs, add eyre auto-install feature
  • Loading branch information
Thoralf-M authored Mar 1, 2024
1 parent 37d52aa commit c713210
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 46 deletions.
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ dialoguer = { version = "0.11.0", default-features = false, features = [
"password",
] }
dotenvy = { version = "0.15.7", default-features = false }
eyre = { version = "0.6.12", default-features = false }
eyre = { version = "0.6.12", default-features = false, features = ["auto-install"] }
fern-logger = { version = "0.5.0", default-features = false }
log = { version = "0.4.20", default-features = false }
prefix-hex = { version = "0.7.1", default-features = false, features = ["std"] }
Expand Down
14 changes: 5 additions & 9 deletions sdk/src/wallet/operations/transaction/input_selection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ where
crate::wallet::Error: From<S::Error>,
crate::client::Error: From<S::Error>,
{
/// Selects inputs for a transaction and locks them in the wallet, so they don't get used again
/// Selects inputs for a transaction. Locking of the inputs only happens in `submit_and_store_transaction()`, so
/// calling this multiple times before submitting can result in conflicting transactions.
/// If this gets problematic we could add a bool in the TransactionOptions to lock them here already.
pub(crate) async fn select_inputs(
&self,
outputs: Vec<Output>,
Expand Down Expand Up @@ -60,8 +62,8 @@ where
RemainderValueStrategy::ReuseAddress => None,
RemainderValueStrategy::CustomAddress(address) => Some(address),
};
// lock so the same inputs can't be selected in multiple transactions
let mut wallet_ledger = self.ledger_mut().await;

let wallet_ledger = self.ledger().await;

#[cfg(feature = "events")]
self.emit(WalletEvent::TransactionProgress(
Expand Down Expand Up @@ -166,12 +168,6 @@ where

validate_transaction_length(&prepared_transaction_data.transaction)?;

// lock outputs so they don't get used by another transaction
for output in &prepared_transaction_data.inputs_data {
log::debug!("[TRANSACTION] locking: {}", output.output_id());
wallet_ledger.locked_outputs.insert(*output.output_id());
}

Ok(prepared_transaction_data)
}
}
Expand Down
50 changes: 24 additions & 26 deletions sdk/src/wallet/operations/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use crate::wallet::core::WalletLedgerDto;
use crate::{
client::{
api::{verify_semantic, PreparedTransactionData, SignedTransactionData},
secret::{types::InputSigningData, SecretManage},
secret::SecretManage,
Error,
},
types::block::{
output::{Output, OutputWithMetadata},
Expand Down Expand Up @@ -89,14 +90,19 @@ where
) -> crate::wallet::Result<TransactionWithMetadata> {
log::debug!("[TRANSACTION] sign_and_submit_transaction");

let signed_transaction_data = match self.sign_transaction(&prepared_transaction_data).await {
Ok(res) => res,
Err(err) => {
// unlock outputs so they are available for a new transaction
self.unlock_inputs(&prepared_transaction_data.inputs_data).await?;
return Err(err);
}
};
let wallet_ledger = self.ledger().await;
// check if inputs got already used by another transaction
for output in &prepared_transaction_data.inputs_data {
if wallet_ledger.locked_outputs.contains(output.output_id()) {
return Err(crate::wallet::Error::CustomInput(format!(
"provided input {} is already used in another transaction",
output.output_id()
)));
};
}
drop(wallet_ledger);

let signed_transaction_data = self.sign_transaction(&prepared_transaction_data).await?;

self.submit_and_store_transaction(signed_transaction_data, options)
.await
Expand Down Expand Up @@ -125,10 +131,16 @@ where
"[TRANSACTION] conflict: {conflict:?} for {:?}",
signed_transaction_data.payload
);
// unlock outputs so they are available for a new transaction
self.unlock_inputs(&signed_transaction_data.inputs_data).await?;
return Err(crate::client::Error::TransactionSemantic(conflict).into());
return Err(Error::TransactionSemantic(conflict).into());
}

let mut wallet_ledger = self.ledger_mut().await;
// lock outputs so they don't get used by another transaction
for output in &signed_transaction_data.inputs_data {
log::debug!("[TRANSACTION] locking: {}", output.output_id());
wallet_ledger.locked_outputs.insert(*output.output_id());
}
drop(wallet_ledger);

// Ignore errors from sending, we will try to send it again during [`sync_pending_transactions`]
let block_id = match self
Expand Down Expand Up @@ -187,18 +199,4 @@ where

Ok(transaction)
}

// unlock outputs
async fn unlock_inputs(&self, inputs: &[InputSigningData]) -> crate::wallet::Result<()> {
let mut wallet_ledger = self.ledger_mut().await;
for input_signing_data in inputs {
let output_id = input_signing_data.output_id();
wallet_ledger.locked_outputs.remove(output_id);
log::debug!(
"[TRANSACTION] Unlocked output {} because of transaction error",
output_id
);
}
Ok(())
}
}
12 changes: 2 additions & 10 deletions sdk/src/wallet/operations/transaction/sign_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,12 @@ where
}

let protocol_parameters = self.client().get_protocol_parameters().await?;
let unlocks = match self
let unlocks = self
.secret_manager
.read()
.await
.transaction_unlocks(prepared_transaction_data, &protocol_parameters)
.await
{
Ok(res) => res,
Err(err) => {
// unlock outputs so they are available for a new transaction
self.unlock_inputs(&prepared_transaction_data.inputs_data).await?;
return Err(err.into());
}
};
.await?;
let payload = SignedTransactionPayload::new(prepared_transaction_data.transaction.clone(), unlocks)?;

log::debug!("[TRANSACTION] signed transaction: {:?}", payload);
Expand Down

0 comments on commit c713210

Please sign in to comment.