Sapling: Decouple ProveAndSign from Build

random-zebra · Fuzzbawls · commit ff2dec22c8e8 · 2020-12-18T21:35:22.000-08:00
Github-Pull: #2065 Rebased-From: efd7139
diff --git a/src/sapling/transaction_builder.cpp b/src/sapling/transaction_builder.cpp
@@ -209,66 +209,31 @@ void TransactionBuilder::SendChangeTo(CTxDestination& changeAddr)
     saplingChangeAddr = nullopt;
 }
 
-TransactionBuilderResult TransactionBuilder::Build()
+TransactionBuilderResult TransactionBuilder::ProveAndSign()
 {
     //
-    // Consistency checks
+    // Sapling spend descriptions
     //
-    // Valid fee
-    if (fee < 0) {
-        return TransactionBuilderResult("Fee cannot be negative");
-    }
-
-    // Valid change
-    CAmount change = mtx.sapData->valueBalance - fee;
-    for (auto& tIn : tIns) {
-        change += tIn.value;
-    }
-    for (auto& tOut : mtx.vout) {
-        change -= tOut.nValue;
-    }
-    if (change < 0) {
-        return TransactionBuilderResult("Change cannot be negative");
-    }
+    if (!spends.empty() || !outputs.empty()) {
 
-    //
-    // Change output
-    //
+        auto ctx = librustzcash_sapling_proving_ctx_init();
 
-    if (change > 0) {
-        // If we get here and the change is dust, add it to the fee
-        CAmount dustThreshold = (spends.empty() && outputs.empty()) ? GetDustThreshold(minRelayTxFee) :
-                                GetShieldedDustThreshold(minRelayTxFee);
-        if (change > dustThreshold) {
-            // Send change to the specified change address. If no change address
-            // was set, send change to the first Sapling address given as input
-            // (A t-address can only be used as the change address if explicitly set.)
-            if (saplingChangeAddr) {
-                AddSaplingOutput(saplingChangeAddr->first, saplingChangeAddr->second, change);
-            } else if (tChangeAddr) {
-                // tChangeAddr has already been validated.
-                AddTransparentOutput(*tChangeAddr, change);
-            } else if (!spends.empty()) {
-                auto fvk = spends[0].expsk.full_viewing_key();
-                auto note = spends[0].note;
-                libzcash::SaplingPaymentAddress changeAddr(note.d, note.pk_d);
-                AddSaplingOutput(fvk.ovk, changeAddr, change);
-            } else {
-                return TransactionBuilderResult("Could not determine change address");
+        // Create Sapling OutputDescriptions
+        for (auto output : outputs) {
+            // Check this out here as well to provide better logging.
+            if (!output.note.cmu()) {
+                librustzcash_sapling_proving_ctx_free(ctx);
+                return TransactionBuilderResult("Output is invalid");
             }
-        } else {
-            // Not used after, but update for consistency
-            fee += change;
-            change = 0;
-        }
-    }
 
-    //
-    // Sapling spends and outputs
-    //
-    if (!spends.empty() || !outputs.empty()) {
+            auto odesc = output.Build(ctx);
+            if (!odesc) {
+                librustzcash_sapling_proving_ctx_free(ctx);
+                return TransactionBuilderResult("Failed to create output description");
+            }
 
-        auto ctx = librustzcash_sapling_proving_ctx_init();
+            mtx.sapData->vShieldedOutput.push_back(odesc.get());
+        }
 
         // Create Sapling SpendDescriptions
         for (auto spend : spends) {
@@ -307,23 +272,6 @@ TransactionBuilderResult TransactionBuilder::Build()
             mtx.sapData->vShieldedSpend.push_back(sdesc);
         }
 
-        // Create Sapling OutputDescriptions
-        for (auto output : outputs) {
-            // Check this out here as well to provide better logging.
-            if (!output.note.cmu()) {
-                librustzcash_sapling_proving_ctx_free(ctx);
-                return TransactionBuilderResult("Output is invalid");
-            }
-
-            auto odesc = output.Build(ctx);
-            if (!odesc) {
-                librustzcash_sapling_proving_ctx_free(ctx);
-                return TransactionBuilderResult("Failed to create output description");
-            }
-
-            mtx.sapData->vShieldedOutput.push_back(odesc.get());
-        }
-
         //
         // Signatures
         //
@@ -375,3 +323,60 @@ TransactionBuilderResult TransactionBuilder::Build()
 
     return TransactionBuilderResult(CTransaction(mtx));
 }
+
+TransactionBuilderResult TransactionBuilder::Build()
+{
+    //
+    // Consistency checks
+    //
+    // Valid fee
+    if (fee < 0) {
+        return TransactionBuilderResult("Fee cannot be negative");
+    }
+
+    // Valid change
+    CAmount change = mtx.sapData->valueBalance - fee;
+    for (auto& tIn : tIns) {
+        change += tIn.value;
+    }
+    for (auto& tOut : mtx.vout) {
+        change -= tOut.nValue;
+    }
+    if (change < 0) {
+        return TransactionBuilderResult("Change cannot be negative");
+    }
+
+    //
+    // Change output
+    //
+
+    if (change > 0) {
+        // If we get here and the change is dust, add it to the fee
+        CAmount dustThreshold = (spends.empty() && outputs.empty()) ? GetDustThreshold(minRelayTxFee) :
+                                GetShieldedDustThreshold(minRelayTxFee);
+        if (change > dustThreshold) {
+            // Send change to the specified change address. If no change address
+            // was set, send change to the first Sapling address given as input
+            // (A t-address can only be used as the change address if explicitly set.)
+            if (saplingChangeAddr) {
+                AddSaplingOutput(saplingChangeAddr->first, saplingChangeAddr->second, change);
+            } else if (tChangeAddr) {
+                // tChangeAddr has already been validated.
+                AddTransparentOutput(*tChangeAddr, change);
+            } else if (!spends.empty()) {
+                auto fvk = spends[0].expsk.full_viewing_key();
+                auto note = spends[0].note;
+                libzcash::SaplingPaymentAddress changeAddr(note.d, note.pk_d);
+                AddSaplingOutput(fvk.ovk, changeAddr, change);
+            } else {
+                return TransactionBuilderResult("Could not determine change address");
+            }
+        } else {
+            // Not used after, but update for consistency
+            fee += change;
+            change = 0;
+        }
+    }
+
+    return ProveAndSign();
+}
diff --git a/src/sapling/transaction_builder.h b/src/sapling/transaction_builder.h
@@ -121,6 +121,8 @@ class TransactionBuilder
     void SendChangeTo(CTxDestination& changeAddr);
 
     TransactionBuilderResult Build();
+    // Add Sapling Spend/Output descriptions, binding sig, and transparent signatures
+    TransactionBuilderResult ProveAndSign();
 };
 
 #endif /* TRANSACTION_BUILDER_H */
