From a3b5b703ba278b570a2f940f01113e520cbd5881 Mon Sep 17 00:00:00 2001
From: Wanseob Lim <email@wanseob.com>
Date: Tue, 6 Oct 2020 17:37:41 +0900
Subject: [PATCH] fix: template LessThan is not secure for large numbers;
 closes #116

---
 packages/circuits/lib/range_limit.circom      |  8 ++++
 packages/circuits/lib/zk_transaction.circom   | 27 +++++--------
 .../circuits/tester/range_limit.test.circom   |  3 ++
 packages/circuits/tests/range_limit.test.ts   | 40 +++++++++++++++++++
 4 files changed, 60 insertions(+), 18 deletions(-)
 create mode 100644 packages/circuits/lib/range_limit.circom
 create mode 100644 packages/circuits/tester/range_limit.test.circom
 create mode 100644 packages/circuits/tests/range_limit.test.ts

diff --git a/packages/circuits/lib/range_limit.circom b/packages/circuits/lib/range_limit.circom
new file mode 100644
index 000000000..4355e26eb
--- /dev/null
+++ b/packages/circuits/lib/range_limit.circom
@@ -0,0 +1,8 @@
+include "../node_modules/circomlib/circuits/bitify.circom";
+
+template RangeLimit(bitLength) {
+  signal input in;
+  // This automatically limits its max value to 2**bitLength - 1
+  component bits = Num2Bits(bitLength);
+  bits.in <== in;
+}
\ No newline at end of file
diff --git a/packages/circuits/lib/zk_transaction.circom b/packages/circuits/lib/zk_transaction.circom
index 385e3d1b0..b368df563 100644
--- a/packages/circuits/lib/zk_transaction.circom
+++ b/packages/circuits/lib/zk_transaction.circom
@@ -7,9 +7,8 @@ include "./asset_hash.circom";
 include "./nullifier.circom";
 include "./ownership_proof.circom";
 include "./spending_pubkey.circom";
-//include "./atomic_swap_mpc.circom";
 include "../node_modules/circomlib/circuits/eddsaposeidon.circom";
-include "../node_modules/circomlib/circuits/comparators.circom";
+include "../node_modules/circomlib/circuits/bitify.circom";
 
 /**
  * Note properties
@@ -211,31 +210,23 @@ template ZkTransaction(tree_depth, n_i, n_o) {
     var range_limit = (0 - 1) >> 8;
     component inflow_eth_range[n_i];
     for(var i = 0; i < n_i; i ++) {
-        inflow_eth_range[i] = LessThan(252);
-        inflow_eth_range[i].in[0] <== spending_note_eth[i];
-        inflow_eth_range[i].in[1] <== range_limit;
-        inflow_eth_range[i].out === 1;
+        inflow_eth_range[i] = RangeLimit(245);
+        inflow_eth_range[i].in <== spending_note_eth[i];
     }
     component inflow_erc20_range[n_i];
     for(var i = 0; i < n_i; i ++) {
-        inflow_erc20_range[i] = LessThan(252);
-        inflow_erc20_range[i].in[0] <== spending_note_erc20[i];
-        inflow_erc20_range[i].in[1] <== range_limit;
-        inflow_erc20_range[i].out === 1;
+        inflow_erc20_range[i] = RangeLimit(245);
+        inflow_erc20_range[i].in <== spending_note_erc20[i];
     }
     component outflow_eth_range[n_o];
     for(var i = 0; i < n_o; i ++) {
-        outflow_eth_range[i] = LessThan(252);
-        outflow_eth_range[i].in[0] <== new_note_eth[i];
-        outflow_eth_range[i].in[1] <== range_limit;
-        outflow_eth_range[i].out === 1;
+        outflow_eth_range[i] = RangeLimit(245);
+        outflow_eth_range[i].in <== new_note_eth[i];
     }
     component outflow_erc20_range[n_o];
     for(var i = 0; i < n_o; i ++) {
-        outflow_erc20_range[i] = LessThan(252);
-        outflow_erc20_range[i].in[0] <== new_note_erc20[i];
-        outflow_erc20_range[i].in[1] <== range_limit;
-        outflow_erc20_range[i].out === 1;
+        outflow_erc20_range[i] = RangeLimit(245);
+        outflow_erc20_range[i].in <== new_note_erc20[i];
     }
 
     /// Zero sum proof of ETH
diff --git a/packages/circuits/tester/range_limit.test.circom b/packages/circuits/tester/range_limit.test.circom
new file mode 100644
index 000000000..db4dddf3f
--- /dev/null
+++ b/packages/circuits/tester/range_limit.test.circom
@@ -0,0 +1,3 @@
+include "../lib/range_limit.circom";
+
+component main = RangeLimit(3);
\ No newline at end of file
diff --git a/packages/circuits/tests/range_limit.test.ts b/packages/circuits/tests/range_limit.test.ts
new file mode 100644
index 000000000..ef08539d4
--- /dev/null
+++ b/packages/circuits/tests/range_limit.test.ts
@@ -0,0 +1,40 @@
+/**
+ * @jest-environment node
+ */
+/* eslint-disable jest/require-tothrow-message */
+/* eslint-disable jest/no-expect-resolves */
+/* eslint-disable @typescript-eslint/camelcase */
+/* eslint-disable jest/no-hooks */
+
+import { genSNARK, SNARKResult } from '~utils/snark'
+import {
+  checkPhase1Setup,
+  compileCircuit,
+  getArtifactPaths,
+  phase2Setup,
+  prepareArtifactsDirectory,
+} from './helper'
+
+const fileName = 'range_limit.test.circom'
+const artifacts = getArtifactPaths(fileName)
+const { wasm, finalZkey, vk } = artifacts
+
+describe('multiplier.test.circom', () => {
+  beforeAll(() => {
+    checkPhase1Setup()
+    prepareArtifactsDirectory()
+  })
+  it('should compile circuits', () => {
+    compileCircuit(fileName)
+  })
+  it('should setup phase 2 for the circuit', () => {
+    phase2Setup(fileName)
+  })
+  it('should create SNARK proof', async () => {
+    const result: SNARKResult = await genSNARK({ in: 7 }, wasm, finalZkey, vk)
+    expect(result).toBeDefined()
+  })
+  it('should throw error with invalid inputs', async () => {
+    await expect(genSNARK({ in: 8 }, wasm, finalZkey, vk)).rejects.toThrow()
+  })
+})