Skip to content

Commit

Permalink
chore: deprecate select_notes function
Browse files Browse the repository at this point in the history
  • Loading branch information
elsirion committed Oct 28, 2024
1 parent 9665838 commit 0f7e329
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 17 deletions.
12 changes: 10 additions & 2 deletions fedimint-load-test-tool/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use fedimint_ln_client::{
LightningClientInit, LightningClientModule, LnPayState, OutgoingLightningPayment,
};
use fedimint_ln_common::LightningGateway;
use fedimint_mint_client::{MintClientInit, MintClientModule, MintCommonInit, OOBNotes};
use fedimint_mint_client::{
MintClientInit, MintClientModule, MintCommonInit, OOBNotes, SelectNotesWithAtleastAmount,
};
use fedimint_wallet_client::WalletClientInit;
use futures::StreamExt;
use lightning_invoice::Bolt11Invoice;
Expand Down Expand Up @@ -91,7 +93,13 @@ pub async fn do_spend_notes(
) -> anyhow::Result<(OperationId, OOBNotes)> {
let mint = &mint.get_first_module::<MintClientModule>()?;
let (operation_id, oob_notes) = mint
.spend_notes(amount, Duration::from_secs(600), false, ())
.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
amount,
Duration::from_secs(600),
false,
(),
)
.await?;
let mut updates = mint
.subscribe_spend_notes(operation_id)
Expand Down
20 changes: 17 additions & 3 deletions fedimint-wasm-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ mod tests {
};
use fedimint_ln_common::lightning_invoice::{Bolt11InvoiceDescription, Description};
use fedimint_ln_common::LightningGateway;
use fedimint_mint_client::{MintClientModule, ReissueExternalNotesState, SpendOOBState};
use fedimint_mint_client::{
MintClientModule, ReissueExternalNotesState, SelectNotesWithAtleastAmount, SpendOOBState,
};
use futures::StreamExt;
use wasm_bindgen_test::wasm_bindgen_test;

