Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
1 change: 0 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ jobs:
run: RUST_BACKTRACE=1 cargo test --workspace --exclude ark-secp256k1

e2e-tests:
needs: [ clippy ]
strategy:
fail-fast: false
matrix:
Expand Down
29 changes: 22 additions & 7 deletions ark-client/src/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use ark_core::proof_of_funds;
use ark_core::server::BatchTreeEventType;
use ark_core::server::StreamEvent;
use ark_core::ArkAddress;
use ark_core::ArkNote;
use ark_core::TxGraph;
use backon::ExponentialBuilder;
use backon::Retryable;
Expand Down Expand Up @@ -76,6 +77,7 @@ where
&mut rng.clone(),
boarding_inputs.clone(),
vtxo_inputs.clone(),
vec![],
BatchOutputType::Board {
to_address,
to_amount: total_amount,
Expand Down Expand Up @@ -136,6 +138,7 @@ where
&mut rng.clone(),
boarding_inputs.clone(),
vtxo_inputs.clone(),
vec![],
BatchOutputType::OffBoard {
to_address: to_address.clone(),
to_amount,
Expand Down Expand Up @@ -248,17 +251,18 @@ where
Ok((boarding_inputs, vtxo_inputs, total_amount))
}

async fn join_next_batch<R>(
pub(crate) async fn join_next_batch<R>(
&self,
rng: &mut R,
onchain_inputs: Vec<batch::OnChainInput>,
vtxo_inputs: Vec<batch::VtxoInput>,
arknotes: Vec<ArkNote>,
Comment on lines +254 to +259
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we just continue the pattern of using the settle API to settle everything?

I do think that it might also be useful to create another API which lets you choose what you are settling, but I'm not sure we want to be adding redeem_notes/redeem_note. To me it's more natural to settle everything (including Ark notes) with settle.

output_type: BatchOutputType,
) -> Result<Txid, Error>
where
R: Rng + CryptoRng,
{
if onchain_inputs.is_empty() && vtxo_inputs.is_empty() {
if onchain_inputs.is_empty() && vtxo_inputs.is_empty() && arknotes.is_empty() {
return Err(Error::ad_hoc("cannot join batch without inputs"));
}

Expand Down Expand Up @@ -304,7 +308,14 @@ where
)
});

boarding_inputs.chain(vtxo_inputs).collect::<Vec<_>>()
let arknotes = arknotes
.iter()
.map(|n| n.into())
.collect::<Vec<proof_of_funds::Input>>();
boarding_inputs
Comment on lines +311 to +315
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use fallible conversion from ArkNote to Input

After making Input a TryFrom<&ArkNote> (see core comment), update this site to handle errors instead of panicking.

Apply this diff:

-            let arknotes = arknotes
-                .iter()
-                .map(|n| n.into())
-                .collect::<Vec<proof_of_funds::Input>>();
+            let arknotes: Vec<proof_of_funds::Input> = arknotes
+                .iter()
+                .map(|n| <proof_of_funds::Input as core::convert::TryFrom<&ArkNote>>::try_from(n))
+                .collect::<Result<_, ark_core::Error>>()?
+                ;

If you prefer less verbosity:

+            use core::convert::TryFrom;
+            let arknotes: Vec<proof_of_funds::Input> =
+                arknotes.iter().map(proof_of_funds::Input::try_from)
+                .collect::<Result<_, ark_core::Error>>()?;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let arknotes = arknotes
.iter()
.map(|n| n.into())
.collect::<Vec<proof_of_funds::Input>>();
boarding_inputs
// Convert ArkNote to Input using fallible TryFrom and propagate any error
- let arknotes = arknotes
- .iter()
- .map(|n| n.into())
let arknotes: Vec<proof_of_funds::Input> = arknotes
.iter()
.map(|n| <proof_of_funds::Input as core::convert::TryFrom<&ArkNote>>::try_from(n))
.collect::<Result<_, ark_core::Error>>()?;
boarding_inputs
🤖 Prompt for AI Agents
In ark-client/src/batch.rs around lines 311 to 315, the code currently uses
.map(|n| n.into()).collect::<Vec<...>>() but Input now implements
TryFrom<&ArkNote>, so replace the infallible conversion with a fallible one: map
each ArkNote with Input::try_from (or TryFrom::try_from) and collect into a
Result<Vec<proof_of_funds::Input>, E> (i.e. collect::<Result<Vec<_>, _>>() )
then propagate or handle the error (use the ? operator to return the error from
the current function or convert it to the appropriate error type). Ensure
function signature supports returning Result if it does not already.

.chain(vtxo_inputs)
.chain(arknotes)
.collect::<Vec<_>>()
};

let mut outputs = vec![];
Expand Down Expand Up @@ -689,9 +700,13 @@ where
Some(commitment_psbt)
};

network_client
.submit_signed_forfeit_txs(signed_forfeit_psbts, commitment_psbt)
.await?;
// Only submit forfeit transactions if we have actual inputs that require
// them ArkNotes don't require forfeit transactions
if !signed_forfeit_psbts.is_empty() || commitment_psbt.is_some() {
network_client
.submit_signed_forfeit_txs(signed_forfeit_psbts, commitment_psbt)
.await?;
}

step = step.next();
}
Expand Down Expand Up @@ -753,7 +768,7 @@ where
}
}

