From ecc5a94dc4cbf5a2c80d4abf7848b1bd3759e78f Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Thu, 18 Jul 2024 09:53:02 +0100 Subject: [PATCH 01/10] chore: fixed += and a = a + --- .../zokrates/nodes/BoilerplateGenerator.ts | 2 +- .../visitors/checks/incrementedVisitor.ts | 18 +++++++++++++----- ...orchestrationInternalFunctionCallVisitor.ts | 2 +- test/contracts/Arrays1.zol | 3 +++ test/contracts/Assign.zol | 9 +++++---- test/contracts/BucketsOfBalls.zol | 4 +++- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/boilerplate/circuit/zokrates/nodes/BoilerplateGenerator.ts b/src/boilerplate/circuit/zokrates/nodes/BoilerplateGenerator.ts index d44994037..e45f39d13 100644 --- a/src/boilerplate/circuit/zokrates/nodes/BoilerplateGenerator.ts +++ b/src/boilerplate/circuit/zokrates/nodes/BoilerplateGenerator.ts @@ -85,6 +85,7 @@ const collectIncrements = (bpg: BoilerplateGenerator) => { incrementsString += ` + ${dec.name}`; } } + return { incrementsArray, incrementsString }; }; @@ -353,7 +354,6 @@ class BoilerplateGenerator { } else { throw new Error('This should be unreachable.'); } - return index; } diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index 4b10c8cff..84d1543c7 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -56,6 +56,7 @@ const markParentIncrementation = ( parent.node.expression.incrementedDeclaration = parent.incrementedDeclaration; state.unmarkedIncrementation = false; state.incrementedIdentifier = incrementedIdentifier; + if (increments?.operands) increments = collectIncrements(increments, incrementedIdentifier); increments?.forEach((inc: any) => { @@ -106,6 +107,8 @@ const binOpToIncrements = (path: NodePath, state: any) => { ); const lhsNode = parentExpressionStatement?.node.expression?.leftHandSide; const assignmentOp = parentExpressionStatement?.node.expression?.operator; + console.log(assignmentOp); + console.log(path.node); const { operator, leftExpression, rightExpression } = path.node; const operands = [leftExpression, rightExpression]; const precedingOperator = ['+', operator]; @@ -153,6 +156,7 @@ export default { ExpressionStatement: { enter(path: NodePath, state: any) { // starts here - if the path hasn't yet been marked as incremented, we find out if it is + if (path.isIncremented === undefined) { state.unmarkedIncrementation = true; state.increments = []; @@ -175,7 +179,8 @@ export default { const { isIncremented, isDecremented } = path; expressionNode.isIncremented = isIncremented; expressionNode.isDecremented = isDecremented; - + // console.log('expression Node:', expressionNode) + // console.log(state); // print if in debug mode if (logger.level === 'debug') backtrace.getSourceCode(node.src); logger.debug(`statement is incremented? ${isIncremented}`); @@ -238,7 +243,6 @@ export default { const { operator, leftHandSide, rightHandSide } = node; const lhsSecret = !!scope.getReferencedBinding(leftHandSide)?.isSecret; - if (['bool', 'address'].includes(leftHandSide.typeDescriptions.typeString)) { markParentIncrementation(path, state, false, false, leftHandSide); const lhsBinding = scope.getReferencedBinding(leftHandSide) @@ -394,7 +398,6 @@ export default { discoveredLHS += 1; isIncremented = { incremented: true, decremented: true }; } - // a = something - a if ( nameMatch && @@ -416,17 +419,22 @@ export default { ) { // a = a + b - c - d counts as an incrementation since the 1st operator is a plus // the mixed operators warning will have been given + // length of operator will be more than 2 in case of mixed operators if ( + precedingOperator.length > 2 && precedingOperator.includes('+') && precedingOperator.includes('-') && precedingOperator[0] === '+' - ) + ){ isIncremented.decremented = false; + + } + markParentIncrementation( path, state, isIncremented.incremented, - false, + isIncremented.decremented, lhsNode.baseExpression || lhsNode, { operands, precedingOperator }, ); diff --git a/src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts b/src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts index a2ea05675..bf767497a 100644 --- a/src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts +++ b/src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts @@ -25,7 +25,7 @@ const internalCallVisitor = { let sendTransactionNode : any; let newdecrementedSecretStates = []; node._newASTPointer.forEach(file => { - state.intFnindex = {}; + state.intFnindex = {}; state.internalFncName?.forEach( (name, index)=> { let callingFncName = state.callingFncName[index].name; if(file.fileName === name && file.nodeType === 'File') { diff --git a/test/contracts/Arrays1.zol b/test/contracts/Arrays1.zol index 2bd6f19c2..87c5c43b6 100644 --- a/test/contracts/Arrays1.zol +++ b/test/contracts/Arrays1.zol @@ -18,7 +18,10 @@ contract Assign { index++; a = a + index; index++; + j =j + 2; + j++; b[index] = value; + a += j; } function remove(secret uint256 value) public { diff --git a/test/contracts/Assign.zol b/test/contracts/Assign.zol index 0abde8418..ffaadce30 100644 --- a/test/contracts/Assign.zol +++ b/test/contracts/Assign.zol @@ -5,13 +5,14 @@ pragma solidity ^0.8.0; contract Assign { secret uint256 private a; + secret uint256 private b; function add(secret uint256 value) public { - unknown a += value; + a += value; + unknown b += value; } - function remove(secret uint256 value) public returns (uint256) { - a -= value; - return a; + function remove(secret uint256 value, secret uint256 value1) public { + b = b - value + value1; } } diff --git a/test/contracts/BucketsOfBalls.zol b/test/contracts/BucketsOfBalls.zol index ba2ab2ac1..da466a034 100644 --- a/test/contracts/BucketsOfBalls.zol +++ b/test/contracts/BucketsOfBalls.zol @@ -5,6 +5,7 @@ pragma solidity ^0.8.0; contract BucketsOfBalls { secret mapping(address => uint256) public buckets; + secret uint256 private a; function deposit( uint256 amountDeposit) public { buckets[msg.sender] += amountDeposit; @@ -12,6 +13,7 @@ contract BucketsOfBalls { function transfer(secret address toBucketId, secret uint256 numberOfBalls) public { buckets[msg.sender] -= numberOfBalls; - encrypt unknown buckets[toBucketId] += numberOfBalls; + unknown buckets[toBucketId] += numberOfBalls; + a += buckets[toBucketId] + numberOfBalls; } } From b28a529514ac7ed25ad280469a63f27b145f9392 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Fri, 19 Jul 2024 08:36:02 +0100 Subject: [PATCH 02/10] chore : added a fix --- src/transformers/visitors/checks/incrementedVisitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index 84d1543c7..4f8d823ea 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -107,7 +107,7 @@ const binOpToIncrements = (path: NodePath, state: any) => { ); const lhsNode = parentExpressionStatement?.node.expression?.leftHandSide; const assignmentOp = parentExpressionStatement?.node.expression?.operator; - console.log(assignmentOp); + console.log("assignmentOp", assignmentOp); console.log(path.node); const { operator, leftExpression, rightExpression } = path.node; const operands = [leftExpression, rightExpression]; From c96fc77410392d771032d5de5a755ca1ff78f700 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Mon, 22 Jul 2024 11:13:27 +0100 Subject: [PATCH 03/10] fix: fixed the incremented visitor --- .../visitors/checks/incrementedVisitor.ts | 42 +++++++++++-------- test/contracts/Assign.zol | 2 +- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index 4f8d823ea..be6900906 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -21,10 +21,16 @@ const literalOneNode = { precedingOperator: '', }; -const collectIncrements = (increments: any, incrementedIdentifier: any) => { +const collectIncrements = (increments: any, incrementedIdentifier: any, assignmentOperator: any) => { + + console.log("assignmentOperator", assignmentOperator); const { operands, precedingOperator } = increments; const newIncrements: any[] = []; for (const [index, operand] of operands.entries()) { + if(assignmentOperator === '=' && precedingOperator[1] === '-' && index != 0){ + operand.precedingOperator = precedingOperator[index] === '+' ? '-' : '+'; + } + else operand.precedingOperator = precedingOperator[index]; if ( operand.name !== incrementedIdentifier.name && @@ -36,6 +42,14 @@ const collectIncrements = (increments: any, incrementedIdentifier: any) => { return newIncrements; }; +const mixedOperatorsWarning = (path: NodePath) => { + backtrace.getSourceCode(path.node.src); + logger.warn( + `When we mix positive and negative operands in assigning to a secret variable, we may encounter underflow errors. Make sure that incrementing (a = a + ...) always increases the secret state value while decrementing (a = a - ...) decreases it. + \nWhenever we see something like a = a + b - c, we assume it's a positive incrementation, so b > c. Similarly, we assume a = a - b + c is a decrementation, so c - b < a.`, + ); +}; + // marks the parent ExpressionStatement const markParentIncrementation = ( path: NodePath, @@ -50,6 +64,7 @@ const markParentIncrementation = ( : incrementedIdentifier; const parent = path.getAncestorOfType('ExpressionStatement'); if (!parent) throw new Error(`No parent of node ${path.node.name} found`); + const assignmentOp = parent?.node.expression?.operator; parent.isIncremented = isIncremented; parent.isDecremented = isDecremented; parent.incrementedDeclaration = incrementedIdentifier.referencedDeclaration; @@ -58,7 +73,7 @@ const markParentIncrementation = ( state.incrementedIdentifier = incrementedIdentifier; if (increments?.operands) - increments = collectIncrements(increments, incrementedIdentifier); + increments = collectIncrements(increments, incrementedIdentifier, assignmentOp); increments?.forEach((inc: any) => { if ( inc.precedingOperator === '-' && @@ -94,12 +109,7 @@ const getIncrementedPath = (path: NodePath, state: any) => { state.stopTraversal = !!state.incrementedPath?.node; }; -const mixedOperatorsWarning = (path: NodePath) => { - backtrace.getSourceCode(path.node.src); - logger.warn( - `When we mix positive and negative operands in assigning to a secret variable, we may encounter underflow errors. Make sure that incrementing (a = a + ...) always increases the secret state value while decrementing (a = a - ...) decreases it. \nWhenever we see something like a = a + b - c, we assume it's a positive incrementation, so b > c. Similarly, we assume a = a - b + c is a decrementation, so c - b < a.`, - ); -}; + const binOpToIncrements = (path: NodePath, state: any) => { const parentExpressionStatement = path.getAncestorOfType( @@ -107,9 +117,7 @@ const binOpToIncrements = (path: NodePath, state: any) => { ); const lhsNode = parentExpressionStatement?.node.expression?.leftHandSide; const assignmentOp = parentExpressionStatement?.node.expression?.operator; - console.log("assignmentOp", assignmentOp); - console.log(path.node); - const { operator, leftExpression, rightExpression } = path.node; + const { operator, leftExpression, rightExpression } = path.node ; const operands = [leftExpression, rightExpression]; const precedingOperator = ['+', operator]; @@ -129,8 +137,8 @@ const binOpToIncrements = (path: NodePath, state: any) => { for (const [index, operand] of operands.entries()) { if (operand.nodeType === 'BinaryOperation') { operands[index] = operand.leftExpression; - operands.push(operand.rightExpression); - precedingOperator.push(operand.operator); + operands.splice(index+1, 0, operand.rightExpression); + precedingOperator.splice(index+1, 0, operand.operator); } } // if we have mixed operators, we may have an underflow or not be able to tell whether this is increasing (incrementation) or decreasing (decrementation) the secret value @@ -354,7 +362,6 @@ export default { } const { operands, precedingOperator } = binOpToIncrements(path, state) || {}; - if (!operands || !precedingOperator) return; // if we find our lhs variable (a) on the rhs (a = a + b), then we make sure we don't find it again (a = a + b + a = b + 2a) @@ -423,11 +430,10 @@ export default { if ( precedingOperator.length > 2 && precedingOperator.includes('+') && - precedingOperator.includes('-') && - precedingOperator[0] === '+' + precedingOperator.includes('-') ){ - isIncremented.decremented = false; - + + isIncremented.decremented = precedingOperator[1] === '+' ? false : true; } markParentIncrementation( diff --git a/test/contracts/Assign.zol b/test/contracts/Assign.zol index ffaadce30..22f73a84a 100644 --- a/test/contracts/Assign.zol +++ b/test/contracts/Assign.zol @@ -12,7 +12,7 @@ contract Assign { } function remove(secret uint256 value, secret uint256 value1) public { - b = b - value + value1; + b = b - (value + value1); } } From ee5ba72acf9fc4eb65d27f120492b76c8f93bf06 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Mon, 22 Jul 2024 17:43:13 +0100 Subject: [PATCH 04/10] fix: fixed the incremented visitor --- .../visitors/checks/incrementedVisitor.ts | 62 ++++++++++++++----- test/contracts/Assign-Increment.zol | 30 +++++++++ test/contracts/Assign.zol | 6 +- 3 files changed, 80 insertions(+), 18 deletions(-) create mode 100644 test/contracts/Assign-Increment.zol diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index be6900906..d1054961e 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -21,14 +21,19 @@ const literalOneNode = { precedingOperator: '', }; -const collectIncrements = (increments: any, incrementedIdentifier: any, assignmentOperator: any) => { - - console.log("assignmentOperator", assignmentOperator); +const collectIncrements = (increments: any, incrementedIdentifier: any, assignmentOperator: any, isTupleExpression: boolean) => { const { operands, precedingOperator } = increments; const newIncrements: any[] = []; for (const [index, operand] of operands.entries()) { if(assignmentOperator === '=' && precedingOperator[1] === '-' && index != 0){ + if(index == 1) + operand.precedingOperator = '+'; + else { + if(!isTupleExpression) operand.precedingOperator = precedingOperator[index] === '+' ? '-' : '+'; + else + operand.precedingOperator = precedingOperator[index]; + } } else operand.precedingOperator = precedingOperator[index]; @@ -48,6 +53,7 @@ const mixedOperatorsWarning = (path: NodePath) => { `When we mix positive and negative operands in assigning to a secret variable, we may encounter underflow errors. Make sure that incrementing (a = a + ...) always increases the secret state value while decrementing (a = a - ...) decreases it. \nWhenever we see something like a = a + b - c, we assume it's a positive incrementation, so b > c. Similarly, we assume a = a - b + c is a decrementation, so c - b < a.`, ); + }; // marks the parent ExpressionStatement @@ -64,6 +70,7 @@ const markParentIncrementation = ( : incrementedIdentifier; const parent = path.getAncestorOfType('ExpressionStatement'); if (!parent) throw new Error(`No parent of node ${path.node.name} found`); + const isTupleExpression = parent?.node.expression?.rightHandSide?.nodeType === 'TupleExpression' || parent?.node.expression?.rightHandSide?.rightExpression?.nodeType === 'TupleExpression' const assignmentOp = parent?.node.expression?.operator; parent.isIncremented = isIncremented; parent.isDecremented = isDecremented; @@ -71,9 +78,8 @@ const markParentIncrementation = ( parent.node.expression.incrementedDeclaration = parent.incrementedDeclaration; state.unmarkedIncrementation = false; state.incrementedIdentifier = incrementedIdentifier; - if (increments?.operands) - increments = collectIncrements(increments, incrementedIdentifier, assignmentOp); + increments = collectIncrements(increments, incrementedIdentifier, assignmentOp, isTupleExpression); increments?.forEach((inc: any) => { if ( inc.precedingOperator === '-' && @@ -118,9 +124,10 @@ const binOpToIncrements = (path: NodePath, state: any) => { const lhsNode = parentExpressionStatement?.node.expression?.leftHandSide; const assignmentOp = parentExpressionStatement?.node.expression?.operator; const { operator, leftExpression, rightExpression } = path.node ; + const operands = [leftExpression, rightExpression]; const precedingOperator = ['+', operator]; - + const isTupleExpression = operands[1].nodeType === 'TupleExpression'; // if we dont have any + or -, it can't be an incrementation if ( !operator.includes('+') && @@ -131,16 +138,32 @@ const binOpToIncrements = (path: NodePath, state: any) => { markParentIncrementation(path, state, false, false, lhsNode); return; } - + // correct the operands for case when a = a - (b + c + d). + if(assignmentOp === '=' && isTupleExpression) { + + operands[0] = operands[1].components[0].leftExpression; + precedingOperator.push(operands[1].components[0].operator); + operands[1] = operands[1].components[0].rightExpression; + + for (const [index, operand] of operands.entries()) { + if (operand.nodeType === 'BinaryOperation') { + operands[index] = operand.leftExpression; + operands.splice(index+1, 0, operand.rightExpression); + precedingOperator.splice(index+2, 0, operand.operator); + } + } + + } // fills an array of operands // e.g. if we have a = b - c + a + d, operands = [b, c, a, d] + if(!isTupleExpression){ for (const [index, operand] of operands.entries()) { if (operand.nodeType === 'BinaryOperation') { operands[index] = operand.leftExpression; operands.splice(index+1, 0, operand.rightExpression); precedingOperator.splice(index+1, 0, operand.operator); } - } + }} // if we have mixed operators, we may have an underflow or not be able to tell whether this is increasing (incrementation) or decreasing (decrementation) the secret value if ( precedingOperator.length > 2 && @@ -148,8 +171,23 @@ const binOpToIncrements = (path: NodePath, state: any) => { precedingOperator.includes('-') && parentExpressionStatement ) - mixedOperatorsWarning(parentExpressionStatement); - + { + mixedOperatorsWarning(parentExpressionStatement); + if(!isTupleExpression) + logger.warn( + `Whenever you have multiple operands in an expression, such as a = a - b - c + d, it's better to use parentheses for clarity. For example, rewrite it as a = a - (b + c - d). This makes the expression easier to understand. `, + ); +} +if(assignmentOp === '=') { + if(isTupleExpression) { + operands.splice(0, 0, path.node.leftExpression); + } else { + operands.splice(1, 0, operands[0].rightExpression); + precedingOperator.splice(1, 0, operands[0].operator); + operands[0] = operands[0].leftExpression; + } + } + return { operands, precedingOperator }; }; @@ -187,8 +225,6 @@ export default { const { isIncremented, isDecremented } = path; expressionNode.isIncremented = isIncremented; expressionNode.isDecremented = isDecremented; - // console.log('expression Node:', expressionNode) - // console.log(state); // print if in debug mode if (logger.level === 'debug') backtrace.getSourceCode(node.src); logger.debug(`statement is incremented? ${isIncremented}`); @@ -432,10 +468,8 @@ export default { precedingOperator.includes('+') && precedingOperator.includes('-') ){ - isIncremented.decremented = precedingOperator[1] === '+' ? false : true; } - markParentIncrementation( path, state, diff --git a/test/contracts/Assign-Increment.zol b/test/contracts/Assign-Increment.zol new file mode 100644 index 000000000..1db0cc540 --- /dev/null +++ b/test/contracts/Assign-Increment.zol @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: CC0 + +pragma solidity ^0.8.0; + +contract Assign { + + secret uint256 private a; + secret uint256 private b; + function add(secret uint256 value) public { + a += value; + unknown b += value; + } + + function remove(secret uint256 value, secret uint256 value1) public { + a += value; + b -= value + value1; + } + + function add1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3) public { + a = a + value - value1 + value3; + unknown b = b + value1 - value2 - value3; +} + + +function remove1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3) public { + a = a - value - value1 + value3; + b = b - (value1 - value2 + value3); +} + +} \ No newline at end of file diff --git a/test/contracts/Assign.zol b/test/contracts/Assign.zol index 22f73a84a..6fad02733 100644 --- a/test/contracts/Assign.zol +++ b/test/contracts/Assign.zol @@ -5,14 +5,12 @@ pragma solidity ^0.8.0; contract Assign { secret uint256 private a; - secret uint256 private b; function add(secret uint256 value) public { a += value; - unknown b += value; } - function remove(secret uint256 value, secret uint256 value1) public { - b = b - (value + value1); + function remove(secret uint256 value) public { + a += value ; } } From d5b6b2477a137379efa3843e7a496ec3e21b4d70 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 09:46:21 +0100 Subject: [PATCH 05/10] fix: fixed for visitor --- src/transformers/visitors/checks/incrementedVisitor.ts | 6 +++--- test/contracts/Arrays2.zol | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index d1054961e..26857b7b0 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -178,16 +178,16 @@ const binOpToIncrements = (path: NodePath, state: any) => { `Whenever you have multiple operands in an expression, such as a = a - b - c + d, it's better to use parentheses for clarity. For example, rewrite it as a = a - (b + c - d). This makes the expression easier to understand. `, ); } -if(assignmentOp === '=') { +if(assignmentOp === '=' && precedingOperator.length > 2) { if(isTupleExpression) { operands.splice(0, 0, path.node.leftExpression); } else { + if(operands[0].rightExpression){ operands.splice(1, 0, operands[0].rightExpression); precedingOperator.splice(1, 0, operands[0].operator); - operands[0] = operands[0].leftExpression; + operands[0] = operands[0].leftExpression;} } } - return { operands, precedingOperator }; }; diff --git a/test/contracts/Arrays2.zol b/test/contracts/Arrays2.zol index d85cc8a0d..fba35bee4 100644 --- a/test/contracts/Arrays2.zol +++ b/test/contracts/Arrays2.zol @@ -20,7 +20,7 @@ contract Assign { b[index] = value; index++; j++; - b[index] = value +index +j; + b[index] = value +index +j; index += 1; a += value + index; b[index] = value + index; From 0b1a045c3c14ecb4c582ab3089fb556ace3645b0 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 09:47:38 +0100 Subject: [PATCH 06/10] chore: modified a contract --- test/contracts/Arrays2.zol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contracts/Arrays2.zol b/test/contracts/Arrays2.zol index fba35bee4..45b72b900 100644 --- a/test/contracts/Arrays2.zol +++ b/test/contracts/Arrays2.zol @@ -20,7 +20,7 @@ contract Assign { b[index] = value; index++; j++; - b[index] = value +index +j; + b[index] = (value - index +j); index += 1; a += value + index; b[index] = value + index; From 66a7dc42aec4efb8b9b76ad0b3d2ae2b073fe468 Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 11:15:38 +0100 Subject: [PATCH 07/10] chore: fixed the contract --- test/contracts/BucketsOfBalls.zol | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/contracts/BucketsOfBalls.zol b/test/contracts/BucketsOfBalls.zol index da466a034..95632a597 100644 --- a/test/contracts/BucketsOfBalls.zol +++ b/test/contracts/BucketsOfBalls.zol @@ -5,7 +5,6 @@ pragma solidity ^0.8.0; contract BucketsOfBalls { secret mapping(address => uint256) public buckets; - secret uint256 private a; function deposit( uint256 amountDeposit) public { buckets[msg.sender] += amountDeposit; @@ -14,6 +13,5 @@ contract BucketsOfBalls { function transfer(secret address toBucketId, secret uint256 numberOfBalls) public { buckets[msg.sender] -= numberOfBalls; unknown buckets[toBucketId] += numberOfBalls; - a += buckets[toBucketId] + numberOfBalls; } } From b6e50f2e29e65330d6225691b4f7a3ffc334dd9f Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 14:23:49 +0100 Subject: [PATCH 08/10] chore: added the requested comments --- .../visitors/checks/incrementedVisitor.ts | 30 ++++++++++++------- test/contracts/Assign-Increment.zol | 8 ++--- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/transformers/visitors/checks/incrementedVisitor.ts b/src/transformers/visitors/checks/incrementedVisitor.ts index 26857b7b0..9b3fdc4c9 100644 --- a/src/transformers/visitors/checks/incrementedVisitor.ts +++ b/src/transformers/visitors/checks/incrementedVisitor.ts @@ -25,6 +25,8 @@ const collectIncrements = (increments: any, incrementedIdentifier: any, assignme const { operands, precedingOperator } = increments; const newIncrements: any[] = []; for (const [index, operand] of operands.entries()) { +// This Logic, changes the sign in case of decrements when don't have a tuple expression as in the circuits/Orchestration we +// translate a = a - b + c - d as a = a - (b - c + d) if(assignmentOperator === '=' && precedingOperator[1] === '-' && index != 0){ if(index == 1) operand.precedingOperator = '+'; @@ -118,14 +120,15 @@ const getIncrementedPath = (path: NodePath, state: any) => { const binOpToIncrements = (path: NodePath, state: any) => { - const parentExpressionStatement = path.getAncestorOfType( + let parentExpressionStatement = path.getAncestorOfType( 'ExpressionStatement', ); const lhsNode = parentExpressionStatement?.node.expression?.leftHandSide; const assignmentOp = parentExpressionStatement?.node.expression?.operator; const { operator, leftExpression, rightExpression } = path.node ; - const operands = [leftExpression, rightExpression]; + let operands = [leftExpression, rightExpression]; + const precedingOperator = ['+', operator]; const isTupleExpression = operands[1].nodeType === 'TupleExpression'; // if we dont have any + or -, it can't be an incrementation @@ -139,32 +142,37 @@ const binOpToIncrements = (path: NodePath, state: any) => { return; } // correct the operands for case when a = a - (b + c + d). - if(assignmentOp === '=' && isTupleExpression) { - - operands[0] = operands[1].components[0].leftExpression; + if(isTupleExpression) { + operands[0] = operands[1].components[0].rightExpression; precedingOperator.push(operands[1].components[0].operator); - operands[1] = operands[1].components[0].rightExpression; + operands[1] = operands[1].components[0].leftExpression; for (const [index, operand] of operands.entries()) { if (operand.nodeType === 'BinaryOperation') { operands[index] = operand.leftExpression; - operands.splice(index+1, 0, operand.rightExpression); - precedingOperator.splice(index+2, 0, operand.operator); + operands.splice(0, 0, operand.rightExpression); + precedingOperator.splice(2, 0, operand.operator); } } + operands.splice(0, 0, operands[operands.length -1]).slice(0, -1); } // fills an array of operands // e.g. if we have a = b - c + a + d, operands = [b, c, a, d] if(!isTupleExpression){ + operands = operands.reverse(); for (const [index, operand] of operands.entries()) { if (operand.nodeType === 'BinaryOperation') { operands[index] = operand.leftExpression; - operands.splice(index+1, 0, operand.rightExpression); - precedingOperator.splice(index+1, 0, operand.operator); + operands.splice(0, 0, operand.rightExpression); + precedingOperator.splice(1, 0, operand.operator); } - }} + } + operands.splice(0, 0, operands[operands.length -1]); +} + // if we have mixed operators, we may have an underflow or not be able to tell whether this is increasing (incrementation) or decreasing (decrementation) the secret value + // Here we give out a warning when we don't use parentheses. if ( precedingOperator.length > 2 && precedingOperator.includes('+') && diff --git a/test/contracts/Assign-Increment.zol b/test/contracts/Assign-Increment.zol index 1db0cc540..68316e748 100644 --- a/test/contracts/Assign-Increment.zol +++ b/test/contracts/Assign-Increment.zol @@ -16,15 +16,15 @@ contract Assign { b -= value + value1; } - function add1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3) public { + function add1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3, secret uint256 value4) public { a = a + value - value1 + value3; - unknown b = b + value1 - value2 - value3; + unknown b = b + (value1 - value2 - value3 + value4); } -function remove1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3) public { +function remove1(secret uint256 value, secret uint256 value1, secret uint256 value2, secret uint256 value3, secret uint256 value4) public { a = a - value - value1 + value3; - b = b - (value1 - value2 + value3); + b = b - value1 - value2 + value3 - value4; } } \ No newline at end of file From 951f999ce56e078d811ac0721a65e179cacf0c7f Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 14:36:57 +0100 Subject: [PATCH 09/10] chore: fixed the contract --- test/contracts/Assign.zol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contracts/Assign.zol b/test/contracts/Assign.zol index 6fad02733..0ccd4b1c0 100644 --- a/test/contracts/Assign.zol +++ b/test/contracts/Assign.zol @@ -6,7 +6,7 @@ contract Assign { secret uint256 private a; function add(secret uint256 value) public { - a += value; + unknown a += value; } function remove(secret uint256 value) public { From 353237a3a8c398f3f5db159c7e06cb776e5bc6dc Mon Sep 17 00:00:00 2001 From: Swati Rawal Date: Tue, 23 Jul 2024 15:14:56 +0100 Subject: [PATCH 10/10] chore: fixed the contract --- test/contracts/Assign.zol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/contracts/Assign.zol b/test/contracts/Assign.zol index 0ccd4b1c0..b7c546fbc 100644 --- a/test/contracts/Assign.zol +++ b/test/contracts/Assign.zol @@ -10,7 +10,7 @@ contract Assign { } function remove(secret uint256 value) public { - a += value ; + a -= value ; } }