Support subaddresses as change outputs

This commit is contained in:
Luke Parker 2024-07-08 20:00:09 -04:00
parent 774424b70b
commit 5bb3256d1f
No known key found for this signature in database
8 changed files with 170 additions and 98 deletions

View file

@ -22,8 +22,8 @@ use crate::{
RctType, RctPrunable, RctProofs,
},
transaction::Transaction,
address::{Network, SubaddressIndex, MoneroAddress},
extra::MAX_ARBITRARY_DATA_SIZE,
address::{Network, MoneroAddress},
rpc::FeeRate,
ViewPair, GuaranteedViewPair, OutputWithDecoys,
};
@ -44,58 +44,48 @@ pub(crate) fn key_image_sort(x: &EdwardsPoint, y: &EdwardsPoint) -> core::cmp::O
#[derive(Clone, PartialEq, Eq, Zeroize)]
enum ChangeEnum {
None,
AddressOnly(MoneroAddress),
AddressWithView(MoneroAddress, Zeroizing<Scalar>),
Standard { view_pair: ViewPair, subaddress: Option<SubaddressIndex> },
Guaranteed { view_pair: GuaranteedViewPair, subaddress: Option<SubaddressIndex> },
}
impl fmt::Debug for ChangeEnum {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
ChangeEnum::None => f.debug_struct("ChangeEnum::None").finish_non_exhaustive(),
ChangeEnum::AddressOnly(addr) => {
f.debug_struct("ChangeEnum::AddressOnly").field("addr", &addr).finish()
}
ChangeEnum::AddressWithView(addr, _) => {
f.debug_struct("ChangeEnum::AddressWithView").field("addr", &addr).finish_non_exhaustive()
}
ChangeEnum::Standard { subaddress, .. } => f
.debug_struct("ChangeEnum::Standard")
.field("subaddress", &subaddress)
.finish_non_exhaustive(),
ChangeEnum::Guaranteed { subaddress, .. } => f
.debug_struct("ChangeEnum::Guaranteed")
.field("subaddress", &subaddress)
.finish_non_exhaustive(),
}
}
}
/// Specification for a change output.
#[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
pub struct Change(ChangeEnum);
pub struct Change(Option<ChangeEnum>);
impl Change {
/// Create a change output specification.
///
/// This take the view key as Monero assumes it has the view key for change outputs. It optimizes
/// its wallet protocol accordingly.
pub fn new(view: &ViewPair) -> Change {
Change(ChangeEnum::AddressWithView(
// Which network doesn't matter as the derivations will all be the same
// TODO: Support subaddresses
view.legacy_address(Network::Mainnet),
view.view.clone(),
))
pub fn new(view_pair: ViewPair, subaddress: Option<SubaddressIndex>) -> Change {
Change(Some(ChangeEnum::Standard { view_pair, subaddress }))
}
/// Create a change output specification for a guaranteed view pair.
///
/// This take the view key as Monero assumes it has the view key for change outputs. It optimizes
/// its wallet protocol accordingly.
pub fn guaranteed(view: &GuaranteedViewPair) -> Change {
Change(ChangeEnum::AddressWithView(
view.address(
// Which network doesn't matter as the derivations will all be the same
Network::Mainnet,
// TODO: Support subaddresses
None,
None,
),
view.0.view.clone(),
))
pub fn guaranteed(view_pair: GuaranteedViewPair, subaddress: Option<SubaddressIndex>) -> Change {
Change(Some(ChangeEnum::Guaranteed { view_pair, subaddress }))
}
/// Create a fingerprintable change output specification.
@ -116,38 +106,34 @@ impl Change {
/// monero-wallet TX without change.
pub fn fingerprintable(address: Option<MoneroAddress>) -> Change {
if let Some(address) = address {
Change(ChangeEnum::AddressOnly(address))
Change(Some(ChangeEnum::AddressOnly(address)))
} else {
Change(ChangeEnum::None)
Change(None)
}
}
}
#[derive(Clone, PartialEq, Eq, Zeroize)]
#[derive(Clone, PartialEq, Eq, Debug, Zeroize)]
enum InternalPayment {
Payment(MoneroAddress, u64),
Change(MoneroAddress, Option<Zeroizing<Scalar>>),
Change(ChangeEnum),
}
impl InternalPayment {
fn address(&self) -> &MoneroAddress {
fn address(&self) -> MoneroAddress {
match self {
InternalPayment::Payment(addr, _) | InternalPayment::Change(addr, _) => addr,
}
}
}
impl fmt::Debug for InternalPayment {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
InternalPayment::Payment(addr, amount) => f
.debug_struct("InternalPayment::Payment")
.field("addr", &addr)
.field("amount", &amount)
.finish(),
InternalPayment::Change(addr, _) => {
f.debug_struct("InternalPayment::Change").field("addr", &addr).finish_non_exhaustive()
InternalPayment::Payment(addr, _) => *addr,
InternalPayment::Change(change) => match change {
ChangeEnum::AddressOnly(addr) => *addr,
// Network::Mainnet as the network won't effect the derivations
ChangeEnum::Standard { view_pair, subaddress } => match subaddress {
Some(subaddress) => view_pair.subaddress(Network::Mainnet, *subaddress),
None => view_pair.legacy_address(Network::Mainnet),
},
ChangeEnum::Guaranteed { view_pair, subaddress } => {
view_pair.address(Network::Mainnet, *subaddress, None)
}
},
}
}
}
@ -276,7 +262,7 @@ impl SignableTransaction {
{
let mut change_count = 0;
for payment in &self.payments {
change_count += usize::from(u8::from(matches!(payment, InternalPayment::Change(_, _))));
change_count += usize::from(u8::from(matches!(payment, InternalPayment::Change(_))));
}
if change_count > 1 {
Err(SendError::MaliciousSerialization)?;
@ -319,7 +305,7 @@ impl SignableTransaction {
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Change(_, _) => None,
InternalPayment::Change(_) => None,
})
.sum::<u64>();
let (weight, necessary_fee) = self.weight_and_necessary_fee();
@ -366,12 +352,9 @@ impl SignableTransaction {
.into_iter()
.map(|(addr, amount)| InternalPayment::Payment(addr, amount))
.collect::<Vec<_>>();
match change.0 {
ChangeEnum::None => {}
ChangeEnum::AddressOnly(addr) => payments.push(InternalPayment::Change(addr, None)),
ChangeEnum::AddressWithView(addr, view) => {
payments.push(InternalPayment::Change(addr, Some(view)))
}
if let Some(change) = change.0 {
payments.push(InternalPayment::Change(change));
}
let mut res =
@ -412,16 +395,36 @@ impl SignableTransaction {
write_vec(write_byte, addr.to_string().as_bytes(), w)?;
w.write_all(&amount.to_le_bytes())
}
InternalPayment::Change(addr, change_view) => {
InternalPayment::Change(change) => match change {
ChangeEnum::AddressOnly(addr) => {
w.write_all(&[1])?;
write_vec(write_byte, addr.to_string().as_bytes(), w)?;
if let Some(view) = change_view.as_ref() {
w.write_all(&[1])?;
write_scalar(view, w)
write_vec(write_byte, addr.to_string().as_bytes(), w)
}
ChangeEnum::Standard { view_pair, subaddress } => {
w.write_all(&[2])?;
write_point(&view_pair.spend(), w)?;
write_scalar(&view_pair.view, w)?;
if let Some(subaddress) = subaddress {
w.write_all(&subaddress.account().to_le_bytes())?;
w.write_all(&subaddress.address().to_le_bytes())
} else {
w.write_all(&[0])
w.write_all(&0u32.to_le_bytes())?;
w.write_all(&0u32.to_le_bytes())
}
}
ChangeEnum::Guaranteed { view_pair, subaddress } => {
w.write_all(&[3])?;
write_point(&view_pair.spend(), w)?;
write_scalar(&view_pair.0.view, w)?;
if let Some(subaddress) = subaddress {
w.write_all(&subaddress.account().to_le_bytes())?;
w.write_all(&subaddress.address().to_le_bytes())
} else {
w.write_all(&0u32.to_le_bytes())?;
w.write_all(&0u32.to_le_bytes())
}
}
},
}
}
@ -458,14 +461,17 @@ impl SignableTransaction {
fn read_payment<R: io::Read>(r: &mut R) -> io::Result<InternalPayment> {
Ok(match read_byte(r)? {
0 => InternalPayment::Payment(read_address(r)?, read_u64(r)?),
1 => InternalPayment::Change(
read_address(r)?,
match read_byte(r)? {
0 => None,
1 => Some(Zeroizing::new(read_scalar(r)?)),
_ => Err(io::Error::other("invalid change view"))?,
},
),
1 => InternalPayment::Change(ChangeEnum::AddressOnly(read_address(r)?)),
2 => InternalPayment::Change(ChangeEnum::Standard {
view_pair: ViewPair::new(read_point(r)?, Zeroizing::new(read_scalar(r)?))
.map_err(io::Error::other)?,
subaddress: SubaddressIndex::new(read_u32(r)?, read_u32(r)?),
}),
3 => InternalPayment::Change(ChangeEnum::Guaranteed {
view_pair: GuaranteedViewPair::new(read_point(r)?, Zeroizing::new(read_scalar(r)?))
.map_err(io::Error::other)?,
subaddress: SubaddressIndex::new(read_u32(r)?, read_u32(r)?),
}),
_ => Err(io::Error::other("invalid payment"))?,
})
}

View file

@ -78,7 +78,7 @@ impl SignableTransaction {
} else {
// If there's no payment ID, we push a dummy (as wallet2 does) if there's only one payment
if (self.payments.len() == 2) &&
self.payments.iter().any(|payment| matches!(payment, InternalPayment::Change(_, _)))
self.payments.iter().any(|payment| matches!(payment, InternalPayment::Change(_)))
{
let (_, payment_id_xor) = self
.payments
@ -292,7 +292,7 @@ impl SignableTransactionWithKeyImages {
.intent
.payments
.iter()
.any(|payment| matches!(payment, InternalPayment::Change(_, _)))
.any(|payment| matches!(payment, InternalPayment::Change(_)))
{
// The necessary fee is the fee
self.intent.weight_and_necessary_fee().1
@ -306,7 +306,7 @@ impl SignableTransactionWithKeyImages {
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Change(_, _) => None,
InternalPayment::Change(_) => None,
})
.sum::<u64>();
// Safe since the constructor checks inputs >= (payments + fee)

View file

@ -12,7 +12,7 @@ use crate::{
primitives::{keccak256, Commitment},
ringct::EncryptedAmount,
SharedKeyDerivations, OutputWithDecoys,
send::{InternalPayment, SignableTransaction, key_image_sort},
send::{ChangeEnum, InternalPayment, SignableTransaction, key_image_sort},
};
impl SignableTransaction {
@ -42,15 +42,13 @@ impl SignableTransaction {
fn has_payments_to_subaddresses(&self) -> bool {
self.payments.iter().any(|payment| match payment {
InternalPayment::Payment(addr, _) => addr.is_subaddress(),
InternalPayment::Change(addr, view) => {
if view.is_some() {
// It should not be possible to construct a change specification to a subaddress with a
// view key
// TODO
debug_assert!(!addr.is_subaddress());
}
addr.is_subaddress()
}
InternalPayment::Change(change) => match change {
ChangeEnum::AddressOnly(addr) => addr.is_subaddress(),
// These aren't considered payments to subaddresses as we don't need to send to them as
// subaddresses
// We can calculate the shared key using the view key, as if we were receiving, instead
ChangeEnum::Standard { .. } | ChangeEnum::Guaranteed { .. } => false,
},
})
}
@ -62,7 +60,10 @@ impl SignableTransaction {
let has_change_view = self.payments.iter().any(|payment| match payment {
InternalPayment::Payment(_, _) => false,
InternalPayment::Change(_, view) => view.is_some(),
InternalPayment::Change(change) => match change {
ChangeEnum::AddressOnly(_) => false,
ChangeEnum::Standard { .. } | ChangeEnum::Guaranteed { .. } => true,
},
});
/*
@ -107,11 +108,17 @@ impl SignableTransaction {
let ecdh = match payment {
// If we don't have the view key, use the key dedicated for this address (r A)
InternalPayment::Payment(_, _) | InternalPayment::Change(_, None) => {
InternalPayment::Payment(_, _) |
InternalPayment::Change(ChangeEnum::AddressOnly { .. }) => {
Zeroizing::new(key_to_use.deref() * addr.view())
}
// If we do have the view key, use the commitment to the key (a R)
InternalPayment::Change(_, Some(view)) => Zeroizing::new(view.deref() * tx_key_pub),
InternalPayment::Change(ChangeEnum::Standard { view_pair, .. }) => {
Zeroizing::new(view_pair.view.deref() * tx_key_pub)
}
InternalPayment::Change(ChangeEnum::Guaranteed { view_pair, .. }) => {
Zeroizing::new(view_pair.0.view.deref() * tx_key_pub)
}
};
res.push(ecdh);
@ -172,9 +179,6 @@ impl SignableTransaction {
panic!("filtered payment wasn't a payment")
};
// TODO: Support subaddresses as change?
debug_assert!(addr.is_subaddress());
return (tx_key.deref() * addr.spend(), vec![]);
}
@ -207,14 +211,14 @@ impl SignableTransaction {
for (payment, shared_key_derivations) in self.payments.iter().zip(shared_key_derivations) {
let amount = match payment {
InternalPayment::Payment(_, amount) => *amount,
InternalPayment::Change(_, _) => {
InternalPayment::Change(_) => {
let inputs = self.inputs.iter().map(|input| input.commitment().amount).sum::<u64>();
let payments = self
.payments
.iter()
.filter_map(|payment| match payment {
InternalPayment::Payment(_, amount) => Some(amount),
InternalPayment::Change(_, _) => None,
InternalPayment::Change(_) => None,
})
.sum::<u64>();
let necessary_fee = self.weight_and_necessary_fee().1;

View file

@ -27,7 +27,7 @@ pub enum ViewPairError {
/// The pair of keys necessary to scan transactions.
///
/// This is composed of the public spend key and the private view key.
#[derive(Clone, Zeroize, ZeroizeOnDrop)]
#[derive(Clone, PartialEq, Eq, Zeroize, ZeroizeOnDrop)]
pub struct ViewPair {
spend: EdwardsPoint,
pub(crate) view: Zeroizing<Scalar>,
@ -99,7 +99,7 @@ impl ViewPair {
/// 'Guaranteed' outputs, or transactions outputs to the burning bug, are not officially specified
/// by the Monero project. They should only be used if necessary. No support outside of
/// monero-wallet is promised.
#[derive(Clone, Zeroize)]
#[derive(Clone, PartialEq, Eq, Zeroize)]
pub struct GuaranteedViewPair(pub(crate) ViewPair);
impl GuaranteedViewPair {

View file

@ -254,10 +254,11 @@ macro_rules! test {
rct_type,
outgoing_view,
Change::new(
&ViewPair::new(
ViewPair::new(
&Scalar::random(&mut OsRng) * ED25519_BASEPOINT_TABLE,
Zeroizing::new(Scalar::random(&mut OsRng))
).unwrap(),
None,
),
rpc.get_fee_rate(FeePriority::Unimportant).await.unwrap(),
);
@ -267,6 +268,8 @@ macro_rules! test {
#[cfg(feature = "multisig")]
let keys = keys.clone();
assert_eq!(&SignableTransaction::read(&mut tx.serialize().as_slice()).unwrap(), &tx);
let eventuality = Eventuality::from(tx.clone());
let tx = if !multisig {

View file

@ -115,7 +115,7 @@ test!(
let mut builder = SignableTransactionBuilder::new(
rct_type,
outgoing_view,
Change::new(&change_view),
Change::new(change_view.clone(), None),
rpc.get_fee_rate(FeePriority::Unimportant).await.unwrap(),
);
add_inputs(rct_type, &rpc, vec![outputs.first().unwrap().clone()], &mut builder).await;
@ -144,6 +144,8 @@ test!(
assert!(sub_outputs.len() == 1);
assert_eq!(sub_outputs[0].transaction(), tx.hash());
assert_eq!(sub_outputs[0].commitment().amount, 1);
assert!(sub_outputs[0].subaddress().unwrap().account() == 0);
assert!(sub_outputs[0].subaddress().unwrap().address() == 1);
// Make sure only one R was included in TX extra
assert!(Extra::read::<&[u8]>(&mut tx.prefix().extra.as_ref())
@ -333,3 +335,60 @@ test!(
},
),
);
test!(
subaddress_change,
(
// Consume this builder for an output we can use in the future
// This is needed because we can't get the input from the passed in builder
|_, mut builder: Builder, addr| async move {
builder.add_payment(addr, 1000000000000);
(builder.build().unwrap(), ())
},
|rpc, block, tx: Transaction, mut scanner: Scanner, ()| async move {
let outputs = scanner.scan(&rpc, &block).await.unwrap().not_additionally_locked();
assert_eq!(outputs.len(), 1);
assert_eq!(outputs[0].transaction(), tx.hash());
assert_eq!(outputs[0].commitment().amount, 1000000000000);
outputs
},
),
(
|rct_type, rpc: SimpleRequestRpc, _, _, outputs: Vec<WalletOutput>| async move {
use monero_wallet::rpc::FeePriority;
let view_priv = Zeroizing::new(Scalar::random(&mut OsRng));
let mut outgoing_view = Zeroizing::new([0; 32]);
OsRng.fill_bytes(outgoing_view.as_mut());
let change_view =
ViewPair::new(&Scalar::random(&mut OsRng) * ED25519_BASEPOINT_TABLE, view_priv.clone())
.unwrap();
let mut builder = SignableTransactionBuilder::new(
rct_type,
outgoing_view,
Change::new(change_view.clone(), Some(SubaddressIndex::new(0, 1).unwrap())),
rpc.get_fee_rate(FeePriority::Unimportant).await.unwrap(),
);
add_inputs(rct_type, &rpc, vec![outputs.first().unwrap().clone()], &mut builder).await;
// Send to a random address
let view = ViewPair::new(
&Scalar::random(&mut OsRng) * ED25519_BASEPOINT_TABLE,
Zeroizing::new(Scalar::random(&mut OsRng)),
)
.unwrap();
builder.add_payment(view.legacy_address(Network::Mainnet), 1);
(builder.build().unwrap(), change_view)
},
|rpc, block, _, _, change_view: ViewPair| async move {
// Make sure the change can pick up its output
let mut change_scanner = Scanner::new(change_view);
change_scanner.register_subaddress(SubaddressIndex::new(0, 1).unwrap());
let outputs = change_scanner.scan(&rpc, &block).await.unwrap().not_additionally_locked();
assert!(outputs.len() == 1);
assert!(outputs[0].subaddress().unwrap().account() == 0);
assert!(outputs[0].subaddress().unwrap().address() == 1);
},
),
);

View file

@ -389,7 +389,7 @@ async fn mint_and_burn_test() {
),
1_100_000_000_000,
)],
Change::new(&view_pair),
Change::new(view_pair.clone(), None),
vec![Shorthand::transfer(None, serai_addr).encode()],
rpc.get_fee_rate(FeePriority::Unimportant).await.unwrap(),
)

View file

@ -474,7 +474,7 @@ impl Wallet {
outgoing_view_key,
inputs,
vec![(to_addr, AMOUNT)],
Change::new(view_pair),
Change::new(view_pair.clone(), None),
data,
rpc.get_fee_rate(FeePriority::Unimportant).await.unwrap(),
)