enum BatchOutputType {
pub(crate) enum BatchOutputType {
Board {
to_address: ArkAddress,
to_amount: Amount,
Expand Down
81 changes: 81 additions & 0 deletions ark-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use ark_core::server::GetVtxosRequest;
use ark_core::server::SubscriptionResponse;
use ark_core::server::VirtualTxOutPoint;
use ark_core::ArkAddress;
use ark_core::ArkNote;
use ark_core::UtxoCoinSelection;
use ark_core::Vtxo;
use ark_grpc::VtxoChainResponse;
Expand All @@ -27,6 +28,8 @@ use bitcoin::Txid;
use futures::Future;
use futures::Stream;
use jiff::Timestamp;
use rand::CryptoRng;
use rand::Rng;
use std::sync::Arc;
use std::time::Duration;

Expand All @@ -39,6 +42,7 @@ mod send_vtxo;
mod unilateral_exit;
mod utils;

use batch::BatchOutputType;
pub use error::Error;

/// A client to interact with Ark Server
Expand Down Expand Up @@ -527,6 +531,83 @@ where
Ok(sum)
}

/// Redeem multiple ArkNotes by settling them into new VTXOs
///
/// This method takes ArkNote objects and creates a batch transaction to convert
/// them into spendable VTXOs at the user's offchain address.
pub async fn redeem_notes<R>(
&self,
rng: &mut R,
arknotes: Vec<ArkNote>,
) -> Result<Option<Txid>, Error>
Comment on lines +534 to +542
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess ark-client should be able to keep track of your Ark notes for you. It's weird to have to pass in the notes from the outside.

Having said that, it might still be useful to expose a method which lets you pick which VTXOs/boarding outputs/notes to settle.

where
R: CryptoRng + Rng + Clone,
{
if arknotes.is_empty() {
return Ok(None);
}

// Calculate total amount from all notes
let total_amount = arknotes
.iter()
.fold(Amount::ZERO, |acc, note| acc + note.value());

// Get user's offchain address to send the redeemed funds
let (to_address, _) = self.get_offchain_address()?;

tracing::info!(
note_count = arknotes.len(),
total_amount = %total_amount,
to_address = %to_address.encode(),
"Redeeming ArkNotes"
);

// Join the next batch with the ArkNotes as inputs
self.join_next_batch(
rng,
vec![],
vec![],
arknotes,
BatchOutputType::Board {
to_address,
to_amount: total_amount,
},
)
.await
.map(Some)
}

/// Redeem a single ArkNote
pub async fn redeem_note<R>(&self, rng: &mut R, arknote: ArkNote) -> Result<Option<Txid>, Error>
where
R: CryptoRng + Rng + Clone,
{
self.redeem_notes(rng, vec![arknote]).await
}

pub async fn create_arknote(&self, amount: Amount) -> Result<ArkNote, Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

AFAIK, this should not be part of the public API of the Client, because it only makes sense to use this for testing (it calls an admin API on the server).

I'm not against using it in the tests if we have to, but this only needs the GRPC client and doesn't use any of the ark_client::Client's state. I think this should just be a function defined in e2e-tests.

let notes = self
.inner
.network_client
.clone()
.create_arknote(amount.to_sat() as u32, 1)
.await
.map_err(Error::ad_hoc)?;

if notes.is_empty() {
return Err(Error::ad_hoc("No notes returned from server"));
}

let note_str = notes
.into_iter()
.next()
.ok_or_else(|| Error::ad_hoc("No note in response"))?;

tracing::info!(note = %note_str, "Created ArkNote");
let note = ArkNote::from_string(&note_str).map_err(Error::ad_hoc)?;
Ok(note)
}
Comment on lines +606 to +609
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Do not log full ArkNote string (leaks the preimage/secret)

note_str encodes the preimage. Logging it at info level leaks spend material.

Apply this diff to log non-sensitive fields after parsing:

-        tracing::info!(note = %note_str, "Created ArkNote");
-        let note = ArkNote::from_string(&note_str).map_err(Error::ad_hoc)?;
+        let note = ArkNote::from_string(&note_str).map_err(Error::ad_hoc)?;
+        tracing::info!(txid = %note.txid(), value = %note.value(), "Created ArkNote");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tracing::info!(note = %note_str, "Created ArkNote");
let note = ArkNote::from_string(&note_str).map_err(Error::ad_hoc)?;
Ok(note)
}
let note = ArkNote::from_string(&note_str).map_err(Error::ad_hoc)?;
tracing::info!(txid = %note.txid(), value = %note.value(), "Created ArkNote");
Ok(note)
🤖 Prompt for AI Agents
In ark-client/src/lib.rs around lines 606 to 609, do not log the raw note_str
(it contains the preimage/secret). Instead, first parse note_str into an ArkNote
and then remove or replace the tracing::info! call so it only logs non-sensitive
derived fields (e.g., commitment, note type, amount or other public identifiers)
after parsing; ensure the log no longer includes note_str or any preimage/secret
material and move logging to after the ArkNote::from_string(...) call so you can
access and log only safe fields.


pub async fn transaction_history(&self) -> Result<Vec<history::Transaction>, Error> {
let mut boarding_transactions = Vec::new();
let mut boarding_commitment_transactions = Vec::new();
Expand Down
2 changes: 2 additions & 0 deletions ark-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ description = "Core types and utilities for Ark"
[dependencies]
bech32 = "0.11"
bitcoin = { version = "0.32.4", features = ["base64", "rand", "serde"] }
bs58 = "0.5"
hex = "0.4"
musig = { package = "ark-secp256k1", path = "../ark-rust-secp256k1", features = ["serde", "rand"] }
rand = "0.8"
serde = { version = "1.0", features = ["derive"] }
Expand Down
Loading
Loading