Expand Down Expand Up @@ -262,7 +264,13 @@ mod tests {
) -> Result<(), anyhow::Error> {
let mint = client.get_first_module::<MintClientModule>()?;
let (_, notes) = mint
.spend_notes(Amount::from_sats(11), Duration::from_secs(10000), false, ())
.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
Amount::from_sats(11),
Duration::from_secs(10000),
false,
(),
)
.await?;
let operation_id = mint.reissue_external_notes(notes, ()).await?;
let mut updates = mint
Expand Down Expand Up @@ -291,7 +299,13 @@ mod tests {
let mint = client.get_first_module::<MintClientModule>()?;
'retry: loop {
let (operation_id, notes) = mint
.spend_notes(amount, Duration::from_secs(10000), false, ())
.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
amount,
Duration::from_secs(10000),
false,
(),
)
.await?;
if notes.total_amount() == amount {
return Ok(());
Expand Down
40 changes: 33 additions & 7 deletions modules/fedimint-mint-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ pub enum ReissueExternalNotesState {
}

/// The high-level state of a raw e-cash spend operation started with
/// [`MintClientModule::spend_notes`].
/// [`MintClientModule::spend_notes_with_selector`].
#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
pub enum SpendOOBState {
/// The e-cash has been selected and given to the caller
Expand Down Expand Up @@ -869,7 +869,13 @@ impl ClientModule for MintClientModule {
}
"spend_notes_expert" => {
let req: SpendNotesExpertRequest = serde_json::from_value(request)?;
let result = self.spend_notes(req.min_amount, req.try_cancel_after, req.include_invite, req.extra_meta).await?;
let result = self.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
req.min_amount,
req.try_cancel_after,
req.include_invite,
req.extra_meta
).await?;
yield serde_json::to_value(result)?;
}
"validate_notes" => {
Expand Down Expand Up @@ -1567,6 +1573,10 @@ impl MintClientModule {
/// should be chosen such that the recipient (who is potentially offline at
/// the time of receiving the e-cash notes) had a reasonable timeframe to
/// come online and reissue the notes themselves.
#[deprecated(
since = "0.5.0",
note = "Use `spend_notes_with_selector` instead, with `SelectNotesWithAtleastAmount` to maintain the same behavior"
)]
pub async fn spend_notes<M: Serialize + Send>(
&self,
min_amount: Amount,
Expand All @@ -1584,7 +1594,21 @@ impl MintClientModule {
.await
}

/// Same as `spend_notes` but allows different to select notes to be used.
/// Fetches and removes notes from the wallet to be sent to the recipient
/// out of band. The not selection algorithm is determined by
/// `note_selector`. See the [`NotesSelector`] trait for available
/// implementations.
///
/// These spends can be canceled by calling
/// [`MintClientModule::try_cancel_spend_notes`] as long
/// as the recipient hasn't reissued the e-cash notes themselves yet.
///
/// The client will also automatically attempt to cancel the operation after
/// `try_cancel_after` time has passed. This is a safety mechanism to avoid
/// users forgetting about failed out-of-band transactions. The timeout
/// should be chosen such that the recipient (who is potentially offline at
/// the time of receiving the e-cash notes) had a reasonable timeframe to
/// come online and reissue the notes themselves.
pub async fn spend_notes_with_selector<M: Serialize + Send>(
&self,
notes_selector: &impl NotesSelector,
Expand Down Expand Up @@ -1692,9 +1716,9 @@ impl MintClientModule {
}

/// Try to cancel a spend operation started with
/// [`MintClientModule::spend_notes`]. If the e-cash notes have already been
/// spent this operation will fail which can be observed using
/// [`MintClientModule::subscribe_spend_notes`].
/// [`MintClientModule::spend_notes_with_selector`]. If the e-cash notes
/// have already been spent this operation will fail which can be
/// observed using [`MintClientModule::subscribe_spend_notes`].
pub async fn try_cancel_spend_notes(&self, operation_id: OperationId) {
let mut dbtx = self.client_ctx.module_db().begin_transaction().await;
dbtx.insert_entry(&CancelledOOBSpendKey(operation_id), &())
Expand All @@ -1705,7 +1729,7 @@ impl MintClientModule {
}

/// Subscribe to updates on the progress of a raw e-cash spend operation
/// started with [`MintClientModule::spend_notes`].
/// started with [`MintClientModule::spend_notes_with_selector`].
pub async fn subscribe_spend_notes(
&self,
operation_id: OperationId,
Expand Down Expand Up @@ -1833,6 +1857,8 @@ pub struct SpendOOBRefund {
pub transaction_id: TransactionId,
}

/// Defines a strategy for selecting e-cash notes given a specific target amount
/// and fee per note transaction input.
#[apply(async_trait_maybe_send!)]
pub trait NotesSelector<Note = SpendableNoteUndecoded>: Send + Sync {
/// Select notes from stream for requested_amount.
Expand Down
23 changes: 18 additions & 5 deletions modules/fedimint-mint-tests/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use fedimint_dummy_common::config::DummyGenParams;
use fedimint_dummy_server::DummyInit;
use fedimint_logging::LOG_TEST;
use fedimint_mint_client::{
MintClientInit, MintClientModule, Note, OOBNotes, ReissueExternalNotesState, SpendOOBState,
MintClientInit, MintClientModule, Note, OOBNotes, ReissueExternalNotesState,
SelectNotesWithAtleastAmount, SpendOOBState,
};
use fedimint_mint_common::config::{FeeConsensus, MintGenParams, MintGenParamsConsensus};
use fedimint_mint_common::{MintInput, MintInputV0, Nonce};
Expand Down Expand Up @@ -113,7 +114,7 @@ async fn sends_ecash_out_of_band() -> anyhow::Result<()> {
let client2_mint = client2.get_first_module::<MintClientModule>()?;
info!("### SPEND NOTES");
let (op, notes) = client1_mint
.spend_notes(sats(750), TIMEOUT, false, ())
.spend_notes_with_selector(&SelectNotesWithAtleastAmount, sats(750), TIMEOUT, false, ())
.await?;
let sub1 = &mut client1_mint.subscribe_spend_notes(op).await?.into_stream();
assert_eq!(sub1.ok().await?, SpendOOBState::Created);
Expand Down Expand Up @@ -165,7 +166,13 @@ async fn sends_ecash_oob_highly_parallel() -> anyhow::Result<()> {
info!("Starting spend {num_spend}");
let client1_mint = task_client1.get_first_module::<MintClientModule>().unwrap();
let (op, notes) = client1_mint
.spend_notes(sats(30), ECASH_TIMEOUT, false, ())
.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
sats(30),
ECASH_TIMEOUT,
false,
(),
)
.await
.unwrap();
let sub1 = &mut client1_mint
Expand Down Expand Up @@ -290,7 +297,7 @@ async fn sends_ecash_out_of_band_cancel() -> anyhow::Result<()> {
// Spend from client1 to client2
let mint_module = client.get_first_module::<MintClientModule>()?;
let (op, _) = mint_module
.spend_notes(sats(750), TIMEOUT, false, ())
.spend_notes_with_selector(&SelectNotesWithAtleastAmount, sats(750), TIMEOUT, false, ())
.await?;
let sub1 = &mut mint_module.subscribe_spend_notes(op).await?.into_stream();
assert_eq!(sub1.ok().await?, SpendOOBState::Created);
Expand Down Expand Up @@ -327,7 +334,13 @@ async fn error_zero_value_oob_spend() -> anyhow::Result<()> {
// Spend from client1 to client2
let err_msg = client1
.get_first_module::<MintClientModule>()?
.spend_notes(Amount::ZERO, TIMEOUT, false, ())
.spend_notes_with_selector(
&SelectNotesWithAtleastAmount,
Amount::ZERO,
TIMEOUT,
false,
(),
)
.await
.expect_err("Zero-amount spends should be forbidden")
.to_string();
Expand Down

0 comments on commit 0f7e329

Please sign in to comment.