Skip to content
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

Receive the randomness used to create descriptions #78

Merged
merged 3 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
18 changes: 9 additions & 9 deletions masp_primitives/src/sapling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ impl From<NoteValue> for u64 {
}

#[derive(Clone, Debug, Copy)]
pub struct Note {
pub struct Note<R = Rseed> {
/// The asset type that the note represents
pub asset_type: AssetType,
/// The value of the note
Expand All @@ -706,7 +706,7 @@ pub struct Note {
/// The public key of the address, g_d^ivk
pub pk_d: jubjub::SubgroupPoint,
/// rseed
pub rseed: Rseed,
pub rseed: R,
}

impl PartialEq for Note {
Expand Down Expand Up @@ -826,30 +826,30 @@ impl Note {
}
}

impl BorshSchema for Note {
impl<T: BorshSchema> BorshSchema for Note<T> {
fn add_definitions_recursively(definitions: &mut BTreeMap<Declaration, Definition>) {
let definition = Definition::Struct {
fields: Fields::NamedFields(vec![
("asset_type".into(), AssetType::declaration()),
("value".into(), u64::declaration()),
("g_d".into(), <[u8; 32]>::declaration()),
("pk_d".into(), <[u8; 32]>::declaration()),
("rseed".into(), Rseed::declaration()),
("rseed".into(), T::declaration()),
]),
};
add_definition(Self::declaration(), definition, definitions);
AssetType::add_definitions_recursively(definitions);
u64::add_definitions_recursively(definitions);
<[u8; 32]>::add_definitions_recursively(definitions);
Rseed::add_definitions_recursively(definitions);
T::add_definitions_recursively(definitions);
}

fn declaration() -> Declaration {
"Note".into()
format!("Note<{}>", T::declaration())
}
}

impl BorshSerialize for Note {
impl<T: BorshSerialize> BorshSerialize for Note<T> {
fn serialize<W: std::io::Write>(&self, writer: &mut W) -> io::Result<()> {
// Write asset type
self.asset_type.serialize(writer)?;
Expand All @@ -865,7 +865,7 @@ impl BorshSerialize for Note {
}
}

impl BorshDeserialize for Note {
impl<T: BorshDeserialize> BorshDeserialize for Note<T> {
fn deserialize_reader<R: Read>(reader: &mut R) -> io::Result<Self> {
// Read asset type
let asset_type = AssetType::deserialize_reader(reader)?;
Expand All @@ -880,7 +880,7 @@ impl BorshDeserialize for Note {
let pk_d = Option::from(jubjub::SubgroupPoint::from_bytes(&pk_d_bytes))
.ok_or_else(|| io::Error::new(io::ErrorKind::InvalidData, "pk_d not in field"))?;
// Read note plaintext lead byte
let rseed = Rseed::deserialize_reader(reader)?;
let rseed = T::deserialize_reader(reader)?;
// Finally construct note object
Ok(Note {
asset_type,
Expand Down
28 changes: 10 additions & 18 deletions masp_primitives/src/sapling/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@ pub trait TxProver {
value: u64,
anchor: bls12_381::Scalar,
merkle_path: MerklePath<Node>,
rcv: jubjub::Fr,
) -> Result<([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint, PublicKey), ()>;

/// Create the value commitment and proof for a MASP OutputDescription,
/// while accumulating its value commitment randomness inside the context for later
/// use.
///
#[allow(clippy::too_many_arguments)]
fn output_proof(
&self,
ctx: &mut Self::SaplingProvingContext,
Expand All @@ -52,6 +54,7 @@ pub trait TxProver {
rcm: jubjub::Fr,
asset_type: AssetType,
value: u64,
rcv: jubjub::Fr,
) -> ([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint);

/// Create the value commitment, and proof for a MASP ConvertDescription,
Expand All @@ -65,6 +68,7 @@ pub trait TxProver {
value: u64,
anchor: bls12_381::Scalar,
merkle_path: MerklePath<Node>,
rcv: jubjub::Fr,
) -> Result<([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint), ()>;

/// Create the `bindingSig` for a Sapling transaction. All calls to
Expand All @@ -80,9 +84,6 @@ pub trait TxProver {

#[cfg(any(test, feature = "test-dependencies"))]
pub mod mock {
use ff::Field;
use rand_core::OsRng;

use crate::{
asset_type::AssetType,
constants::SPENDING_KEY_GENERATOR,
Expand Down Expand Up @@ -115,13 +116,9 @@ pub mod mock {
value: u64,
_anchor: bls12_381::Scalar,
_merkle_path: MerklePath<Node>,
rcv: jubjub::Fr,
) -> Result<([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint, PublicKey), ()> {
let mut rng = OsRng;

let cv = asset_type
.value_commitment(value, jubjub::Fr::random(&mut rng))
.commitment()
.into();
let cv = asset_type.value_commitment(value, rcv).commitment().into();

let rk =
PublicKey(proof_generation_key.ak.into()).randomize(ar, SPENDING_KEY_GENERATOR);
Expand All @@ -137,13 +134,9 @@ pub mod mock {
_rcm: jubjub::Fr,
asset_type: AssetType,
value: u64,
rcv: jubjub::Fr,
) -> ([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint) {
let mut rng = OsRng;

let cv = asset_type
.value_commitment(value, jubjub::Fr::random(&mut rng))
.commitment()
.into();
let cv = asset_type.value_commitment(value, rcv).commitment().into();

([0u8; GROTH_PROOF_SIZE], cv)
}
Expand All @@ -155,11 +148,10 @@ pub mod mock {
value: u64,
_anchor: bls12_381::Scalar,
_merkle_path: MerklePath<Node>,
rcv: jubjub::Fr,
) -> Result<([u8; GROTH_PROOF_SIZE], jubjub::ExtendedPoint), ()> {
let mut rng = OsRng;

let cv = allowed_conversion
.value_commitment(value, jubjub::Fr::random(&mut rng))
.value_commitment(value, rcv)
.commitment()
.into();

Expand Down
23 changes: 14 additions & 9 deletions masp_primitives/src/sapling/util.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use blake2b_simd::Params;
use ff::Field;
use rand_core::{CryptoRng, RngCore};

use crate::consensus::{self, BlockHeight, NetworkUpgrade};

use super::Rseed;
use ff::Field;
use rand_core::{CryptoRng, RngCore};

pub fn hash_to_scalar(persona: &[u8], a: &[u8], b: &[u8]) -> jubjub::Fr {
let mut hasher = Params::new().hash_length(64).personal(persona).to_state();
Expand All @@ -19,19 +19,24 @@ pub fn generate_random_rseed<P: consensus::Parameters, R: RngCore + CryptoRng>(
height: BlockHeight,
rng: &mut R,
) -> Rseed {
generate_random_rseed_internal(params, height, rng)
if params.is_nu_active(NetworkUpgrade::MASP, height) {
let mut buffer = [0u8; 32];
rng.fill_bytes(&mut buffer);
Rseed::AfterZip212(buffer)
} else {
Rseed::BeforeZip212(jubjub::Fr::random(rng))
}
}

pub(crate) fn generate_random_rseed_internal<P: consensus::Parameters, R: RngCore>(
pub(crate) fn generate_random_rseed_internal<P: consensus::Parameters>(
params: &P,
height: BlockHeight,
rng: &mut R,
before: jubjub::Fr,
after: [u8; 32],
Comment on lines +34 to +35
Copy link
Contributor

Choose a reason for hiding this comment

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

Both before and after are provided in BuildParams, but only one will be selected depending on the version. However, since NetworkUpgrade::MASP is actually activated at height 0, we don't need to do the check. Can we just use after? If that makes sense, we may not need both fn output_rcm and fn output_rseed in RngBuildParams either.
BeforeZip212 seems like something old that we didn't clean up, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both before and after are provided in BuildParams, but only one will be selected depending on the version. However, since NetworkUpgrade::MASP is actually activated at height 0, we don't need to do the check. Can we just use after? If that makes sense, we may not need both fn output_rcm and fn output_rseed in RngBuildParams either. BeforeZip212 seems like something old that we didn't clean up, right?

Just took a quick check, and it seems the activation height depends on the network. While NetworkUpgrade::MASP activates on block 0 for MainNetwork, it seems to activate only on block 1 for TestNetwork. That being said, I agree with your overall point. Could we maybe remove BeforeZip212 in a separate PR?

) -> Rseed {
if params.is_nu_active(NetworkUpgrade::MASP, height) {
let mut buffer = [0u8; 32];
rng.fill_bytes(&mut buffer);
Rseed::AfterZip212(buffer)
Rseed::AfterZip212(after)
} else {
Rseed::BeforeZip212(jubjub::Fr::random(rng))
Rseed::BeforeZip212(before)
}
}
Loading
Loading