Use reference in zcheck, to avoid allocations

sebastiencs · sebastiencs · commit 4b2450746aef · 2024-10-04T10:37:25.000+02:00
diff --git a/ledger/src/proofs/transaction.rs b/ledger/src/proofs/transaction.rs
@@ -4633,7 +4633,7 @@ pub(super) mod tests {
     }
 
     #[test]
-    fn test_zkapp_proof_sig() {
+    fn test_proof_zkapp_sig() {
         let Ok(data) = std::fs::read(
             Path::new(env!("CARGO_MANIFEST_DIR"))
                 .join(devnet_circuit_directory())
diff --git a/ledger/src/scan_state/transaction_logic.rs b/ledger/src/scan_state/transaction_logic.rs
@@ -1541,7 +1541,7 @@ pub mod zkapp_command {
         type B;
 
         /// zkapp check
-        fn zcheck(&self, label: String, x: Self::B) -> Result<(), String>;
+        fn zcheck<F: Fn() -> String>(&self, label: F, x: &Self::B) -> Result<(), String>;
     }
 
     impl<T> OutSnarkCheck for T
@@ -1552,11 +1552,11 @@ pub mod zkapp_command {
         type B = T;
 
         /// zkapp check
-        fn zcheck(&self, label: String, rhs: Self::B) -> Result<(), String> {
-            if *self == rhs {
+        fn zcheck<F: Fn() -> String>(&self, label: F, rhs: &Self::B) -> Result<(), String> {
+            if self == rhs {
                 Ok(())
             } else {
-                Err(format!("Equality check failed: {}", label))
+                Err(format!("Equality check failed: {}", label()))
             }
         }
     }
@@ -1625,15 +1625,15 @@ pub mod zkapp_command {
         type B = T;
 
         /// zkapp check
-        fn zcheck(&self, label: String, rhs: Self::B) -> Result<(), String> {
+        fn zcheck<F: Fn() -> String>(&self, label: F, rhs: &Self::B) -> Result<(), String> {
             /*println!(
                 "bounds check lower {:?} rhs {:?} upper {:?}",
                 self.lower, rhs, self.upper
             );*/
-            if self.lower <= rhs && rhs <= self.upper {
+            if &self.lower <= rhs && rhs <= &self.upper {
                 Ok(())
             } else {
-                Err(format!("Bounds check failed: {}", label))
+                Err(format!("Bounds check failed: {}", label()))
             }
         }
     }
@@ -1740,7 +1740,7 @@ pub mod zkapp_command {
         T: OutSnarkCheck<A = T>,
     {
         /// zkapp check
-        pub fn zcheck(&self, label: String, rhs: T::B) -> Result<(), String> {
+        pub fn zcheck<F: Fn() -> String>(&self, label: F, rhs: &T::B) -> Result<(), String> {
             // println!("[rust] check {}, {:?}", label, ret);
             match self {
                 Self::Ignore => Ok(()),
@@ -1806,9 +1806,12 @@ pub mod zkapp_command {
 
     impl EpochLedger {
         pub fn epoch_ledger(&self, t: &protocol_state::EpochLedger<Fp>) -> Result<(), String> {
-            self.hash.zcheck("epoch_ledger_hash".to_string(), t.hash)?;
-            self.total_currency
-                .zcheck("epoch_ledger_total_currency".to_string(), t.total_currency)
+            self.hash
+                .zcheck(|| "epoch_ledger_hash".to_string(), &t.hash)?;
+            self.total_currency.zcheck(
+                || "epoch_ledger_total_currency".to_string(),
+                &t.total_currency,
+            )
         }
     }
 
@@ -1913,15 +1916,15 @@ pub mod zkapp_command {
             self.ledger.epoch_ledger(&t.ledger)?;
             // ignore seed
             self.start_checkpoint.zcheck(
-                format!("{}_{}", label, "start_checkpoint"),
-                t.start_checkpoint,
+                || format!("{}_{}", label, "start_checkpoint"),
+                &t.start_checkpoint,
             )?;
             self.lock_checkpoint.zcheck(
-                format!("{}_{}", label, "lock_checkpoint"),
-                t.lock_checkpoint,
+                || format!("{}_{}", label, "lock_checkpoint"),
+                &t.lock_checkpoint,
             )?;
             self.epoch_length
-                .zcheck(format!("{}_{}", label, "epoch_length"), t.epoch_length)
+                .zcheck(|| format!("{}_{}", label, "epoch_length"), &t.epoch_length)
         }
 
         pub fn gen() -> Self {
@@ -1956,16 +1959,16 @@ pub mod zkapp_command {
         /// zkapp check
         pub fn zcheck(&self, s: &ProtocolStateView) -> Result<(), String> {
             self.snarked_ledger_hash
-                .zcheck("snarker_ledger_hash".to_string(), s.snarked_ledger_hash)?;
+                .zcheck(|| "snarker_ledger_hash".to_string(), &s.snarked_ledger_hash)?;
             self.blockchain_length
-                .zcheck("blockchain_length".to_string(), s.blockchain_length)?;
+                .zcheck(|| "blockchain_length".to_string(), &s.blockchain_length)?;
             self.min_window_density
-                .zcheck("min_window_density".to_string(), s.min_window_density)?;
+                .zcheck(|| "min_window_density".to_string(), &s.min_window_density)?;
             self.total_currency
-                .zcheck("total_currency".to_string(), s.total_currency)?;
+                .zcheck(|| "total_currency".to_string(), &s.total_currency)?;
             self.global_slot_since_genesis.zcheck(
-                "global_slot_since_genesis".to_string(),
-                s.global_slot_since_genesis,
+                || "global_slot_since_genesis".to_string(),
+                &s.global_slot_since_genesis,
             )?;
             self.staking_epoch_data
                 .epoch_data("staking_epoch_data", &s.staking_epoch_data)?;
@@ -2188,31 +2191,31 @@ pub mod zkapp_command {
             let mut ret = vec![
                 (
                     TransactionFailure::AccountBalancePreconditionUnsatisfied,
-                    self.balance.zcheck("balance".to_string(), a.balance),
+                    self.balance.zcheck(|| "balance".to_string(), &a.balance),
                 ),
                 (
                     TransactionFailure::AccountNoncePreconditionUnsatisfied,
-                    self.nonce.zcheck("nonce".to_string(), a.nonce),
+                    self.nonce.zcheck(|| "nonce".to_string(), &a.nonce),
                 ),
                 (
                     TransactionFailure::AccountReceiptChainHashPreconditionUnsatisfied,
                     self.receipt_chain_hash
-                        .zcheck("receipt_chain_hash".to_string(), a.receipt_chain_hash.0),
+                        .zcheck(|| "receipt_chain_hash".to_string(), &a.receipt_chain_hash.0),
                 ),
                 (
                     TransactionFailure::AccountDelegatePreconditionUnsatisfied,
                     self.delegate.zcheck(
-                        "delegate".to_string(),
-                        a.delegate.clone().unwrap_or_else(invalid_public_key),
+                        || "delegate".to_string(),
+                        &a.delegate.clone().unwrap_or_else(invalid_public_key),
                     ),
                 ),
                 (
                     TransactionFailure::AccountActionStatePreconditionUnsatisfied,
-                    match zkapp
-                        .action_state
-                        .iter()
-                        .find(|state| self.action_state.zcheck("".to_string(), **state).is_ok())
-                    {
+                    match zkapp.action_state.iter().find(|state| {
+                        self.action_state
+                            .zcheck(|| "".to_string(), &**state)
+                            .is_ok()
+                    }) {
                         None => Err("Action state mismatch".to_string()),
                         Some(_) => Ok(()),
                     },
@@ -2222,19 +2225,19 @@ pub mod zkapp_command {
             for (i, (c, v)) in self.state.iter().zip(zkapp.app_state.iter()).enumerate() {
                 ret.push((
                     TransactionFailure::AccountAppStatePreconditionUnsatisfied(i as u64),
-                    c.zcheck(format!("state[{}]", i), *v),
+                    c.zcheck(|| format!("state[{}]", i), &*v),
                 ));
             }
 
             let mut ret2 = vec![
                 (
                     TransactionFailure::AccountProvedStatePreconditionUnsatisfied,
                     self.proved_state
-                        .zcheck("proved_state".to_string(), zkapp.proved_state),
+                        .zcheck(|| "proved_state".to_string(), &zkapp.proved_state),
                 ),
                 (
                     TransactionFailure::AccountIsNewPreconditionUnsatisfied,
-                    self.is_new.zcheck("is_new".to_string(), new_account),
+                    self.is_new.zcheck(|| "is_new".to_string(), &new_account),
                 ),
             ];
 
@@ -5853,8 +5856,8 @@ where
             Eff::CheckValidWhilePrecondition(valid_while, global_state) => PerformResult::Bool(
                 valid_while
                     .zcheck(
-                        "valid_while_precondition".to_string(),
-                        global_state.block_global_slot,
+                        || "valid_while_precondition".to_string(),
+                        &global_state.block_global_slot,
                     )
                     .is_ok(),
             ),
diff --git a/ledger/src/zkapps/intefaces.rs b/ledger/src/zkapps/intefaces.rs
@@ -81,8 +81,7 @@ pub trait ZkappHandler {
         global_state: &mut Self::GlobalState,
         w: &mut Self::W,
     ) -> Self::Bool;
-    fn init_account(account_update: &Self::AccountUpdate, account: &Self::Account)
-        -> Self::Account;
+    fn init_account(account_update: &Self::AccountUpdate, account: Self::Account) -> Self::Account;
 }
 
 pub struct Opt<T> {
diff --git a/ledger/src/zkapps/non_snark.rs b/ledger/src/zkapps/non_snark.rs
@@ -171,17 +171,17 @@ impl<
     ) -> Self::Bool {
         valid_while
             .zcheck(
-                "valid_while_precondition".to_string(),
-                global_state.block_global_slot,
+                || "valid_while_precondition".to_string(),
+                &global_state.block_global_slot,
             )
             .is_ok()
     }
 
     fn init_account(
         _account_update: &Self::AccountUpdate,
-        account: &Self::Account,
+        account: Self::Account,
     ) -> Self::Account {
-        account.clone()
+        account
     }
 }
 
diff --git a/ledger/src/zkapps/snark.rs b/ledger/src/zkapps/snark.rs
@@ -253,18 +253,15 @@ impl ZkappHandler for SnarkHandler {
             .var()
     }
 
-    fn init_account(
-        account_update: &Self::AccountUpdate,
-        account: &Self::Account,
-    ) -> Self::Account {
+    fn init_account(account_update: &Self::AccountUpdate, account: Self::Account) -> Self::Account {
         let AccountUpdateSkeleton {
             body: account_update,
             authorization: _,
         } = account_update;
         let account = Box::new(crate::Account {
             public_key: account_update.data.public_key.clone(),
             token_id: account_update.data.token_id.clone(),
-            ..(*account.data).clone()
+            ..*account.data
         });
         let account2 = account.clone();
         let account = WithLazyHash::new(account, move |w: &mut Witness<Fp>| {
diff --git a/ledger/src/zkapps/zkapp_logic.rs b/ledger/src/zkapps/zkapp_logic.rs
@@ -1177,7 +1177,7 @@ where
         ((), ())
     };
 
-    let a = Z::Handler::init_account(&account_update, &a);
+    let a = Z::Handler::init_account(&account_update, a);
 
     let local_delta = account_update_balance_change.negate();
 

Original file line number	Diff line number	Diff line change
`@@ -4633,7 +4633,7 @@ pub(super) mod tests {`
`4633`	`4633`	`}`
`4634`	`4634`
`4635`	`4635`	`#[test]`
`4636`		`- fn test_zkapp_proof_sig() {`
	`4636`	`+ fn test_proof_zkapp_sig() {`
`4637`	`4637`	`let Ok(data) = std::fs::read(`
`4638`	`4638`	`Path::new(env!("CARGO_MANIFEST_DIR"))`
`4639`	`4639`	`.join(devnet_circuit_directory())`
Original file line number	Diff line number	Diff line change
`@@ -81,8 +81,7 @@ pub trait ZkappHandler {`
`81`	`81`	`global_state: &mut Self::GlobalState,`
`82`	`82`	`w: &mut Self::W,`
`83`	`83`	`) -> Self::Bool;`
`84`		`- fn init_account(account_update: &Self::AccountUpdate, account: &Self::Account)`
`85`		`- -> Self::Account;`
	`84`	`+ fn init_account(account_update: &Self::AccountUpdate, account: Self::Account) -> Self::Account;`
`86`	`85`	`}`
`87`	`86`
`88`	`87`	`pub struct Opt<T> {`
Original file line number	Diff line number	Diff line change
`@@ -171,17 +171,17 @@ impl<`
`171`	`171`	`) -> Self::Bool {`
`172`	`172`	`valid_while`
`173`	`173`	`.zcheck(`
`174`		`- "valid_while_precondition".to_string(),`
`175`		`- global_state.block_global_slot,`
	`174`	`+ \|\| "valid_while_precondition".to_string(),`
	`175`	`+ &global_state.block_global_slot,`
`176`	`176`	`)`
`177`	`177`	`.is_ok()`
`178`	`178`	`}`
`179`	`179`
`180`	`180`	`fn init_account(`
`181`	`181`	`_account_update: &Self::AccountUpdate,`
`182`		`- account: &Self::Account,`
	`182`	`+ account: Self::Account,`
`183`	`183`	`) -> Self::Account {`
`184`		`- account.clone()`
	`184`	`+ account`
`185`	`185`	`}`
`186`	`186`	`}`
`187`	`187`