From 9c217913e6964d4220afb0219a68411ece8ff443 Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Fri, 14 Jun 2024 11:47:57 -0400 Subject: [PATCH] Further documentation, start shoring up API boundaries of existing crates --- Cargo.lock | 2 - coins/monero/generators/src/hash_to_point.rs | 2 +- coins/monero/generators/src/lib.rs | 30 ++++++--- .../generators/src/tests/hash_to_point.rs | 38 ------------ coins/monero/generators/src/tests/mod.rs | 37 ++++++++++- coins/monero/io/src/lib.rs | 55 ++++++++++++---- coins/monero/primitives/Cargo.toml | 2 - coins/monero/primitives/src/lib.rs | 62 ++++++++++++++----- coins/monero/ringct/clsag/Cargo.toml | 2 - coins/monero/ringct/clsag/src/lib.rs | 24 +++---- coins/monero/ringct/clsag/src/multisig.rs | 8 +-- coins/monero/src/tests/clsag.rs | 9 +-- coins/monero/src/wallet/decoys.rs | 15 +++-- coins/monero/src/wallet/send/mod.rs | 13 +++- coins/monero/src/wallet/send/multisig.rs | 8 +-- coins/monero/tests/decoys.rs | 4 +- 16 files changed, 195 insertions(+), 116 deletions(-) delete mode 100644 coins/monero/generators/src/tests/hash_to_point.rs diff --git a/Cargo.lock b/Cargo.lock index 401d33a0..6cda32f6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4765,7 +4765,6 @@ dependencies = [ "monero-primitives", "rand_chacha", "rand_core", - "sha3", "std-shims", "subtle", "thiserror", @@ -4801,7 +4800,6 @@ dependencies = [ "curve25519-dalek", "monero-generators", "monero-io", - "rand_core", "sha3", "std-shims", "zeroize", diff --git a/coins/monero/generators/src/hash_to_point.rs b/coins/monero/generators/src/hash_to_point.rs index 36c0b3a3..23b3a086 100644 --- a/coins/monero/generators/src/hash_to_point.rs +++ b/coins/monero/generators/src/hash_to_point.rs @@ -9,7 +9,7 @@ use monero_io::decompress_point; use crate::keccak256; -/// Monero's hash to point function, as named `hash_to_ec`. +/// Monero's `hash_to_ec` function. pub fn hash_to_point(bytes: [u8; 32]) -> EdwardsPoint { #[allow(non_snake_case)] let A = FieldElement::from(486662u64); diff --git a/coins/monero/generators/src/lib.rs b/coins/monero/generators/src/lib.rs index 60f81bfa..cd9ee237 100644 --- a/coins/monero/generators/src/lib.rs +++ b/coins/monero/generators/src/lib.rs @@ -24,7 +24,10 @@ fn keccak256(data: &[u8]) -> [u8; 32] { } static H_CELL: OnceLock = OnceLock::new(); -/// Monero's alternate generator `H`, used for amounts in Pedersen commitments. +/// Monero's `H` generator. +/// +/// Contrary to convention (`G` for values, `H` for randomness), `H` is used by Monero for amounts +/// within Pedersen commitments. #[allow(non_snake_case)] pub fn H() -> DalekPoint { *H_CELL.get_or_init(|| { @@ -33,7 +36,9 @@ pub fn H() -> DalekPoint { } static H_POW_2_CELL: OnceLock<[DalekPoint; 64]> = OnceLock::new(); -/// Monero's alternate generator `H`, multiplied by 2**i for i in 1 ..= 64. +/// Monero's `H` generator, multiplied by 2**i for i in 1 ..= 64. +/// +/// This table is useful when working with amounts, which are u64s. #[allow(non_snake_case)] pub fn H_pow_2() -> &'static [DalekPoint; 64] { H_POW_2_CELL.get_or_init(|| { @@ -45,8 +50,11 @@ pub fn H_pow_2() -> &'static [DalekPoint; 64] { }) } +// The maximum amount of commitments proven for within a single range proof. const MAX_M: usize = 16; +// The amount of bits the value within a commitment may use. const N: usize = 64; +// The maximum amount of bits used within a single range proof. const MAX_MN: usize = MAX_M * N; /// Container struct for Bulletproofs(+) generators. @@ -57,18 +65,24 @@ pub struct Generators { } /// Generate generators as needed for Bulletproofs(+), as Monero does. +/// +/// Consumers should not call this function ad-hoc, yet call it within a build script or use a +/// once-initialized static. pub fn bulletproofs_generators(dst: &'static [u8]) -> Generators { + let mut preimage = H().compress().to_bytes().to_vec(); + preimage.extend(dst); + let mut res = Generators { G: Vec::with_capacity(MAX_MN), H: Vec::with_capacity(MAX_MN) }; for i in 0 .. MAX_MN { + // We generate a pair of generators per iteration let i = 2 * i; - let mut even = H().compress().to_bytes().to_vec(); - even.extend(dst); - let mut odd = even.clone(); - - write_varint::, u64>(&i.try_into().unwrap(), &mut even).unwrap(); - write_varint::, u64>(&(i + 1).try_into().unwrap(), &mut odd).unwrap(); + let mut even = preimage.clone(); + write_varint(&i, &mut even).unwrap(); res.H.push(EdwardsPoint(hash_to_point(keccak256(&even)))); + + let mut odd = preimage.clone(); + write_varint(&(i + 1), &mut odd).unwrap(); res.G.push(EdwardsPoint(hash_to_point(keccak256(&odd)))); } res diff --git a/coins/monero/generators/src/tests/hash_to_point.rs b/coins/monero/generators/src/tests/hash_to_point.rs deleted file mode 100644 index c4535e08..00000000 --- a/coins/monero/generators/src/tests/hash_to_point.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::{decompress_point, hash_to_point}; - -#[test] -fn crypto_tests() { - // tests.txt file copied from monero repo - // https://github.com/monero-project/monero/ - // blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/tests/crypto/tests.txt - let reader = include_str!("./tests.txt"); - - for line in reader.lines() { - let mut words = line.split_whitespace(); - let command = words.next().unwrap(); - - match command { - "check_key" => { - let key = words.next().unwrap(); - let expected = match words.next().unwrap() { - "true" => true, - "false" => false, - _ => unreachable!("invalid result"), - }; - - let actual = decompress_point(hex::decode(key).unwrap().try_into().unwrap()); - - assert_eq!(actual.is_some(), expected); - } - "hash_to_ec" => { - let bytes = words.next().unwrap(); - let expected = words.next().unwrap(); - - let actual = hash_to_point(hex::decode(bytes).unwrap().try_into().unwrap()); - - assert_eq!(hex::encode(actual.compress().to_bytes()), expected); - } - _ => unreachable!("unknown command"), - } - } -} diff --git a/coins/monero/generators/src/tests/mod.rs b/coins/monero/generators/src/tests/mod.rs index ec208e9c..3ab9449f 100644 --- a/coins/monero/generators/src/tests/mod.rs +++ b/coins/monero/generators/src/tests/mod.rs @@ -1 +1,36 @@ -mod hash_to_point; +use crate::{decompress_point, hash_to_point}; + +#[test] +fn test_vectors() { + // tests.txt file copied from monero repo + // https://github.com/monero-project/monero/ + // blob/ac02af92867590ca80b2779a7bbeafa99ff94dcb/tests/crypto/tests.txt + let reader = include_str!("./tests.txt"); + + for line in reader.lines() { + let mut words = line.split_whitespace(); + let command = words.next().unwrap(); + + match command { + "check_key" => { + let key = words.next().unwrap(); + let expected = match words.next().unwrap() { + "true" => true, + "false" => false, + _ => unreachable!("invalid result"), + }; + + let actual = decompress_point(hex::decode(key).unwrap().try_into().unwrap()); + assert_eq!(actual.is_some(), expected); + } + "hash_to_ec" => { + let bytes = words.next().unwrap(); + let expected = words.next().unwrap(); + + let actual = hash_to_point(hex::decode(bytes).unwrap().try_into().unwrap()); + assert_eq!(hex::encode(actual.compress().to_bytes()), expected); + } + _ => unreachable!("unknown command"), + } + } +} diff --git a/coins/monero/io/src/lib.rs b/coins/monero/io/src/lib.rs index 0a8880b7..7c7001b7 100644 --- a/coins/monero/io/src/lib.rs +++ b/coins/monero/io/src/lib.rs @@ -17,9 +17,13 @@ use curve25519_dalek::{ const VARINT_CONTINUATION_MASK: u8 = 0b1000_0000; mod sealed { + /// A trait for a number readable/writable as a VarInt. + /// + /// This is sealed to prevent unintended implementations. pub trait VarInt: TryInto + TryFrom + Copy { const BITS: usize; } + impl VarInt for u8 { const BITS: usize = 8; } @@ -34,17 +38,24 @@ mod sealed { } } -// This will panic if the VarInt exceeds u64::MAX -pub fn varint_len(varint: U) -> usize { +/// The amount of bytes this number will take when serialized as a VarInt. +/// +/// This function will panic if the VarInt exceeds u64::MAX. +pub fn varint_len(varint: V) -> usize { let varint_u64: u64 = varint.try_into().map_err(|_| "varint exceeded u64").unwrap(); ((usize::try_from(u64::BITS - varint_u64.leading_zeros()).unwrap().saturating_sub(1)) / 7) + 1 } +/// Write a byte. +/// +/// This is used as a building block within generic functions. pub fn write_byte(byte: &u8, w: &mut W) -> io::Result<()> { w.write_all(&[*byte]) } -// This will panic if the VarInt exceeds u64::MAX +/// Write a number, VarInt-encoded,. +/// +/// This will panic if the VarInt exceeds u64::MAX. pub fn write_varint(varint: &U, w: &mut W) -> io::Result<()> { let mut varint: u64 = (*varint).try_into().map_err(|_| "varint exceeded u64").unwrap(); while { @@ -59,14 +70,17 @@ pub fn write_varint(varint: &U, w: &mut W) -> io::R Ok(()) } +/// Write a scalar. pub fn write_scalar(scalar: &Scalar, w: &mut W) -> io::Result<()> { w.write_all(&scalar.to_bytes()) } +/// Write a point. pub fn write_point(point: &EdwardsPoint, w: &mut W) -> io::Result<()> { w.write_all(&point.compress().to_bytes()) } +/// Write a list of elements, without length-prefixing,. pub fn write_raw_vec io::Result<()>>( f: F, values: &[T], @@ -78,6 +92,7 @@ pub fn write_raw_vec io::Result<()>>( Ok(()) } +/// Write a list of elements, with length-prefixing,. pub fn write_vec io::Result<()>>( f: F, values: &[T], @@ -87,28 +102,34 @@ pub fn write_vec io::Result<()>>( write_raw_vec(f, values, w) } +/// Read a constant amount of bytes. pub fn read_bytes(r: &mut R) -> io::Result<[u8; N]> { let mut res = [0; N]; r.read_exact(&mut res)?; Ok(res) } +/// Read a single byte. pub fn read_byte(r: &mut R) -> io::Result { Ok(read_bytes::<_, 1>(r)?[0]) } +/// Read a u16, little-endian encoded,. pub fn read_u16(r: &mut R) -> io::Result { read_bytes(r).map(u16::from_le_bytes) } +/// Read a u32, little-endian encoded,. pub fn read_u32(r: &mut R) -> io::Result { read_bytes(r).map(u32::from_le_bytes) } +/// Read a u64, little-endian encoded,. pub fn read_u64(r: &mut R) -> io::Result { read_bytes(r).map(u64::from_le_bytes) } +/// Read a canonically-encoded VarInt. pub fn read_varint(r: &mut R) -> io::Result { let mut bits = 0; let mut res = 0; @@ -128,20 +149,24 @@ pub fn read_varint(r: &mut R) -> io::Result { res.try_into().map_err(|_| io::Error::other("VarInt does not fit into integer type")) } -// All scalar fields supported by monero-serai are checked to be canonical for valid transactions -// While from_bytes_mod_order would be more flexible, it's not currently needed and would be -// inaccurate to include now. While casting a wide net may be preferable, it'd also be inaccurate -// for now. There's also further edge cases as noted by -// https://github.com/monero-project/monero/issues/8438, where some scalars had an archaic -// reduction applied +/// Read a canonically-encoded scalar. +/// +/// Some scalars within the Monero protocol are not enforced to be canonically encoded. For such +/// scalars, they should be represented as `[u8; 32]` and later converted to scalars as relevant. pub fn read_scalar(r: &mut R) -> io::Result { Option::from(Scalar::from_canonical_bytes(read_bytes(r)?)) .ok_or_else(|| io::Error::other("unreduced scalar")) } -/// Decompress a canonically encoded ed25519 point. +/// Decompress a canonically-encoded Ed25519 point. /// -/// This function does not check if the point is within the prime order subgroup. +/// Ed25519 is of order `8 * l`. This function ensures each of those `8 * l` points have a singular +/// encoding by checking points aren't encoded with an unreduced field element, and aren't negative +/// when the negative is equivalent (0 == -0). +/// +/// Since this decodes an Ed25519 point, it does not check the point is in the prime-order +/// subgroup. Torsioned points do have a canonical encoding, and only aren't canonical when +/// considered in relation to the prime-order subgroup. pub fn decompress_point(bytes: [u8; 32]) -> Option { CompressedEdwardsY(bytes) .decompress() @@ -149,11 +174,16 @@ pub fn decompress_point(bytes: [u8; 32]) -> Option { .filter(|point| point.compress().to_bytes() == bytes) } +/// Read a canonically-encoded Ed25519 point. +/// +/// This internally calls `decompress_point` and has the same definition of canonicity. This +/// function does not check the resulting point is within the prime-order subgroup. pub fn read_point(r: &mut R) -> io::Result { let bytes = read_bytes(r)?; decompress_point(bytes).ok_or_else(|| io::Error::other("invalid point")) } +/// Read a canonically-encoded Ed25519 point, within the prime-order subgroup. pub fn read_torsion_free_point(r: &mut R) -> io::Result { read_point(r) .ok() @@ -161,6 +191,7 @@ pub fn read_torsion_free_point(r: &mut R) -> io::Result { .ok_or_else(|| io::Error::other("invalid point")) } +/// Read a variable-length list of elements, without length-prefixing. pub fn read_raw_vec io::Result>( f: F, len: usize, @@ -173,6 +204,7 @@ pub fn read_raw_vec io::Result>( Ok(res) } +/// Read a constant-length list of elements. pub fn read_array io::Result, const N: usize>( f: F, r: &mut R, @@ -180,6 +212,7 @@ pub fn read_array io::Result, const N: us read_raw_vec(f, N, r).map(|vec| vec.try_into().unwrap()) } +/// Read a length-prefixed variable-length list of elements. pub fn read_vec io::Result>(f: F, r: &mut R) -> io::Result> { read_raw_vec(f, read_varint(r)?, r) } diff --git a/coins/monero/primitives/Cargo.toml b/coins/monero/primitives/Cargo.toml index a6aebb60..05646f40 100644 --- a/coins/monero/primitives/Cargo.toml +++ b/coins/monero/primitives/Cargo.toml @@ -18,7 +18,6 @@ workspace = true [dependencies] std-shims = { path = "../../../common/std-shims", version = "^0.1.1", default-features = false } -rand_core = { version = "0.6", default-features = false } zeroize = { version = "^1.5", default-features = false, features = ["zeroize_derive"] } # Cryptographic dependencies @@ -33,7 +32,6 @@ monero-generators = { path = "../generators", version = "0.4", default-features std = [ "std-shims/std", - "rand_core/std", "zeroize/std", "sha3/std", diff --git a/coins/monero/primitives/src/lib.rs b/coins/monero/primitives/src/lib.rs index 5da1d858..75f48410 100644 --- a/coins/monero/primitives/src/lib.rs +++ b/coins/monero/primitives/src/lib.rs @@ -2,7 +2,7 @@ #![doc = include_str!("../README.md")] #![cfg_attr(not(feature = "std"), no_std)] -use std_shims::{vec, vec::Vec}; +use std_shims::vec::Vec; #[cfg(feature = "std")] use std_shims::sync::OnceLock; @@ -16,25 +16,25 @@ use curve25519_dalek::{ edwards::{EdwardsPoint, VartimeEdwardsPrecomputation}, }; -use monero_io::varint_len; use monero_generators::H; -// TODO: Replace this with a const +// On std, we cache some variables in statics. #[cfg(feature = "std")] static INV_EIGHT_CELL: OnceLock = OnceLock::new(); #[cfg(feature = "std")] #[allow(non_snake_case)] +/// The inverse of 8 over l. pub fn INV_EIGHT() -> Scalar { *INV_EIGHT_CELL.get_or_init(|| Scalar::from(8u8).invert()) } +// In no-std environments, we prefer the reduced memory use and calculate it ad-hoc. #[cfg(not(feature = "std"))] #[allow(non_snake_case)] +/// The inverse of 8 over l. pub fn INV_EIGHT() -> Scalar { Scalar::from(8u8).invert() } -// On std, we cache this in a static -// In no-std environments, we prefer the reduced memory use and calculate it ad-hoc #[cfg(feature = "std")] static BASEPOINT_PRECOMP_CELL: OnceLock = OnceLock::new(); #[cfg(feature = "std")] @@ -49,11 +49,14 @@ pub fn BASEPOINT_PRECOMP() -> VartimeEdwardsPrecomputation { VartimeEdwardsPrecomputation::new([ED25519_BASEPOINT_POINT]) } +/// The Keccak-256 hash function. pub fn keccak256(data: impl AsRef<[u8]>) -> [u8; 32] { Keccak256::digest(data.as_ref()).into() } /// Hash the provided data to a scalar via keccak256(data) % l. +/// +/// This function panics if it finds the Keccak-256 preimage for [0; 32]. pub fn keccak256_to_scalar(data: impl AsRef<[u8]>) -> Scalar { let scalar = Scalar::from_bytes_mod_order(keccak256(data.as_ref())); // Monero will explicitly error in this case @@ -84,39 +87,68 @@ impl Commitment { Commitment { mask: Scalar::ONE, amount: 0 } } + /// Create a new Commitment. pub fn new(mask: Scalar, amount: u64) -> Commitment { Commitment { mask, amount } } - /// Calculate a Pedersen commitment, as a point, from the transparent structure. + /// Calculate the Pedersen commitment, as a point, from this transparent structure. pub fn calculate(&self) -> EdwardsPoint { EdwardsPoint::vartime_double_scalar_mul_basepoint(&Scalar::from(self.amount), &H(), &self.mask) } } -/// Decoy data, containing the actual member as well (at index `i`). +/// Decoy data, as used for producing Monero's ring signatures. #[derive(Clone, PartialEq, Eq, Debug, Zeroize, ZeroizeOnDrop)] pub struct Decoys { - pub i: u8, - pub offsets: Vec, - pub ring: Vec<[EdwardsPoint; 2]>, + offsets: Vec, + signer_index: u8, + ring: Vec<[EdwardsPoint; 2]>, } #[allow(clippy::len_without_is_empty)] impl Decoys { - pub fn fee_weight(offsets: &[u64]) -> usize { - varint_len(offsets.len()) + offsets.iter().map(|offset| varint_len(*offset)).sum::() + /// Create a new instance of decoy data. + /// + /// `offsets` are the positions of each ring member within the Monero blockchain, offset from the + /// prior member's position (with the initial ring member offset from 0). + pub fn new(offsets: Vec, signer_index: u8, ring: Vec<[EdwardsPoint; 2]>) -> Option { + if (offsets.len() != ring.len()) || (usize::from(signer_index) >= ring.len()) { + None?; + } + Some(Decoys { offsets, signer_index, ring }) } + /// The length of the ring. pub fn len(&self) -> usize { self.offsets.len() } - pub fn indexes(&self) -> Vec { - let mut res = vec![self.offsets[0]; self.len()]; + /// The positions of the ring members within the Monero blockchain, as their offsets. + /// + /// The list is formatted as the position of the first ring member, then the offset from each + /// ring member to its prior. + pub fn offsets(&self) -> &[u64] { + &self.offsets + } + + /// The positions of the ring members within the Monero blockchain. + pub fn positions(&self) -> Vec { + let mut res = Vec::with_capacity(self.len()); + res.push(self.offsets[0]); for m in 1 .. res.len() { - res[m] = res[m - 1] + self.offsets[m]; + res.push(res[m - 1] + self.offsets[m]); } res } + + /// The index of the signer within the ring. + pub fn signer_index(&self) -> u8 { + self.signer_index + } + + // The ring. + pub fn ring(&self) -> &[[EdwardsPoint; 2]] { + &self.ring + } } diff --git a/coins/monero/ringct/clsag/Cargo.toml b/coins/monero/ringct/clsag/Cargo.toml index dfb2fd49..1e540273 100644 --- a/coins/monero/ringct/clsag/Cargo.toml +++ b/coins/monero/ringct/clsag/Cargo.toml @@ -25,7 +25,6 @@ zeroize = { version = "^1.5", default-features = false, features = ["zeroize_der subtle = { version = "^2.4", default-features = false } # Cryptographic dependencies -sha3 = { version = "0.10", default-features = false } curve25519-dalek = { version = "4", default-features = false, features = ["alloc", "zeroize"] } # Multisig dependencies @@ -50,7 +49,6 @@ std = [ "zeroize/std", "subtle/std", - "sha3/std", "curve25519-dalek/precomputed-tables", "rand_chacha?/std", diff --git a/coins/monero/ringct/clsag/src/lib.rs b/coins/monero/ringct/clsag/src/lib.rs index 1abae389..ae467869 100644 --- a/coins/monero/ringct/clsag/src/lib.rs +++ b/coins/monero/ringct/clsag/src/lib.rs @@ -73,12 +73,12 @@ impl ClsagInput { Err(ClsagError::InvalidRing)?; } let n = u8::try_from(n).unwrap(); - if decoys.i >= n { - Err(ClsagError::InvalidRingMember(decoys.i, n))?; + if decoys.signer_index() >= n { + Err(ClsagError::InvalidRingMember(decoys.signer_index(), n))?; } // Validate the commitment matches - if decoys.ring[usize::from(decoys.i)][1] != commitment.calculate() { + if decoys.ring()[usize::from(decoys.signer_index())][1] != commitment.calculate() { Err(ClsagError::InvalidCommitment)?; } @@ -244,19 +244,19 @@ impl Clsag { A: EdwardsPoint, AH: EdwardsPoint, ) -> ClsagSignCore { - let r: usize = input.decoys.i.into(); + let r: usize = input.decoys.signer_index().into(); let pseudo_out = Commitment::new(mask, input.commitment.amount).calculate(); let mask_delta = input.commitment.mask - mask; - let H = hash_to_point(input.decoys.ring[r][0].compress().0); + let H = hash_to_point(input.decoys.ring()[r][0].compress().0); let D = H * mask_delta; - let mut s = Vec::with_capacity(input.decoys.ring.len()); - for _ in 0 .. input.decoys.ring.len() { + let mut s = Vec::with_capacity(input.decoys.ring().len()); + for _ in 0 .. input.decoys.ring().len() { s.push(Scalar::random(rng)); } let ((D, c_p, c_c), c1) = - core(&input.decoys.ring, I, &pseudo_out, msg, &D, &s, &Mode::Sign(r, A, AH)); + core(input.decoys.ring(), I, &pseudo_out, msg, &D, &s, &Mode::Sign(r, A, AH)); ClsagSignCore { incomplete_clsag: Clsag { D, s, c1 }, @@ -297,13 +297,15 @@ impl Clsag { nonce.deref() * ED25519_BASEPOINT_TABLE, nonce.deref() * hash_to_point( - inputs[i].2.decoys.ring[usize::from(inputs[i].2.decoys.i)][0].compress().0, + inputs[i].2.decoys.ring()[usize::from(inputs[i].2.decoys.signer_index())][0] + .compress() + .0, ), ); // Effectively r - cx, except cx is (c_p x) + (c_c z), where z is the delta between a ring // member's commitment and our input commitment (which will only have a known discrete log // over G if the amounts cancel out) - incomplete_clsag.s[usize::from(inputs[i].2.decoys.i)] = + incomplete_clsag.s[usize::from(inputs[i].2.decoys.signer_index())] = nonce.deref() - ((key_challenge * inputs[i].0.deref()) + challenged_mask); let clsag = incomplete_clsag; @@ -312,7 +314,7 @@ impl Clsag { nonce.zeroize(); debug_assert!(clsag - .verify(&inputs[i].2.decoys.ring, &inputs[i].1, &pseudo_out, &msg) + .verify(inputs[i].2.decoys.ring(), &inputs[i].1, &pseudo_out, &msg) .is_ok()); res.push((clsag, pseudo_out)); diff --git a/coins/monero/ringct/clsag/src/multisig.rs b/coins/monero/ringct/clsag/src/multisig.rs index d8e29475..82d0f88c 100644 --- a/coins/monero/ringct/clsag/src/multisig.rs +++ b/coins/monero/ringct/clsag/src/multisig.rs @@ -35,10 +35,10 @@ impl ClsagInput { // Doesn't domain separate as this is considered part of the larger CLSAG proof // Ring index - transcript.append_message(b"real_spend", [self.decoys.i]); + transcript.append_message(b"real_spend", [self.decoys.signer_index()]); // Ring - for (i, pair) in self.decoys.ring.iter().enumerate() { + for (i, pair) in self.decoys.ring().iter().enumerate() { // Doesn't include global output indexes as CLSAG doesn't care and won't be affected by it // They're just a unreliable reference to this data which will be included in the message // if in use @@ -249,10 +249,10 @@ impl Algorithm for ClsagMultisig { let mut clsag = interim.clsag.clone(); // We produced shares as `r - p x`, yet the signature is `r - p x - c x` // Substract `c x` (saved as `c`) now - clsag.s[usize::from(self.input().decoys.i)] = sum.0 - interim.c; + clsag.s[usize::from(self.input().decoys.signer_index())] = sum.0 - interim.c; if clsag .verify( - &self.input().decoys.ring, + self.input().decoys.ring(), &self.image.expect("verifying a signature despite never processing any addendums").0, &interim.pseudo_out, self.msg.as_ref().unwrap(), diff --git a/coins/monero/src/tests/clsag.rs b/coins/monero/src/tests/clsag.rs index 24444b0f..fe488729 100644 --- a/coins/monero/src/tests/clsag.rs +++ b/coins/monero/src/tests/clsag.rs @@ -64,11 +64,8 @@ fn clsag() { image, ClsagInput::new( Commitment::new(secrets.1, AMOUNT), - Decoys { - i: u8::try_from(real).unwrap(), - offsets: (1 ..= RING_LEN).collect(), - ring: ring.clone(), - }, + Decoys::new((1 ..= RING_LEN).collect(), u8::try_from(real).unwrap(), ring.clone()) + .unwrap(), ) .unwrap(), )], @@ -115,7 +112,7 @@ fn clsag_multisig() { Arc::new(RwLock::new(Some(ClsagDetails::new( ClsagInput::new( Commitment::new(randomness, AMOUNT), - Decoys { i: RING_INDEX, offsets: (1 ..= RING_LEN).collect(), ring: ring.clone() }, + Decoys::new((1 ..= RING_LEN).collect(), RING_INDEX, ring.clone()).unwrap(), ) .unwrap(), mask_sum, diff --git a/coins/monero/src/wallet/decoys.rs b/coins/monero/src/wallet/decoys.rs index effc4887..83762c33 100644 --- a/coins/monero/src/wallet/decoys.rs +++ b/coins/monero/src/wallet/decoys.rs @@ -260,12 +260,15 @@ async fn select_decoys( // members } - res.push(Decoys { - // Binary searches for the real spend since we don't know where it sorted to - i: u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(), - offsets: offset(&ring.iter().map(|output| output.0).collect::>()), - ring: ring.iter().map(|output| output.1).collect(), - }); + res.push( + Decoys::new( + offset(&ring.iter().map(|output| output.0).collect::>()), + // Binary searches for the real spend since we don't know where it sorted to + u8::try_from(ring.partition_point(|x| x.0 < o.0)).unwrap(), + ring.iter().map(|output| output.1).collect(), + ) + .unwrap(), + ); } Ok(res) diff --git a/coins/monero/src/wallet/send/mod.rs b/coins/monero/src/wallet/send/mod.rs index 9f26461a..a6cb107e 100644 --- a/coins/monero/src/wallet/send/mod.rs +++ b/coins/monero/src/wallet/send/mod.rs @@ -22,6 +22,8 @@ use dalek_ff_group as dfg; #[cfg(feature = "multisig")] use frost::FrostError; +use monero_io::varint_len; + use crate::{ Protocol, Commitment, hash, serialize::{ @@ -181,7 +183,7 @@ fn prepare_inputs( tx.prefix.inputs.push(Input::ToKey { amount: None, - key_offsets: decoys.offsets.clone(), + key_offsets: decoys.offsets().to_vec(), key_image: signable[i].1, }); } @@ -518,8 +520,13 @@ impl SignableTransaction { } // Caclculate weight of decoys - let decoy_weights = - inputs.iter().map(|(_, decoy)| Decoys::fee_weight(&decoy.offsets)).collect::>(); + let decoy_weights = inputs + .iter() + .map(|(_, decoys)| { + let offsets = decoys.offsets(); + varint_len(offsets.len()) + offsets.iter().map(|offset| varint_len(*offset)).sum::() + }) + .collect::>(); // Deterministically calculate tx weight and fee let (weight, fee) = diff --git a/coins/monero/src/wallet/send/multisig.rs b/coins/monero/src/wallet/send/multisig.rs index cdff2d68..b3b5f4de 100644 --- a/coins/monero/src/wallet/send/multisig.rs +++ b/coins/monero/src/wallet/send/multisig.rs @@ -108,11 +108,11 @@ impl SignableTransaction { transcript.append_message(b"input_shared_key", input.key_offset().to_bytes()); // Ensure all signers are signing the same rings - transcript.append_message(b"real_spend", [decoys.i]); - for (i, ring_member) in decoys.ring.iter().enumerate() { + transcript.append_message(b"real_spend", [decoys.signer_index()]); + for (i, ring_member) in decoys.ring().iter().enumerate() { transcript .append_message(b"ring_member", [u8::try_from(i).expect("ring size exceeded 255")]); - transcript.append_message(b"ring_member_offset", decoys.offsets[i].to_le_bytes()); + transcript.append_message(b"ring_member_offset", decoys.offsets()[i].to_le_bytes()); transcript.append_message(b"ring_member_key", ring_member[0].compress().to_bytes()); transcript.append_message(b"ring_member_commitment", ring_member[1].compress().to_bytes()); } @@ -356,7 +356,7 @@ impl SignMachine for TransactionSignMachine { tx.prefix.inputs.push(Input::ToKey { amount: None, - key_offsets: value.2.offsets.clone(), + key_offsets: value.2.offsets().to_vec(), key_image: value.0, }); diff --git a/coins/monero/tests/decoys.rs b/coins/monero/tests/decoys.rs index e85eab9d..1967afde 100644 --- a/coins/monero/tests/decoys.rs +++ b/coins/monero/tests/decoys.rs @@ -74,7 +74,7 @@ test!( .await .unwrap(); - selected_fresh_decoy = decoys[0].indexes().contains(&output_tx1.global_index); + selected_fresh_decoy = decoys[0].positions().contains(&output_tx1.global_index); attempts -= 1; } @@ -151,7 +151,7 @@ test!( .await .unwrap(); - selected_fresh_decoy = decoys[0].indexes().contains(&output_tx1.global_index); + selected_fresh_decoy = decoys[0].positions().contains(&output_tx1.global_index); attempts -= 1; }