Merge pull request #5311 from str4d/wallet-load-orchard-perf

Fix regression in wallet load performance

src/rust/src/orchard_ffi.rs

@@ -200,6 +200,48 @@ impl BatchValidator {
     fn new() -> Self {
         BatchValidator { signatures: vec![] }
     }
+
+    fn add_bundle(&mut self, bundle: &Bundle<Authorized, Amount>, txid: TxId) {
+        for action in bundle.actions().iter() {
+            self.signatures.push(BundleSignature {
+                signature: action
+                    .rk()
+                    .create_batch_item(action.authorization().clone(), txid.as_ref()),
+            });
+        }
+
+        self.signatures.push(BundleSignature {
+            signature: bundle.binding_validating_key().create_batch_item(
+                bundle.authorization().binding_signature().clone(),
+                txid.as_ref(),
+            ),
+        });
+    }
+
+    fn validate(&self) -> bool {
+        if self.signatures.is_empty() {
+            // An empty batch is always valid, but is not free to run; skip it.
+            return true;
+        }
+
+        let mut validator = redpallas::batch::Verifier::new();
+        for sig in self.signatures.iter() {
+            validator.queue(sig.signature.clone());
+        }
+
+        match validator.verify(OsRng) {
+            Ok(()) => true,
+            Err(e) => {
+                error!("RedPallas batch validation failed: {}", e);
+                // TODO: Try sub-batches to figure out which signatures are invalid. We can
+                // postpone this for now:
+                // - For per-transaction batching (when adding to the mempool), we don't care
+                //   which signature within the transaction failed.
+                // - For per-block batching, we currently don't care which transaction failed.
+                false
+            }
+        }
+    }
 }
 
 /// Creates a RedPallas batch validation context.
@@ -234,22 +276,7 @@ pub extern "C" fn orchard_batch_add_bundle(
     let txid = unsafe { txid.as_ref() }.cloned().map(TxId::from_bytes);
 
     match (batch, bundle, txid) {
-        (Some(batch), Some(bundle), Some(txid)) => {
-            for action in bundle.actions().iter() {
-                batch.signatures.push(BundleSignature {
-                    signature: action
-                        .rk()
-                        .create_batch_item(action.authorization().clone(), txid.as_ref()),
-                });
-            }
-
-            batch.signatures.push(BundleSignature {
-                signature: bundle.binding_validating_key().create_batch_item(
-                    bundle.authorization().binding_signature().clone(),
-                    txid.as_ref(),
-                ),
-            });
-        }
+        (Some(batch), Some(bundle), Some(txid)) => batch.add_bundle(bundle, txid),
         (_, _, None) => error!("orchard_batch_add_bundle() called without txid!"),
         (Some(_), None, Some(txid)) => debug!("Tx {} has no Orchard component", txid),
         (None, Some(_), _) => debug!("Orchard BatchValidator not provided, assuming disabled."),
@@ -264,23 +291,7 @@ pub extern "C" fn orchard_batch_add_bundle(
 #[no_mangle]
 pub extern "C" fn orchard_batch_validate(batch: *const BatchValidator) -> bool {
     if let Some(batch) = unsafe { batch.as_ref() } {
-        let mut validator = redpallas::batch::Verifier::new();
-        for sig in batch.signatures.iter() {
-            validator.queue(sig.signature.clone());
-        }
-
-        match validator.verify(OsRng) {
-            Ok(()) => true,
-            Err(e) => {
-                error!("RedPallas batch validation failed: {}", e);
-                // TODO: Try sub-batches to figure out which signatures are invalid. We can
-                // postpone this for now:
-                // - For per-transaction batching (when adding to the mempool), we don't care
-                //   which signature within the transaction failed.
-                // - For per-block batching, we currently don't care which transaction failed.
-                false
-            }
-        }
+        batch.validate()
     } else {
         // The orchard::BatchValidator C++ class uses null to represent a disabled batch
         // validator.

src/wallet/walletdb.cpp

@@ -298,8 +298,9 @@ public:
     bool fAnyUnordered;
     int nFileVersion;
     vector<uint256> vWalletUpgrade;
+    orchard::AuthValidator orchardAuth;
 
-    CWalletScanState() {
+    CWalletScanState(): orchardAuth(orchard::AuthValidator::Batch()) {
         nKeys = nCKeys = nKeyMeta = nZKeys = nCZKeys = nZKeyMeta = nSapZAddrs = 0;
         fIsEncrypted = false;
         fAnyUnordered = false;
@@ -338,11 +339,9 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
             ssValue >> wtx;
             CValidationState state;
             auto verifier = ProofVerifier::Strict();
-            auto orchardAuth = orchard::AuthValidator::Batch();
             if (!(
-                CheckTransaction(wtx, state, verifier, orchardAuth) &&
+                CheckTransaction(wtx, state, verifier, wss.orchardAuth) &&
                 (wtx.GetHash() == hash) &&
-                orchardAuth.Validate() &&
                 state.IsValid())
             ) {
                 return false;
@@ -802,6 +801,12 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
                 LogPrintf("%s\n", strErr);
         }
         pcursor->close();
+
+        // Run the Orchard batch validator; if it fails, treat it like a bad transaction record.
+        if (!wss.orchardAuth.Validate()) {
+            fNoncriticalErrors = true;
+            SoftSetBoolArg("-rescan", true);
+        }
     }
     catch (const boost::thread_interrupted&) {
         throw;