-
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 1 commit
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(); | ||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,3 +1,6 @@ | ||||||
use crate::arknote::PREIMAGE_LENGTH; | ||||||
use crate::proof_of_funds::taproot::LeafVersion; | ||||||
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. Broken import path for LeafVersion (won’t compile)
Apply this diff: -use crate::proof_of_funds::taproot::LeafVersion;
+use bitcoin::taproot::LeafVersion; 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||
use crate::ArkNote; | ||||||
use crate::Error; | ||||||
use crate::ErrorContext; | ||||||
use bitcoin::absolute::LockTime; | ||||||
|
@@ -46,6 +49,8 @@ pub struct Input { | |||||
pk: XOnlyPublicKey, | ||||||
spend_info: (ScriptBuf, taproot::ControlBlock), | ||||||
is_onchain: bool, | ||||||
// FIXME: make the input type an enum based on the input type. | ||||||
preimage: Option<[u8; PREIMAGE_LENGTH]>, | ||||||
vincenzopalazzo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
impl Input { | ||||||
|
@@ -66,6 +71,39 @@ impl Input { | |||||
pk, | ||||||
spend_info, | ||||||
is_onchain, | ||||||
preimage: None, | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
impl From<&ArkNote> for Input { | ||||||
fn from(value: &ArkNote) -> Self { | ||||||
let spending_info = value.vtxo_script().spend_info(); | ||||||
|
||||||
// this is inside the taproot script path | ||||||
let node_script = value.note_script.clone(); | ||||||
let Some(control_block) = | ||||||
spending_info.control_block(&(node_script.clone(), LeafVersion::TapScript)) | ||||||
else { | ||||||
// FIXME: probably we need a tryfrom? | ||||||
panic!("no control block found"); | ||||||
}; | ||||||
|
||||||
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 think we need to model things differently so that the conversion from Ark note to proof of funds input is infallible. An Ark note always has the same structure AFAIK, so it should just work. 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. Ok to me this just sounds calling |
||||||
Self { | ||||||
vincenzopalazzo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
outpoint: value.outpoint(), | ||||||
sequence: Sequence::MAX, | ||||||
witness_utxo: TxOut { | ||||||
value: value.value(), | ||||||
// This should be unspendable script? | ||||||
script_pubkey: value.vtxo_script().script_pubkey(), | ||||||
}, | ||||||
// This should be empty? | ||||||
tapscripts: vec![], | ||||||
pk: value.vtxo_script().x_only_public_key(), | ||||||
// This contains the extra info to spend the note, right? | ||||||
vincenzopalazzo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
spend_info: (node_script, control_block), | ||||||
is_onchain: false, | ||||||
preimage: Some(*value.preimage()), | ||||||
} | ||||||
} | ||||||
} | ||||||
|
@@ -93,8 +131,8 @@ impl Bip322Proof { | |||||
} | ||||||
|
||||||
pub fn make_bip322_signature<F>( | ||||||
signing_kps: &[Keypair], | ||||||
sign_for_onchain_pk_fn: F, | ||||||
signing_kps: &[Keypair], // This is to sign VTXOs in the `inputs` argument. | ||||||
sign_for_onchain_pk_fn: F, // This is to sign boarding outputs in the `inputs` argument. | ||||||
vincenzopalazzo marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
inputs: Vec<Input>, | ||||||
outputs: Vec<Output>, | ||||||
own_cosigner_pks: Vec<PublicKey>, | ||||||
|
@@ -153,7 +191,7 @@ where | |||||
|
||||||
proof_input.tap_scripts = BTreeMap::from_iter([( | ||||||
leaf_proof.1.clone(), | ||||||
(leaf_proof.0.clone(), taproot::LeafVersion::TapScript), | ||||||
(leaf_proof.0.clone(), LeafVersion::TapScript), | ||||||
)]); | ||||||
} | ||||||
|
||||||
|
@@ -189,7 +227,7 @@ where | |||||
|
||||||
let pk = input.pk; | ||||||
|
||||||
let sig = match input.is_onchain { | ||||||
let sig: Vec<u8> = match input.is_onchain { | ||||||
true => { | ||||||
let sig = sign_for_onchain_pk_fn(&pk, &msg)?; | ||||||
|
||||||
|
@@ -204,40 +242,52 @@ where | |||||
|
||||||
proof_input.tap_script_sigs = BTreeMap::from_iter([((pk, leaf_hash), sig)]); | ||||||
|
||||||
sig | ||||||
sig.signature.serialize().to_vec() | ||||||
} | ||||||
false => { | ||||||
let signing_kp = signing_kps | ||||||
.iter() | ||||||
.find(|kp| { | ||||||
let (xonly_ok, _) = kp.x_only_public_key(); | ||||||
xonly_ok == pk | ||||||
}) | ||||||
.ok_or_else(|| Error::ad_hoc("Could not find suitable kp for pk"))?; | ||||||
|
||||||
let sig = secp.sign_schnorr_no_aux_rand(&msg, signing_kp); | ||||||
|
||||||
secp.verify_schnorr(&sig, &msg, &pk) | ||||||
.map_err(Error::crypto) | ||||||
.context("failed to verify own proof of funds vtxo signature")?; | ||||||
|
||||||
let sig = taproot::Signature { | ||||||
signature: sig, | ||||||
sighash_type: TapSighashType::Default, | ||||||
}; | ||||||
|
||||||
proof_input.tap_script_sigs = BTreeMap::from_iter([((pk, leaf_hash), sig)]); | ||||||
|
||||||
sig | ||||||
} | ||||||
// FIXME: this is an horrible hack to handle arknotes. | ||||||
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. The FIXME comment indicates this is a temporary hack. Consider implementing a proper enum-based solution for different input types as suggested in the comment on line 52. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
if let Some(preimage) = input.preimage { | ||||||
preimage.to_vec() | ||||||
} else { | ||||||
let signing_kp = signing_kps | ||||||
.iter() | ||||||
.find(|kp| { | ||||||
let (xonly_ok, _) = kp.x_only_public_key(); | ||||||
xonly_ok == pk | ||||||
}) | ||||||
.ok_or_else(|| Error::ad_hoc("Could not find suitable kp for pk"))?; | ||||||
|
||||||
let sig = secp.sign_schnorr_no_aux_rand(&msg, signing_kp); | ||||||
|
||||||
secp.verify_schnorr(&sig, &msg, &pk) | ||||||
.map_err(Error::crypto) | ||||||
.context("failed to verify own proof of funds vtxo signature")?; | ||||||
|
||||||
let sig = taproot::Signature { | ||||||
signature: sig, | ||||||
sighash_type: TapSighashType::Default, | ||||||
}; | ||||||
|
||||||
proof_input.tap_script_sigs = BTreeMap::from_iter([((pk, leaf_hash), sig)]); | ||||||
|
||||||
sig.signature.serialize().to_vec() | ||||||
} | ||||||
} // We need to branch here once more to handle "signing" (satisfying) the Note script. | ||||||
}; | ||||||
|
||||||
// This is different for the Note script! | ||||||
let witness = Witness::from_slice(&[ | ||||||
&sig.signature[..], | ||||||
sig.as_slice(), | ||||||
exit_script.as_bytes(), | ||||||
&exit_control_block.serialize(), | ||||||
]); | ||||||
|
||||||
// let witness = Witness::from_slice(&[ | ||||||
// preimage, | ||||||
// note_script.as_bytes(), | ||||||
// &control_block.serialize(), | ||||||
// ]); | ||||||
|
||||||
proof_input.final_script_witness = Some(witness); | ||||||
} | ||||||
|
||||||
|
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
.