-
Notifications
You must be signed in to change notification settings - Fork 21
ark-core: implement ArkNote encoding and cross-language validation #98
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
bf0d3d8
2fedc9d
eb97995
11a021e
ddb37e2
d485786
2bc8697
e71c732
cc1a886
6a717c8
264efc7
8b0ee15
9225c47
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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; | ||||||||||||||||||||||||||||||
|
@@ -76,6 +77,7 @@ where | |||||||||||||||||||||||||||||
&mut rng.clone(), | ||||||||||||||||||||||||||||||
boarding_inputs.clone(), | ||||||||||||||||||||||||||||||
vtxo_inputs.clone(), | ||||||||||||||||||||||||||||||
vec![], | ||||||||||||||||||||||||||||||
BatchOutputType::Board { | ||||||||||||||||||||||||||||||
to_address, | ||||||||||||||||||||||||||||||
to_amount: total_amount, | ||||||||||||||||||||||||||||||
|
@@ -136,6 +138,7 @@ where | |||||||||||||||||||||||||||||
&mut rng.clone(), | ||||||||||||||||||||||||||||||
boarding_inputs.clone(), | ||||||||||||||||||||||||||||||
vtxo_inputs.clone(), | ||||||||||||||||||||||||||||||
vec![], | ||||||||||||||||||||||||||||||
BatchOutputType::OffBoard { | ||||||||||||||||||||||||||||||
to_address: to_address.clone(), | ||||||||||||||||||||||||||||||
to_amount, | ||||||||||||||||||||||||||||||
|
@@ -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>, | ||||||||||||||||||||||||||||||
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")); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use fallible conversion from ArkNote to Input After making 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
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||
.chain(vtxo_inputs) | ||||||||||||||||||||||||||||||
.chain(arknotes) | ||||||||||||||||||||||||||||||
.collect::<Vec<_>>() | ||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
let mut outputs = vec![]; | ||||||||||||||||||||||||||||||
|
@@ -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?; | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
vincenzopalazzo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
step = step.next(); | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
@@ -753,7 +768,7 @@ where | |||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
enum BatchOutputType { | ||||||||||||||||||||||||||||||
pub(crate) enum BatchOutputType { | ||||||||||||||||||||||||||||||
Board { | ||||||||||||||||||||||||||||||
to_address: ArkAddress, | ||||||||||||||||||||||||||||||
to_amount: Amount, | ||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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; | ||||||||||||||||
|
@@ -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; | ||||||||||||||||
|
||||||||||||||||
|
@@ -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 | ||||||||||||||||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess 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> { | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AFAIK, this should not be part of the public API of the 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 |
||||||||||||||||
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(¬e_str).map_err(Error::ad_hoc)?; | ||||||||||||||||
Ok(note) | ||||||||||||||||
} | ||||||||||||||||
Comment on lines
+606
to
+609
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not log full ArkNote string (leaks the preimage/secret)
Apply this diff to log non-sensitive fields after parsing: - tracing::info!(note = %note_str, "Created ArkNote");
- let note = ArkNote::from_string(¬e_str).map_err(Error::ad_hoc)?;
+ let note = ArkNote::from_string(¬e_str).map_err(Error::ad_hoc)?;
+ tracing::info!(txid = %note.txid(), value = %note.value(), "Created ArkNote"); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||
|
||||||||||||||||
pub async fn transaction_history(&self) -> Result<Vec<history::Transaction>, Error> { | ||||||||||||||||
let mut boarding_transactions = Vec::new(); | ||||||||||||||||
let mut boarding_commitment_transactions = Vec::new(); | ||||||||||||||||
|
There was a problem hiding this comment.
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) withsettle
.