Throw more descriptive exceptions when the constraint system is violated.

src/gtest/test_joinsplit.cpp

@@ -154,6 +154,63 @@ void test_full_api(ZCJoinSplit* js)
     ));
 }
 
+// Invokes the API (but does not compute a proof)
+// to test exceptions
+void invokeAPI(
+    ZCJoinSplit* js,
+    const boost::array<JSInput, 2>& inputs,
+    const boost::array<JSOutput, 2>& outputs,
+    uint64_t vpub_old,
+    uint64_t vpub_new,
+    const uint256& rt
+) {
+    uint256 ephemeralKey;
+    uint256 randomSeed;
+    uint256 pubKeyHash = random_uint256();
+    boost::array<uint256, 2> macs;
+    boost::array<uint256, 2> nullifiers;
+    boost::array<uint256, 2> commitments;
+    boost::array<ZCNoteEncryption::Ciphertext, 2> ciphertexts;
+
+    boost::array<Note, 2> output_notes;
+
+    ZCProof proof = js->prove(
+        inputs,
+        outputs,
+        output_notes,
+        ciphertexts,
+        ephemeralKey,
+        pubKeyHash,
+        randomSeed,
+        macs,
+        nullifiers,
+        commitments,
+        vpub_old,
+        vpub_new,
+        rt,
+        false
+    );
+}
+
+void invokeAPIFailure(
+    ZCJoinSplit* js,
+    const boost::array<JSInput, 2>& inputs,
+    const boost::array<JSOutput, 2>& outputs,
+    uint64_t vpub_old,
+    uint64_t vpub_new,
+    const uint256& rt,
+    std::string reason
+)
+{
+    try {
+        invokeAPI(js, inputs, outputs, vpub_old, vpub_new, rt);
+    } catch(std::invalid_argument const & err) {
+        EXPECT_EQ(err.what(), reason);
+    } catch(...) {
+        FAIL() << "Expected invalid_argument exception.";
+    }
+}
+
 TEST(joinsplit, h_sig)
 {
     auto js = ZCJoinSplit::Unopened();
@@ -233,10 +290,204 @@ for test_input in TEST_VECTORS:
     delete js;
 }
 
