Merge pull request #462 from EYBlockchain/lyd/burnError

This PR fixes several errors:

An error when it is detected that a variable is set to zero i.e. burned. Previously, it was checking that the owner binding was set to zero, which wasn't always giving the correct result.
An error for a function where there are no new nullifiers but we do use checkNullifiers, i.e. we access secret variables and so need to make sure we are using the most up to date commitment by a nullifier doesn't exist in the nullifier list. The length of the inputs during proof verification in the Shield Contract were wrong in this case.
In orchestration, we do a check to make sure the new leaves have been successfully added on chain at the end. However, if there are no new commitments, we should not include this check or an error will be returned incorrectly. Previously things were being marked as not burned only, i.e. the check included, even when there were no new commitments.

Author: lydiagarms

src/boilerplate/contract/solidity/raw/ContractBoilerplateGenerator.ts

@@ -218,7 +218,18 @@ class ContractBoilerplateGenerator {
             'customInputs.length',
             ...(newNullifiers ? ['newNullifiers.length'] : []),
             ...(checkNullifiers ? ['checkNullifiers.length']: []),
-            ...(commitmentRoot ? ['(newNullifiers.length > 0 ? 1 : 0)'] : []), 
+            ...(() => {
+              if (commitmentRoot){
+                if (checkNullifiers && newNullifiers) {
+                  return ['((newNullifiers.length + checkNullifiers.length) > 0 ? 1 : 0)'];
+                }
+                if (checkNullifiers) {
+                  return ['((checkNullifiers.length) > 0 ? 1 : 0)'];
+                }
+                return ['(newNullifiers.length > 0 ? 1 : 0)'];
+              }
+              return [];
+            })(),
             ...(newCommitments ? ['newCommitments.length'] : []),
             ...(encryptionRequired ? ['encInputsLen'] : []),
           ].join(' + ')});`,

src/boilerplate/orchestration/javascript/raw/toOrchestration.ts

@@ -1052,7 +1052,7 @@ export const OrchestrationCodeBoilerPlate: any = (node: any) => {
         \n if (!tx) {
           throw new Error( 'Tx failed - the commitment was not accepted on-chain, or the contract is not deployed.');
         } \n`;
-      if (!node.newCommitmentsRequired){
+      if (!node.newCommitmentsRequired) {
         checkLeaves = '';
       }
       return {

src/traverse/Binding.ts

@@ -601,17 +601,7 @@ export class VariableBinding extends Binding {
         }
       }
     }
-    // mapping[key] = msg.sender is owned by msg.sender => look for mapping[key] = 0
-    // OR owner is some value (admin = address) => look for admin = 0
-    if (
-      ownerNode.name === 'msg' &&
-      ownerNode.mappingOwnershipType === 'value'
-    ) {
-      // the owner is represented by the mapping value - we look through the modifyingPaths for 0
-      this.searchModifyingPathsForZero();
-    } else if (ownerBinding && ownerBinding instanceof VariableBinding) {
-      ownerBinding.searchModifyingPathsForZero();
-    }
+    this.searchModifyingPathsForZero();
     if (this.reinitialisable && !this.isBurned)
       throw new SyntaxUsageError(
         `The state ${this.name} has been marked as reinitialisable but we can't find anywhere to burn a commitment ready for reinitialisation.`,

src/traverse/Indicator.ts

@@ -142,10 +142,11 @@ export class FunctionDefinitionIndicator extends ContractDefinitionIndicator {
     let burnedOnly = true;
     for (const [, stateVarIndicator] of Object.entries(this)) {
       if (!(stateVarIndicator instanceof StateVariableIndicator)) continue; // eslint-disable-line no-continue, no-use-before-define
-      // if we have a indicator which is NOT burned, then we do need new commitments
+      // if we have a indicator which is NOT burned and requires new commitments, then we do need new commitments
       if (
         stateVarIndicator.isSecret &&
-        (!stateVarIndicator.isBurned || stateVarIndicator.newCommitmentsRequired)
+        !stateVarIndicator.isBurned &&
+        stateVarIndicator.newCommitmentsRequired
       ) {
         burnedOnly = false;
         break;