Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(wallet): allow generating transactions without persisting change addresses #2385

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion wallet/src/actors/app/handlers/create_data_req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub struct CreateDataReqRequest {
#[serde(deserialize_with = "deserialize_fee_backwards_compatible")]
fee: Fee,
fee_type: Option<FeeType>,
#[serde(default)]
preview: bool,
}

#[derive(Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -81,7 +83,11 @@ impl Handler<CreateDataReqRequest> for app::App {
let fee = fee_compat(msg.fee, msg.fee_type);

let f = fut::result(validated).and_then(move |request, slf: &mut Self, _ctx| {
let params = types::DataReqParams { request, fee };
let params = types::DataReqParams {
request,
fee,
preview: msg.preview,
};

slf.create_data_req(&msg.session_id, &msg.wallet_id, params)
.map_ok(
Expand Down
3 changes: 3 additions & 0 deletions wallet/src/actors/app/handlers/create_vtt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub struct CreateVttRequest {
utxo_strategy: UtxoSelectionStrategy,
#[serde(default)]
selected_utxos: HashSet<OutputPointer>,
#[serde(default)]
preview: bool,
}

/// Part of CreateVttResponse struct, containing additional data to be displayed in clients
Expand Down Expand Up @@ -124,6 +126,7 @@ impl Handler<CreateVttRequest> for app::App {
outputs,
utxo_strategy: msg.utxo_strategy.clone(),
selected_utxos: msg.selected_utxos.iter().map(|x| x.into()).collect(),
preview: msg.preview,
};

act.create_vtt(&msg.session_id, &msg.wallet_id, params)
Expand Down
2 changes: 1 addition & 1 deletion wallet/src/actors/app/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ macro_rules! routes {
{
let api_addr = $api.clone();
$io.add_method($method_jsonrpc, move |params: Params| {
log::debug!("Handling request for method: {}", $method_jsonrpc);
log::debug!("Handling request for method {}: {:?}", $method_jsonrpc, params);
let addr = api_addr.clone();
// Try to parse the request params into the actor message
let fut03 = future::ready(params.parse::<$actor_msg>())
Expand Down
2 changes: 1 addition & 1 deletion wallet/src/actors/worker/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl Worker {
let address = if external {
wallet.gen_external_address(label)?
} else {
wallet.gen_internal_address(label)?
wallet.gen_internal_address(label, false)?
};

Ok(address)
Expand Down
51 changes: 43 additions & 8 deletions wallet/src/repository/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,14 @@ where
}

/// Generate an address in the internal keychain (WIP-0001).
pub fn gen_internal_address(&self, label: Option<String>) -> Result<Arc<model::Address>> {
pub fn gen_internal_address(
&self,
label: Option<String>,
preview: bool,
) -> Result<Arc<model::Address>> {
let mut state = self.state.write()?;

self._gen_internal_address(&mut state, label)
self._gen_internal_address(&mut state, label, preview)
}

/// Return a list of the generated external addresses that.
Expand Down Expand Up @@ -1082,6 +1086,7 @@ where
outputs,
utxo_strategy,
selected_utxos,
preview,
}: types::VttParams,
) -> Result<(model::ExtendedTransaction, AbsoluteFee)> {
let mut state = self.state.write()?;
Expand All @@ -1095,6 +1100,7 @@ where
fee,
&utxo_strategy,
selected_utxos,
preview,
)?;

let pointers_as_inputs = inputs
Expand All @@ -1118,14 +1124,18 @@ where

pub fn create_data_req(
&self,
types::DataReqParams { fee, request }: types::DataReqParams,
types::DataReqParams {
fee,
request,
preview,
}: types::DataReqParams,
) -> Result<(model::ExtendedTransaction, AbsoluteFee)> {
let mut state = self.state.write()?;
let TransactionComponents {
fee,
inputs,
outputs,
} = self.create_dr_transaction_components(&mut state, request.clone(), fee)?;
} = self.create_dr_transaction_components(&mut state, request.clone(), fee, preview)?;

let pointers_as_inputs = inputs
.pointers
Expand Down Expand Up @@ -1190,6 +1200,7 @@ where
fee: Fee,
utxo_strategy: &UtxoSelectionStrategy,
selected_utxos: HashSet<model::OutPtr>,
preview: bool,
) -> Result<TransactionComponents> {
let timestamp = u64::try_from(get_timestamp()).unwrap();

Expand All @@ -1203,6 +1214,7 @@ where
utxo_strategy,
self.params.max_vt_weight,
selected_utxos,
preview,
)
}

Expand All @@ -1211,6 +1223,7 @@ where
state: &mut State,
request: DataRequestOutput,
fee: Fee,
preview: bool,
) -> Result<TransactionComponents> {
let utxo_strategy = UtxoSelectionStrategy::Random { from: None };
let timestamp = u64::try_from(get_timestamp()).unwrap();
Expand All @@ -1225,6 +1238,7 @@ where
&utxo_strategy,
self.params.max_dr_weight,
HashSet::default(),
preview,
)
}

Expand All @@ -1233,14 +1247,15 @@ where
&self,
state: &mut State,
force_input: Option<Input>,
preview: bool,
) -> Result<PublicKeyHash> {
let pkh = if let Some(input) = force_input {
let forced = input.output_pointer();
let key_balance = state.utxo_set.get(&forced.into()).unwrap();

key_balance.pkh
} else {
self._gen_internal_address(state, None)?.pkh
self._gen_internal_address(state, None, preview)?.pkh
};

Ok(pkh)
Expand All @@ -1263,6 +1278,7 @@ where
utxo_strategy: &UtxoSelectionStrategy,
max_weight: u32,
selected_utxos: HashSet<model::OutPtr>,
preview: bool,
) -> Result<TransactionComponents> {
let empty_hashset = HashSet::default();
let unconfirmed_transactions = if self.params.use_unconfirmed_utxos {
Expand Down Expand Up @@ -1298,6 +1314,7 @@ where
let change_pkh = self.calculate_change_address(
state,
dr_output.and_then(|_| inputs.pointers.get(0).cloned().map(Input::new)),
preview,
)?;

let mut outputs = outputs;
Expand All @@ -1318,16 +1335,22 @@ where
&self,
state: &mut State,
label: Option<String>,
preview: bool,
) -> Result<Arc<model::Address>> {
let keychain = constants::INTERNAL_KEYCHAIN;
let account = state.account;
let index = state.next_internal_index;
let parent_key = &state.keychains[keychain as usize];

// `preview` is negated because it turns into `persist_db`
let (address, next_index) =
self.derive_and_persist_address(label, parent_key, account, keychain, index, true)?;
self.derive_and_persist_address(label, parent_key, account, keychain, index, !preview)?;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key point here. This will avoid persisting the change address if the transaction is being generated solely for preview purposes (e.g. estimating fees).


state.next_internal_index = next_index;
// Don't advance the internal index if we are simply previewing
if !preview {
state.next_internal_index = next_index;
log::debug!("Internal keychain advanced to index #{next_index}");
}

Ok(address)
}
Expand Down Expand Up @@ -1443,6 +1466,18 @@ where
// - Cannot borrow `state` as mutable because it is also borrowed as immutable
let state = &mut *state;

// Move internal keychain forward if we used a new change address
let next = self._gen_internal_address(state, None, true)?;
if match &txn.transaction {
Transaction::ValueTransfer(vtt) => {
vtt.body.outputs.iter().any(|vto| vto.pkh == next.pkh)
}
Transaction::DataRequest(dr) => dr.body.outputs.iter().any(|vto| vto.pkh == next.pkh),
_ => false,
} {
let _ = self._gen_internal_address(state, None, false);
}

// Mark UTXOs as used so we don't double spend
// Save the timestamp to after which the UTXO can be spent again
let tx_pending_timeout = self.params.pending_transactions_timeout_seconds;
Expand Down Expand Up @@ -1569,7 +1604,7 @@ where
state.transient_external_addresses.remove(&addr.pkh);
}
for _ in state.next_internal_index..new_internal_index {
let addr = self._gen_internal_address(&mut state, None)?;
let addr = self._gen_internal_address(&mut state, None, false)?;
state.transient_internal_addresses.remove(&addr.pkh);
}

Expand Down
Loading