From ba520701a8ca8788a35aa551190e89c98db35a30 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 16 Jul 2024 10:18:10 +0100 Subject: [PATCH 01/13] fix: ensure if statements that interact with secrets are included in the circuit --- .../circuit/zokrates/toCircuit.ts | 27 ++++++- src/transformers/visitors/toCircuitVisitor.ts | 78 ++++++++++++++++--- 2 files changed, 91 insertions(+), 14 deletions(-) diff --git a/src/codeGenerators/circuit/zokrates/toCircuit.ts b/src/codeGenerators/circuit/zokrates/toCircuit.ts index d040e3ef..cd9ebb94 100644 --- a/src/codeGenerators/circuit/zokrates/toCircuit.ts +++ b/src/codeGenerators/circuit/zokrates/toCircuit.ts @@ -264,12 +264,31 @@ function codeGenerator(node: any) { } }); for (let i =0; i { }; +//Finds a statement with the correct ID +const findStatementId = (statements: any, ID: number) => { + let expNode = statements.find((n:any) => n?.id === ID); + let index_expNode = statements.indexOf(expNode); + let location = {index: index_expNode, trueIndex: -1, falseIndex: -1}; + statements.forEach((st:any) => { + if (st.trueBody){ + if (!expNode) { + expNode = st.trueBody.find((n:any) => n?.id === ID); + location.index = statements.indexOf(st); + location.trueIndex = st.trueBody.indexOf(expNode); + } + } else if (st.falseBody){ + if (!expNode) { + expNode = st.falseBody.find((n:any) => n?.id === ID); + location.index = statements.indexOf(st); + location.falseIndex = st.falseBody.indexOf(expNode); + } + } + }); + return {expNode, location}; +}; + + // public variables that interact with the secret also need to be modified within the circuit. const publicVariables = (path: NodePath, state: any, IDnode: any) => { const {parent, node } = path; @@ -91,8 +115,8 @@ const publicVariables = (path: NodePath, state: any, IDnode: any) => { if (path.containerName !== 'indexExpression') { num_modifiers++; } - let expNode = statements.find((n:any) => n?.id === expressionId); - let index_expNode = fnDefNode.node._newASTPointer.body.statements.indexOf(expNode); + findStatementId(statements,expressionId); + let {expNode, location} = findStatementId(statements, expressionId); if (expNode && !expNode.isAccessed) { expNode.isAccessed = true; if((expNode.expression && expNode.expression.leftHandSide && expNode.expression.leftHandSide?.name === node.name) || @@ -109,8 +133,11 @@ const publicVariables = (path: NodePath, state: any, IDnode: any) => { interactsWithSecret: true, isVarDec: true, }); - if (index_expNode !== -1) { - fnDefNode.node._newASTPointer.body.statements.splice(index_expNode + 1, 0, newNode1); + newNode1.outsideIf = true; + if (location.index!== -1) { + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].trueBody.splice(location.trueIndex + 1, 0, newNode1); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].falseBody.splice(location.falseIndex + 1, 0, newNode1); } + else {fnDefNode.node._newASTPointer.body.statements.splice(location.index + 1, 0, newNode1);} } } } else{ @@ -124,8 +151,11 @@ const publicVariables = (path: NodePath, state: any, IDnode: any) => { expression: InnerNode, interactsWithSecret: true, }); - if (index_expNode !== -1) { - fnDefNode.node._newASTPointer.body.statements.splice(index_expNode + 1, 0, newNode1); + newNode1.outsideIf = true; + if (location.index!== -1) { + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].trueBody.splice(location.trueIndex + 1, 0, newNode1); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].falseBody.splice(location.falseIndex + 1, 0, newNode1); } + else {fnDefNode.node._newASTPointer.body.statements.splice(location.index + 1, 0, newNode1);} } if (`${modName}` !== `${node.name}_${num_modifiers}` && num_modifiers !==0){ const initInnerNode1 = buildNode('Assignment', { @@ -138,8 +168,11 @@ const publicVariables = (path: NodePath, state: any, IDnode: any) => { interactsWithSecret: true, isVarDec: true, }); - if (index_expNode !== -1) { - fnDefNode.node._newASTPointer.body.statements.splice(index_expNode + 2, 0, newNode2); + newNode2.outsideIf = true; + if (location.index!== -1) { + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].trueBody.splice(location.trueIndex + 2, 0, newNode2); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.statements[location.index].falseBody.splice(location.falseIndex + 2, 0, newNode2); } + else {fnDefNode.node._newASTPointer.body.statements.splice(location.index + 2, 0, newNode2);} } } } @@ -1193,9 +1226,34 @@ const visitor = { IfStatement: { enter(path: NodePath, state: any) { - const { node, parent } = path; + const { node, parent, scope } = path; let isIfStatementSecret; - if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || !node.condition?.containsPublic) + let interactsWithSecret = false; + function bodyInteractsWithSecrets(statements) { + statements.forEach((st) => { + if (st.nodeType === 'ExpressionStatement') { + if (st.expression.nodeType === 'UnaryOperation') { + const { operator, subExpression } = st.expression; + if ((operator === '++' || operator === '--') && subExpression.nodeType === 'Identifier') { + const referencedIndicator = scope.getReferencedIndicator(subExpression); + if (referencedIndicator?.interactsWithSecret) { + interactsWithSecret = true; + } + } + } else { + const referencedIndicator = scope.getReferencedIndicator(st.expression.leftHandSide); + if (referencedIndicator?.interactsWithSecret) { + interactsWithSecret = true; + } + } + } + }); + } + if (node.trueBody?.statements) bodyInteractsWithSecrets(node.trueBody?.statements); + if (node.falseBody?.statements) bodyInteractsWithSecrets(node.falseBody?.statements); + + + if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || !node.condition?.containsPublic) isIfStatementSecret = true; if(isIfStatementSecret) { if(node.trueBody.statements[0].expression.nodeType === 'FunctionCall') From 5d7da17c165e59b67a403f10192ff560ecfa20a5 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 16 Jul 2024 16:26:21 +0100 Subject: [PATCH 02/13] fix: move if statements that are public but interact with secret into orchestration --- .../orchestration/nodejs/toOrchestration.ts | 17 ++- src/transformers/visitors/toCircuitVisitor.ts | 1 - .../visitors/toOrchestrationVisitor.ts | 104 ++++++++++++++++-- src/types/orchestration-types.ts | 3 +- 4 files changed, 114 insertions(+), 11 deletions(-) diff --git a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts index f780fe9f..a6b7c2c3 100644 --- a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts +++ b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts @@ -1,4 +1,5 @@ /* eslint-disable import/no-cycle, no-param-reassign, consistent-return */ +import cloneDeep from 'lodash.clonedeep'; import {OrchestrationCodeBoilerPlate} from '../../../boilerplate/orchestration/javascript/raw/toOrchestration.js'; import fileGenerator from '../files/toOrchestration.js'; @@ -75,6 +76,9 @@ export default function codeGenerator(node: any, options: any = {}): any { return node.name; case 'VariableDeclarationStatement': { + if (node.outsideIf){ + return `${codeGenerator(node.initialValue)}`; + } if (!node.interactsWithSecret) return `\n// non-secret line would go here but has been filtered out`; if (node.initialValue?.nodeType === 'Assignment') { @@ -168,14 +172,23 @@ export default function codeGenerator(node: any, options: any = {}): any { return ` `; case 'IfStatement': { + let preIfStatements = node.trueBody.filter((node: any) => node.outsideIf).concat(node.falseBody.filter((node: any) => node.outsideIf)); + let newPreIfStatements = []; + preIfStatements.forEach((node: any) => { + newPreIfStatements.push(cloneDeep(node)); + newPreIfStatements[newPreIfStatements.length - 1].outsideIf = false; + }); + let preIfStatementsString = newPreIfStatements.flatMap(codeGenerator).join('\n'); if(node.falseBody.length) - return `if (${codeGenerator(node.condition)}) { + return `${preIfStatementsString} + if (${codeGenerator(node.condition)}) { ${node.trueBody.flatMap(codeGenerator).join('\n')} } else { ${node.falseBody.flatMap(codeGenerator).join('\n')} }` else - return `if (${codeGenerator(node.condition)}) { + return `${preIfStatementsString} + if (${codeGenerator(node.condition)}) { ${node.trueBody.flatMap(codeGenerator).join('\n')} }` } diff --git a/src/transformers/visitors/toCircuitVisitor.ts b/src/transformers/visitors/toCircuitVisitor.ts index 3f8af644..fc6749ea 100644 --- a/src/transformers/visitors/toCircuitVisitor.ts +++ b/src/transformers/visitors/toCircuitVisitor.ts @@ -115,7 +115,6 @@ const publicVariables = (path: NodePath, state: any, IDnode: any) => { if (path.containerName !== 'indexExpression') { num_modifiers++; } - findStatementId(statements,expressionId); let {expNode, location} = findStatementId(statements, expressionId); if (expNode && !expNode.isAccessed) { expNode.isAccessed = true; diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index aebf6926..1ef67d14 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -83,6 +83,32 @@ const collectIncrements = (stateVarIndicator: StateVariableIndicator | MappingKe return { incrementsArray, incrementsString }; }; +//Finds a statement with the correct ID +const findStatementId = (statements: any, ID: number) => { + let expNode = statements.find((n:any) => n?.id === ID); + let index_expNode = statements.indexOf(expNode); + let location = {index: index_expNode, trueIndex: -1, falseIndex: -1, ifNode: null}; + statements.forEach((st:any) => { + if (st.trueBody){ + if (!expNode) { + expNode = st.trueBody.find((n:any) => n?.id === ID); + location.index = statements.indexOf(st); + location.trueIndex = st.trueBody.indexOf(expNode); + location.ifNode = st; + } + } else if (st.falseBody){ + if (!expNode) { + expNode = st.falseBody.find((n:any) => n?.id === ID); + location.index = statements.indexOf(st); + location.falseIndex = st.falseBody.indexOf(expNode); + location.ifNode = st; + } + } + }); + return {expNode, location}; +}; + + // gathers public inputs we need to extract from the contract // i.e. public 'accessed' variables const addPublicInput = (path: NodePath, state: any, IDnode: any) => { @@ -163,7 +189,7 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { modifiedBeforePaths?.forEach((p: NodePath) => { const expressionId = p.getAncestorOfType('ExpressionStatement')?.node?.id; if (expressionId) { - let expNode = statements.find((n:any) => n?.id === expressionId); + let {expNode, location} = findStatementId(statements, expressionId); if (path.containerName !== 'indexExpression') { num_modifiers++; } @@ -172,8 +198,35 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { // we have to go back and mark any editing statements as interactsWithSecret so they show up expNode.interactsWithSecret = true; const moveExpNode = cloneDeep(expNode); - fnDefNode.node._newASTPointer.body.preStatements.push(moveExpNode); - delete statements[statements.indexOf(expNode)]; + let ifPreIndex = null; + if (location.ifNode) { + let {expNode: newIfnode, location: locIf } = findStatementId(fnDefNode.node._newASTPointer.body.preStatements, location.ifNode.id); + ifPreIndex = locIf.index; + if (locIf.index !== -1 && location.trueIndex !== -1) fnDefNode.node._newASTPointer.body.preStatements[locIf.index].trueBody.push(moveExpNode); + else if (locIf.index !== -1 && location.falseIndex !== -1) fnDefNode.node._newASTPointer.body.preStatements[locIf.index].falseBody.push(moveExpNode); + else if (!locIf.index || locIf.index === -1 ){ + let newIfNode = cloneDeep(location.ifNode); + newIfNode.trueBody = []; + newIfNode.falseBody = []; + if (location.trueIndex !== -1) newIfNode.trueBody.push(moveExpNode); + if (location.falseIndex !== -1) newIfNode.falseBody.push(moveExpNode); + fnDefNode.node._newASTPointer.body.preStatements.push(newIfNode); + ifPreIndex = fnDefNode.node._newASTPointer.body.preStatements.length -1; + } + } else{ + fnDefNode.node._newASTPointer.body.preStatements.push(moveExpNode); + } + if (location.index!== -1) { + if (location.trueIndex !== -1){ delete statements[location.index].trueBody[location.trueIndex]; } + else if (location.falseIndex !== -1){ delete statements[location.index].falseBody[location.falseIndex]; } + else { + delete statements[location.index]; + } + } + if ((statements[location.index].trueBody && statements[location.index].trueBody.every(element => element === null || element === undefined)) && (statements[location.index].falseBody && statements[location.index].falseBody.every(element => element === null || element === undefined))) { + delete statements[location.index]; + } + if( (expNode.expression && expNode.expression.leftHandSide && expNode.expression.leftHandSide?.name === node.name) || (expNode.initialValue && expNode.initialValue.leftHandSide && expNode.initialValue.leftHandSide?.name === node.name) @@ -196,7 +249,11 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { interactsWithSecret: true, isModifiedDeclaration: true, }); - fnDefNode.node._newASTPointer.body.preStatements.push(newNode1); + if (location.ifNode) newNode1.outsideIf = true; + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].trueBody.push(newNode1); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].falseBody.push(newNode1); } + else {fnDefNode.node._newASTPointer.body.preStatements.push(newNode1);} + } } else{ let name_new = expNode.expression?.initialValue?.leftHandSide?.name || expNode.initialValue?.leftHandSide.name || expNode.expression?.leftHandSide.name; @@ -209,6 +266,9 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { expression: InnerNode, interactsWithSecret: true, }); + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].trueBody.push(newNode1); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].falseBody.push(newNode1); } + else {fnDefNode.node._newASTPointer.body.preStatements.push(newNode1);} fnDefNode.node._newASTPointer.body.preStatements.push(newNode1); if (`${name_new}` !== `${node.name}_${num_modifiers}` && num_modifiers !==0){ const decInnerNode1 = buildNode('VariableDeclaration', { @@ -228,7 +288,10 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { interactsWithSecret: true, isModifiedDeclaration: true, }); - fnDefNode.node._newASTPointer.body.preStatements.push(newNode2); + if (location.ifNode) newNode2.outsideIf = true; + if (location.trueIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].trueBody.push(newNode2); } + else if (location.falseIndex !== -1){ fnDefNode.node._newASTPointer.body.preStatements[ifPreIndex].falseBody.push(newNode2); } + else {fnDefNode.node._newASTPointer.body.preStatements.push(newNode2);} } } } @@ -1746,13 +1809,40 @@ const visitor = { IfStatement: { enter(path: NodePath , state: any) { - const { node, parent, } = path; - if(!node.containsSecret) { + const { node, parent, scope } = path; + let isIfStatementSecret; + let interactsWithSecret = false; + function bodyInteractsWithSecrets(statements) { + statements.forEach((st) => { + if (st.nodeType === 'ExpressionStatement') { + if (st.expression.nodeType === 'UnaryOperation') { + const { operator, subExpression } = st.expression; + if ((operator === '++' || operator === '--') && subExpression.nodeType === 'Identifier') { + const referencedIndicator = scope.getReferencedIndicator(subExpression); + if (referencedIndicator?.interactsWithSecret) { + interactsWithSecret = true; + } + } + } else { + const referencedIndicator = scope.getReferencedIndicator(st.expression.leftHandSide); + if (referencedIndicator?.interactsWithSecret) { + interactsWithSecret = true; + } + } + } + }); + } + if (node.trueBody?.statements) bodyInteractsWithSecrets(node.trueBody?.statements); + if (node.falseBody?.statements) bodyInteractsWithSecrets(node.falseBody?.statements); + if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || !node.condition?.containsPublic) + isIfStatementSecret = true; + if(!isIfStatementSecret) { state.skipSubNodes = true; return; } const newNode = buildNode(node.nodeType); newNode.interactsWithSecret = true; + newNode.id = node.id; node._newASTPointer = newNode; parent._newASTPointer.push(newNode); }, diff --git a/src/types/orchestration-types.ts b/src/types/orchestration-types.ts index 88b4d72a..35c44ae8 100644 --- a/src/types/orchestration-types.ts +++ b/src/types/orchestration-types.ts @@ -183,8 +183,9 @@ export default function buildNode(nodeType: string, fields: any = {}): any { } } case 'IfStatement': { - const { condition = {} , trueBody= [] , falseBody= [] } = fields; + const { condition = {} , trueBody= [] , falseBody= [], oldASTId, } = fields; return { + id: oldASTId, nodeType, condition, trueBody, From 38173b1592fe3caceffe008c9d77cf3ffb339c87 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 16 Jul 2024 16:27:29 +0100 Subject: [PATCH 03/13] chore: add test contract --- test/contracts/If-Statement6.zol | 51 ++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test/contracts/If-Statement6.zol diff --git a/test/contracts/If-Statement6.zol b/test/contracts/If-Statement6.zol new file mode 100644 index 00000000..8f13af94 --- /dev/null +++ b/test/contracts/If-Statement6.zol @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: CC0 + +pragma solidity ^0.8.0; + +contract MyContract { + + secret uint256 private a; + uint256 private b; + secret address private admin; + address private pubAdmin; + + + constructor() { + admin = msg.sender; + pubAdmin = msg.sender; + } + + function add(uint256 value) public { + if(b < 5) { + b += 1; + } + a += b + value; + } + + function add1(uint256 value) public { + if(b < 5) { + b++; + } + a += b + value; + } + + function add2(uint256 value) public { + if(b < 5) { + b += 1; + b++; + } + a += b + value; + } + + function add3(uint256 value) public { + if(a < b) { + revert("a less than 5"); + } + a+= value; + b += 1; + } + + + + +} From 09a1c128421109857a886212d434a689fe1c13dd Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 16 Jul 2024 16:38:34 +0100 Subject: [PATCH 04/13] fix: zappify error in Arrays --- src/transformers/visitors/toOrchestrationVisitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index 1ef67d14..32ff6d96 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -223,7 +223,7 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { delete statements[location.index]; } } - if ((statements[location.index].trueBody && statements[location.index].trueBody.every(element => element === null || element === undefined)) && (statements[location.index].falseBody && statements[location.index].falseBody.every(element => element === null || element === undefined))) { + if ((statements[location.index]?.trueBody && statements[location.index].trueBody.every(element => element === null || element === undefined)) && (statements[location.index]?.falseBody && statements[location.index].falseBody.every(element => element === null || element === undefined))) { delete statements[location.index]; } From b419bac8df534a103c24afa26485c4319d28f518 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 16 Jul 2024 17:15:24 +0100 Subject: [PATCH 05/13] chore: add comment to if statement in pre statements --- src/codeGenerators/orchestration/nodejs/toOrchestration.ts | 7 +++++-- src/transformers/visitors/toOrchestrationVisitor.ts | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts index a6b7c2c3..2adce15e 100644 --- a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts +++ b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts @@ -172,6 +172,7 @@ export default function codeGenerator(node: any, options: any = {}): any { return ` `; case 'IfStatement': { + let comment = (node.inPreStatements) ? "// the public statements of this if statement have been moved to pre-statements here, any statements that involve secret variables appear later" : ''; let preIfStatements = node.trueBody.filter((node: any) => node.outsideIf).concat(node.falseBody.filter((node: any) => node.outsideIf)); let newPreIfStatements = []; preIfStatements.forEach((node: any) => { @@ -180,14 +181,16 @@ export default function codeGenerator(node: any, options: any = {}): any { }); let preIfStatementsString = newPreIfStatements.flatMap(codeGenerator).join('\n'); if(node.falseBody.length) - return `${preIfStatementsString} + return `${comment} + ${preIfStatementsString} if (${codeGenerator(node.condition)}) { ${node.trueBody.flatMap(codeGenerator).join('\n')} } else { ${node.falseBody.flatMap(codeGenerator).join('\n')} }` else - return `${preIfStatementsString} + return `${comment} + ${preIfStatementsString} if (${codeGenerator(node.condition)}) { ${node.trueBody.flatMap(codeGenerator).join('\n')} }` diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index 32ff6d96..0db8570f 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -206,6 +206,7 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { else if (locIf.index !== -1 && location.falseIndex !== -1) fnDefNode.node._newASTPointer.body.preStatements[locIf.index].falseBody.push(moveExpNode); else if (!locIf.index || locIf.index === -1 ){ let newIfNode = cloneDeep(location.ifNode); + newIfNode.inPreStatements = true; newIfNode.trueBody = []; newIfNode.falseBody = []; if (location.trueIndex !== -1) newIfNode.trueBody.push(moveExpNode); From 993f73e20dd5703b571f9d4d55bc1378a8691430 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Wed, 17 Jul 2024 14:29:45 +0100 Subject: [PATCH 06/13] fix: revert in if statement that contains public variable in condition wasn't appearing in the circuit --- src/transformers/visitors/toCircuitVisitor.ts | 2 +- .../visitors/toOrchestrationVisitor.ts | 2 +- test/contracts/If-Statement6.zol | 25 +++++++++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/transformers/visitors/toCircuitVisitor.ts b/src/transformers/visitors/toCircuitVisitor.ts index fc6749ea..d23ab6b2 100644 --- a/src/transformers/visitors/toCircuitVisitor.ts +++ b/src/transformers/visitors/toCircuitVisitor.ts @@ -1252,7 +1252,7 @@ const visitor = { if (node.falseBody?.statements) bodyInteractsWithSecrets(node.falseBody?.statements); - if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || !node.condition?.containsPublic) + if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || node.condition?.containsSecret) isIfStatementSecret = true; if(isIfStatementSecret) { if(node.trueBody.statements[0].expression.nodeType === 'FunctionCall') diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index 0db8570f..a47d154f 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -1835,7 +1835,7 @@ const visitor = { } if (node.trueBody?.statements) bodyInteractsWithSecrets(node.trueBody?.statements); if (node.falseBody?.statements) bodyInteractsWithSecrets(node.falseBody?.statements); - if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || !node.condition?.containsPublic) + if(node.falseBody?.containsSecret || node.trueBody?.containsSecret || interactsWithSecret || node.condition?.containsSecret) isIfStatementSecret = true; if(!isIfStatementSecret) { state.skipSubNodes = true; diff --git a/test/contracts/If-Statement6.zol b/test/contracts/If-Statement6.zol index 8f13af94..c2da00d3 100644 --- a/test/contracts/If-Statement6.zol +++ b/test/contracts/If-Statement6.zol @@ -5,7 +5,8 @@ pragma solidity ^0.8.0; contract MyContract { secret uint256 private a; - uint256 private b; + uint256 public b; + uint256 public c; secret address private admin; address private pubAdmin; @@ -18,31 +19,45 @@ contract MyContract { function add(uint256 value) public { if(b < 5) { b += 1; + b++; + } else{ + b++; } a += b + value; } + function add1(uint256 value) public { if(b < 5) { + b += 1; b++; } - a += b + value; + a += value; } + function add2(uint256 value) public { if(b < 5) { b += 1; - b++; + c += 1; } a += b + value; } function add3(uint256 value) public { + if(b < 5) { + b += 1; + b++; + } + a += b + value; + } + + function add4(uint256 value) public { if(a < b) { - revert("a less than 5"); + revert("a less than b"); } a+= value; - b += 1; + b += 20; } From 21d3321fb3aa4a421634afa59236e587934299a4 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Wed, 17 Jul 2024 16:50:18 +0100 Subject: [PATCH 07/13] fix: issue in orchestration for when an if statements that includes else has statements that are non-secret but later interact with secret variables --- src/transformers/visitors/toCircuitVisitor.ts | 3 +- .../visitors/toOrchestrationVisitor.ts | 35 ++++++++++++++++++- test/contracts/If-Statement6.zol | 15 ++++---- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/transformers/visitors/toCircuitVisitor.ts b/src/transformers/visitors/toCircuitVisitor.ts index d23ab6b2..a79cbfe8 100644 --- a/src/transformers/visitors/toCircuitVisitor.ts +++ b/src/transformers/visitors/toCircuitVisitor.ts @@ -69,7 +69,8 @@ const findStatementId = (statements: any, ID: number) => { location.index = statements.indexOf(st); location.trueIndex = st.trueBody.indexOf(expNode); } - } else if (st.falseBody){ + } + if (st.falseBody){ if (!expNode) { expNode = st.falseBody.find((n:any) => n?.id === ID); location.index = statements.indexOf(st); diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index a47d154f..06f2a770 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -96,7 +96,8 @@ const findStatementId = (statements: any, ID: number) => { location.trueIndex = st.trueBody.indexOf(expNode); location.ifNode = st; } - } else if (st.falseBody){ + } + if (st.falseBody){ if (!expNode) { expNode = st.falseBody.find((n:any) => n?.id === ID); location.index = statements.indexOf(st); @@ -569,6 +570,8 @@ const visitor = { node._newASTPointer.msgSenderParam ??= state.msgSenderParam; node._newASTPointer.msgValueParam ??= state.msgValueParam; + + if(node.containsPublic && !scope.modifiesSecretState()){ interface PublicParam { name: string; @@ -912,6 +915,36 @@ const visitor = { const newFunctionDefinitionNode = node._newASTPointer; + // In If Statements we might have non-secret statements editing variables that later interact with a secret variable. + // We therefore have statements of the form b_6 = b so that b_6 can be used later. + // We need to add the final such statement from the false body to after the if statement so that e.g. b_6 can be used even if the true body is executed. + let nodesToAdd = []; + newFunctionDefinitionNode.body.preStatements.forEach((n: any, index: number) => { + if (n.nodeType === 'IfStatement'){ + let finalName; + let originalName; + n.falseBody.forEach((falseNode: any) => { + if (falseNode.outsideIf){ + finalName = falseNode.initialValue.leftHandSide.name; + originalName = falseNode.initialValue.rightHandSide.name; + } + }); + const InnerNode = buildNode('Assignment', { + leftHandSide: buildNode('Identifier', { name: finalName, subType: 'generalNumber' }), + operator: '=', + rightHandSide: buildNode('Identifier', { name: originalName, subType: 'generalNumber' }) + }); + const finalIfNode = buildNode('ExpressionStatement', { + expression: InnerNode, + interactsWithSecret: true, + }); + nodesToAdd.push({node: finalIfNode, index: index+1}); + } + }); + for (let i = nodesToAdd.length - 1; i >= 0; i--) { + newFunctionDefinitionNode.body.preStatements.splice(nodesToAdd[i].index, 0, nodesToAdd[i].node); + } + // this adds other values we need in the circuit for (const param of node._newASTPointer.parameters.parameters) { if (param.isPrivate || param.isSecret || param.interactsWithSecret) { diff --git a/test/contracts/If-Statement6.zol b/test/contracts/If-Statement6.zol index c2da00d3..66ba0eb8 100644 --- a/test/contracts/If-Statement6.zol +++ b/test/contracts/If-Statement6.zol @@ -21,6 +21,7 @@ contract MyContract { b += 1; b++; } else{ + b = b+2; b++; } a += b + value; @@ -36,13 +37,13 @@ contract MyContract { } - function add2(uint256 value) public { - if(b < 5) { - b += 1; - c += 1; - } - a += b + value; - } + function add2(uint256 value) public { + if(b < 5) { + b += 1; + c += 1; + } + a += b + value; + } function add3(uint256 value) public { if(b < 5) { From 979e0999beecd60fa257ff83a0dc1464ab1c1a82 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Wed, 17 Jul 2024 17:38:24 +0100 Subject: [PATCH 08/13] fix: errors in circuit for else statements --- src/codeGenerators/circuit/zokrates/toCircuit.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/codeGenerators/circuit/zokrates/toCircuit.ts b/src/codeGenerators/circuit/zokrates/toCircuit.ts index cd9ebb94..d37c971a 100644 --- a/src/codeGenerators/circuit/zokrates/toCircuit.ts +++ b/src/codeGenerators/circuit/zokrates/toCircuit.ts @@ -279,11 +279,11 @@ function codeGenerator(node: any) { } for (let j =0; j Date: Wed, 17 Jul 2024 20:46:18 +0100 Subject: [PATCH 09/13] fix: error in orchestration visitor due to undefined statement added --- .../visitors/toOrchestrationVisitor.ts | 22 ++++++++++--------- test/contracts/If-Statement6.zol | 16 -------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index 06f2a770..8ce24bf5 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -929,16 +929,18 @@ const visitor = { originalName = falseNode.initialValue.rightHandSide.name; } }); - const InnerNode = buildNode('Assignment', { - leftHandSide: buildNode('Identifier', { name: finalName, subType: 'generalNumber' }), - operator: '=', - rightHandSide: buildNode('Identifier', { name: originalName, subType: 'generalNumber' }) - }); - const finalIfNode = buildNode('ExpressionStatement', { - expression: InnerNode, - interactsWithSecret: true, - }); - nodesToAdd.push({node: finalIfNode, index: index+1}); + if (finalName && originalName){ + const InnerNode = buildNode('Assignment', { + leftHandSide: buildNode('Identifier', { name: finalName, subType: 'generalNumber' }), + operator: '=', + rightHandSide: buildNode('Identifier', { name: originalName, subType: 'generalNumber' }) + }); + const finalIfNode = buildNode('ExpressionStatement', { + expression: InnerNode, + interactsWithSecret: true, + }); + nodesToAdd.push({node: finalIfNode, index: index+1}); + } } }); for (let i = nodesToAdd.length - 1; i >= 0; i--) { diff --git a/test/contracts/If-Statement6.zol b/test/contracts/If-Statement6.zol index 66ba0eb8..22e1051f 100644 --- a/test/contracts/If-Statement6.zol +++ b/test/contracts/If-Statement6.zol @@ -45,22 +45,6 @@ contract MyContract { a += b + value; } - function add3(uint256 value) public { - if(b < 5) { - b += 1; - b++; - } - a += b + value; - } - - function add4(uint256 value) public { - if(a < b) { - revert("a less than b"); - } - a+= value; - b += 20; - } - From 8bec0811e4056a037a669e2e3ebace996691d926 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Thu, 18 Jul 2024 10:27:43 +0100 Subject: [PATCH 10/13] chore: add comments --- src/codeGenerators/orchestration/nodejs/toOrchestration.ts | 6 +++++- src/transformers/visitors/toOrchestrationVisitor.ts | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts index 2adce15e..3caef410 100644 --- a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts +++ b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts @@ -76,6 +76,7 @@ export default function codeGenerator(node: any, options: any = {}): any { return node.name; case 'VariableDeclarationStatement': { + // If the statement is inside an if statement but declared outside. if (node.outsideIf){ return `${codeGenerator(node.initialValue)}`; } @@ -172,7 +173,9 @@ export default function codeGenerator(node: any, options: any = {}): any { return ` `; case 'IfStatement': { - let comment = (node.inPreStatements) ? "// the public statements of this if statement have been moved to pre-statements here, any statements that involve secret variables appear later" : ''; + let comment = (node.inPreStatements) ? "// some public statements of this if statement have been moved to pre-statements here, any other statements appear later" : ''; + + // We need to declare some variables before the if statement begins (because they are used outside the if statement). let preIfStatements = node.trueBody.filter((node: any) => node.outsideIf).concat(node.falseBody.filter((node: any) => node.outsideIf)); let newPreIfStatements = []; preIfStatements.forEach((node: any) => { @@ -180,6 +183,7 @@ export default function codeGenerator(node: any, options: any = {}): any { newPreIfStatements[newPreIfStatements.length - 1].outsideIf = false; }); let preIfStatementsString = newPreIfStatements.flatMap(codeGenerator).join('\n'); + if(node.falseBody.length) return `${comment} ${preIfStatementsString} diff --git a/src/transformers/visitors/toOrchestrationVisitor.ts b/src/transformers/visitors/toOrchestrationVisitor.ts index 8ce24bf5..afbb0b2d 100644 --- a/src/transformers/visitors/toOrchestrationVisitor.ts +++ b/src/transformers/visitors/toOrchestrationVisitor.ts @@ -199,9 +199,11 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { // we have to go back and mark any editing statements as interactsWithSecret so they show up expNode.interactsWithSecret = true; const moveExpNode = cloneDeep(expNode); + // We now move the statement in expNode to preStatements. + //If the statement is within an if statement we need to find the correct if statement in preStatements or create a new one. let ifPreIndex = null; if (location.ifNode) { - let {expNode: newIfnode, location: locIf } = findStatementId(fnDefNode.node._newASTPointer.body.preStatements, location.ifNode.id); + let {location: locIf } = findStatementId(fnDefNode.node._newASTPointer.body.preStatements, location.ifNode.id); ifPreIndex = locIf.index; if (locIf.index !== -1 && location.trueIndex !== -1) fnDefNode.node._newASTPointer.body.preStatements[locIf.index].trueBody.push(moveExpNode); else if (locIf.index !== -1 && location.falseIndex !== -1) fnDefNode.node._newASTPointer.body.preStatements[locIf.index].falseBody.push(moveExpNode); @@ -218,6 +220,7 @@ const addPublicInput = (path: NodePath, state: any, IDnode: any) => { } else{ fnDefNode.node._newASTPointer.body.preStatements.push(moveExpNode); } + // We now remove the statement from the statements array. if (location.index!== -1) { if (location.trueIndex !== -1){ delete statements[location.index].trueBody[location.trueIndex]; } else if (location.falseIndex !== -1){ delete statements[location.index].falseBody[location.falseIndex]; } From 257938fcb43255ece28b7ca1cf4802679698379e Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Thu, 18 Jul 2024 14:39:59 +0100 Subject: [PATCH 11/13] add test contracts --- test/contracts/If-Statement7.zol | 40 ++++++++++++++++++++++++++++++++ test/error-checks/_Variable.zol | 15 ++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 test/contracts/If-Statement7.zol create mode 100644 test/error-checks/_Variable.zol diff --git a/test/contracts/If-Statement7.zol b/test/contracts/If-Statement7.zol new file mode 100644 index 00000000..eaecc425 --- /dev/null +++ b/test/contracts/If-Statement7.zol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: CC0 + +pragma solidity ^0.8.0; + +contract MyContract { + + secret uint256 private a; + uint256 public b; + uint256 public c; + secret address private admin; + address private pubAdmin; + + + constructor() { + admin = msg.sender; + pubAdmin = msg.sender; + } + + + + function add3(uint256 value) public { + if(b < 5) { + b += 1; + b++; + } + a += b + value; + } + + function add4(uint256 value) public { + if(a < b) { + revert("a less than b"); + } + a+= value; + b += 20; + } + + + + +} diff --git a/test/error-checks/_Variable.zol b/test/error-checks/_Variable.zol new file mode 100644 index 00000000..0c610a01 --- /dev/null +++ b/test/error-checks/_Variable.zol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: CC0 + +pragma solidity ^0.8.0; + +contract Assign { + + secret uint256 private a; + function add(secret uint256 _value) public { + unknown a += _value; + } + + function remove(secret uint256 value) public { + a -= value; + } +} From 91605664a989cc1afa3bf6721799b85d9c071143 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Thu, 18 Jul 2024 17:28:14 +0100 Subject: [PATCH 12/13] fix: generalise in initialization expression for for statement --- src/codeGenerators/orchestration/nodejs/toOrchestration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts index 3caef410..ed40f283 100644 --- a/src/codeGenerators/orchestration/nodejs/toOrchestration.ts +++ b/src/codeGenerators/orchestration/nodejs/toOrchestration.ts @@ -108,7 +108,7 @@ export default function codeGenerator(node: any, options: any = {}): any { if (!node.initialValue.operator) { if (!node.initialValue.nodeType) return `\nlet ${codeGenerator(node.declarations[0])};` // local var dec - if (node.initialValue.nodeType === 'Literal' && !node.isInitializationExpression) return `\nlet ${codeGenerator(node.declarations[0])} = generalise(${codeGenerator(node.initialValue)});`; + if (node.initialValue.nodeType === 'Literal' && node.isInitializationExpression) return `\nlet ${codeGenerator(node.declarations[0])} = ${codeGenerator(node.initialValue)};`; return `\nlet ${codeGenerator(node.declarations[0])} = generalise(${codeGenerator(node.initialValue)});`; } return `\nlet ${codeGenerator(node.initialValue)};`; From ea819dcea32efdbc9d161f455745505294c05af1 Mon Sep 17 00:00:00 2001 From: Lydia Garms Date: Tue, 23 Jul 2024 15:59:17 +0100 Subject: [PATCH 13/13] chore: add boolean to type --- src/transformers/visitors/toCircuitVisitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/visitors/toCircuitVisitor.ts b/src/transformers/visitors/toCircuitVisitor.ts index a79cbfe8..34a95eda 100644 --- a/src/transformers/visitors/toCircuitVisitor.ts +++ b/src/transformers/visitors/toCircuitVisitor.ts @@ -1227,7 +1227,7 @@ const visitor = { IfStatement: { enter(path: NodePath, state: any) { const { node, parent, scope } = path; - let isIfStatementSecret; + let isIfStatementSecret: boolean; let interactsWithSecret = false; function bodyInteractsWithSecrets(statements) { statements.forEach((st) => {