Skip to content

Commit

Permalink
Change initializeSapling to avoid cloning
Browse files Browse the repository at this point in the history
The Sapling parameters defined in `ironfish::sapling_bls12::SAPLING` are
behind a `lazy_static`. As such, the parameters are not loaded until
accessed fo the first time. The purpose of `initializeSapling` is to
acccess the parameters to ensure that they get loaded early.

The current implementation of `initializeSapling` was calling `.clone()`
on `ironfish::sapling_bls12::SAPLING`, causing large amounts of memory
to be allocated and written, only to be discarded shortly afterwards.
The implementation has been changed to just dereference `SAPLING`,
without doing anything else. This is enough to trigger `lazy_static` to
load the parameters.

Also:

- Removed the `Arc` from `ironfish::sapling_bls12::SAPLING`: `Arc` is
  used to extend lifetimes in multithreaded environments, but the
  lifetime of `SAPLING` is static, so there's no point in extending it.

- Removed `ironfish::sapling_bls12::load` because it's not public and
  redundant
  • Loading branch information
andiflabs committed Jun 18, 2024
1 parent 4e63986 commit fedd9d0
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 15 deletions.
3 changes: 2 additions & 1 deletion ironfish-rust-nodejs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ pub fn generate_key_from_private_key(private_key: String) -> Result<Key> {

#[napi]
pub fn initialize_sapling() {
let _ = sapling_bls12::SAPLING.clone();
// Deref the `SAPLING` lazy-static, to ensure it gets initialized
let _ = &*sapling_bls12::SAPLING;
}

#[napi(constructor)]
Expand Down
18 changes: 4 additions & 14 deletions ironfish-rust/src/sapling_bls12.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
pub use blstrs::Scalar;
use lazy_static::lazy_static;
use std::sync::Arc;

use crate::Sapling;
use lazy_static::lazy_static;

pub use blstrs::Scalar;

// Loads the Sapling object once when dereferenced,
// then reuses the reference on future calls.
lazy_static! {
pub static ref SAPLING: Arc<Sapling> = Arc::new(load());
}

/// Load a sapling object configured to a BLS12 jubjub curve. This is currently
/// the only pairing for which a jubjub curve has been defined, and is the
/// default implementation.
///
/// Provided as a convenience method so clients don't have to depend
/// explicitly on zcash_primitives just to define a JubjubBls12 point.
fn load() -> Sapling {
Sapling::load()
pub static ref SAPLING: Sapling = Sapling::load();
}

0 comments on commit fedd9d0

Please sign in to comment.