+void increment_note_witnesses(
+    const uint256& element,
+    std::vector<ZCIncrementalWitness>& witnesses,
+    ZCIncrementalMerkleTree& tree
+)
+{
+    tree.append(element);
+    for (ZCIncrementalWitness& w : witnesses) {
+        w.append(element);
+    }
+    witnesses.push_back(tree.witness());
+}
+
 TEST(joinsplit, full_api_test)
 {
     auto js = ZCJoinSplit::Generate();
 
+    {
+        std::vector<ZCIncrementalWitness> witnesses;
+        ZCIncrementalMerkleTree tree;
+        increment_note_witnesses(uint256(), witnesses, tree);
+        SpendingKey sk = SpendingKey::random();
+        PaymentAddress addr = sk.address();
+        Note note1(addr.a_pk, 100, random_uint256(), random_uint256());
+        increment_note_witnesses(note1.cm(), witnesses, tree);
+        Note note2(addr.a_pk, 100, random_uint256(), random_uint256());
+        increment_note_witnesses(note2.cm(), witnesses, tree);
+        Note note3(addr.a_pk, 2100000000000001, random_uint256(), random_uint256());
+        increment_note_witnesses(note3.cm(), witnesses, tree);
+        Note note4(addr.a_pk, 1900000000000000, random_uint256(), random_uint256());
+        increment_note_witnesses(note4.cm(), witnesses, tree);
+        Note note5(addr.a_pk, 1900000000000000, random_uint256(), random_uint256());
+        increment_note_witnesses(note5.cm(), witnesses, tree);
+
+        // Should work
+        invokeAPI(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root());
+
+        // lhs > MAX_MONEY
+        invokeAPIFailure(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        2100000000000001,
+        0,
+        tree.root(),
+        "nonsensical vpub_old value");
+
+        // rhs > MAX_MONEY
+        invokeAPIFailure(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        2100000000000001,
+        tree.root(),
+        "nonsensical vpub_new value");
+
+        // input is not in tree
+        invokeAPIFailure(js,
+        {
+            JSInput(witnesses[0], note1, sk),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        100,
+        tree.root(),
+        "joinsplit not anchored to the correct root");
+
+        // input is in the tree now! this should work
+        invokeAPI(js,
+        {
+            JSInput(witnesses[1], note1, sk),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        100,
+        tree.root());
+
+        // Wrong secret key
+        invokeAPIFailure(js,
+        {
+            JSInput(witnesses[1], note1, SpendingKey::random()),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root(),
+        "input note not authorized to spend with given key");
+
+        // Absurd input value
+        invokeAPIFailure(js,
+        {
+            JSInput(witnesses[3], note3, sk),
+            JSInput()
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root(),
+        "nonsensical input note value");
+
+        // Absurd total input value
+        invokeAPIFailure(js,
+        {
+            JSInput(witnesses[4], note4, sk),
+            JSInput(witnesses[5], note5, sk)
+        },
+        {
+            JSOutput(),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root(),
+        "nonsensical left hand size of joinsplit balance");
+
+        // Absurd output value
+        invokeAPIFailure(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(addr, 2100000000000001),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root(),
+        "nonsensical output value");
+
+        // Absurd total output value
+        invokeAPIFailure(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(addr, 1900000000000000),
+            JSOutput(addr, 1900000000000000)
+        },
+        0,
+        0,
+        tree.root(),
+        "nonsensical right hand side of joinsplit balance");
+
+        // Absurd total output value
+        invokeAPIFailure(js,
+        {
+            JSInput(),
+            JSInput()
+        },
+        {
+            JSOutput(addr, 1900000000000000),
+            JSOutput()
+        },
+        0,
+        0,
+        tree.root(),
+        "invalid joinsplit balance");
+    }
+
     test_full_api(js);
 
     js->saveProvingKey("./zcashTest.pk");

src/wallet/gtest/test_wallet.cpp

@@ -99,7 +99,7 @@ CWalletTx GetValidReceive(const libzcash::SpendingKey& sk, CAmount value, bool r
     // Prepare JoinSplits
     uint256 rt;
     JSDescription jsdesc {*params, mtx.joinSplitPubKey, rt,
-                          inputs, outputs, value, 0, false};
+                          inputs, outputs, 2*value, 0, false};
     mtx.vjoinsplit.push_back(jsdesc);
 
     // Empty output script.
@@ -147,20 +147,39 @@ CWalletTx GetValidSpend(const libzcash::SpendingKey& sk,
     // Fake tree for the unused witness
     ZCIncrementalMerkleTree tree;
 
+    libzcash::JSOutput dummyout;
+    libzcash::JSInput dummyin;
+
+    {
+        if (note.value > value) {
+            libzcash::SpendingKey dummykey = libzcash::SpendingKey::random();
+            libzcash::PaymentAddress dummyaddr = dummykey.address();
+            dummyout = libzcash::JSOutput(dummyaddr, note.value - value);
+        } else if (note.value < value) {
+            libzcash::SpendingKey dummykey = libzcash::SpendingKey::random();
+            libzcash::PaymentAddress dummyaddr = dummykey.address();
+            libzcash::Note dummynote(dummyaddr.a_pk, (value - note.value), uint256(), uint256());
+            tree.append(dummynote.cm());
+            dummyin = libzcash::JSInput(tree.witness(), dummynote, dummykey);
+        }
+    }
+
+    tree.append(note.cm());
+
     boost::array<libzcash::JSInput, 2> inputs = {
         libzcash::JSInput(tree.witness(), note, sk),
-        libzcash::JSInput() // dummy input
+        dummyin
     };
 
     boost::array<libzcash::JSOutput, 2> outputs = {
-        libzcash::JSOutput(), // dummy output
+        dummyout, // dummy output
         libzcash::JSOutput() // dummy output
     };
 
     boost::array<libzcash::Note, 2> output_notes;
 
     // Prepare JoinSplits
-    uint256 rt;
+    uint256 rt = tree.root();
     JSDescription jsdesc {*params, mtx.joinSplitPubKey, rt,
                           inputs, outputs, 0, value, false};
     mtx.vjoinsplit.push_back(jsdesc);

src/zcash/JoinSplit.cpp

@@ -16,6 +16,7 @@
 #include "libsnark/gadgetlib1/gadgets/merkle_tree/merkle_tree_check_read_gadget.hpp"
 
 #include "sync.h"
+#include "amount.h"
 
 using namespace libsnark;
 
@@ -181,8 +182,44 @@ public:
             throw std::runtime_error("JoinSplit proving key not loaded");
         }
 
-        // Compute nullifiers of inputs
+        if (vpub_old > MAX_MONEY) {
+            throw std::invalid_argument("nonsensical vpub_old value");
+        }
+
+        if (vpub_new > MAX_MONEY) {
+            throw std::invalid_argument("nonsensical vpub_new value");
+        }
+
+        uint64_t lhs_value = vpub_old;
+        uint64_t rhs_value = vpub_new;
+
         for (size_t i = 0; i < NumInputs; i++) {
+            // Sanity checks of input
+            {
+                // If note has nonzero value, its witness's root must be equal to the
+                // input.
+                if ((inputs[i].note.value != 0) && (inputs[i].witness.root() != rt)) {
+                    throw std::invalid_argument("joinsplit not anchored to the correct root");
+                }
+
+                // Ensure we have the key to this note.
+                if (inputs[i].note.a_pk != inputs[i].key.address().a_pk) {
+                    throw std::invalid_argument("input note not authorized to spend with given key");
+                }
+
+                // Balance must be sensical
+                if (inputs[i].note.value > MAX_MONEY) {
+                    throw std::invalid_argument("nonsensical input note value");
+                }
+
+                lhs_value += inputs[i].note.value;
+
+                if (lhs_value > MAX_MONEY) {
+                    throw std::invalid_argument("nonsensical left hand size of joinsplit balance");
+                }
+            }
+
+            // Compute nullifier of input
             out_nullifiers[i] = inputs[i].nullifier();
         }
 
@@ -197,12 +234,29 @@ public:
 
         // Compute notes for outputs
         for (size_t i = 0; i < NumOutputs; i++) {
+            // Sanity checks of output
+            {
+                if (outputs[i].value > MAX_MONEY) {
+                    throw std::invalid_argument("nonsensical output value");
+                }
+
+                rhs_value += outputs[i].value;
+
+                if (rhs_value > MAX_MONEY) {
+                    throw std::invalid_argument("nonsensical right hand side of joinsplit balance");
+                }
+            }
+
             // Sample r
             uint256 r = random_uint256();
 
             out_notes[i] = outputs[i].note(phi, r, i, h_sig);
         }
 
+        if (lhs_value != rhs_value) {
+            throw std::invalid_argument("invalid joinsplit balance");
+        }
+
         // Compute the output commitments
         for (size_t i = 0; i < NumOutputs; i++) {
             out_commitments[i] = out_notes[i].cm();
@@ -214,7 +268,6 @@ public:
             ZCNoteEncryption encryptor(h_sig);
 
             for (size_t i = 0; i < NumOutputs; i++) {
-
                 NotePlaintext pt(out_notes[i], outputs[i].memo);
 
                 out_ciphertexts[i] = pt.encrypt(encryptor, outputs[i].addr.pk_enc);
@@ -249,9 +302,9 @@ public:
             );
         }
 
-        if (!pb.is_satisfied()) {
+        // The constraint system must be satisfied or there is an unimplemented
-            throw std::invalid_argument("Constraint system not satisfied by inputs");
+        // or incorrect sanity check above. Or the constraint system is broken!
-        }
+        assert(pb.is_satisfied());
 
         // TODO: These are copies, which is not strictly necessary.
         std::vector<FieldT> primary_input = pb.primary_input();