replaced CairoAssert by FunctionCall (#1057)

* replaced CairoAssert by FunctionCall * lint fixed * lint fixed * fixed Expression problems * renamed test files * added errorHandling tests to expectedResults * added errorHandling imports * updating * removed unused function * Comment failing test add some comments * Handle assertion functions at ast level --------- Co-authored-by: Carmen Cabrera <[email protected]>
NethermindEth · Jun 14, 2023 · 321bfe7 · 321bfe7
1 parent 74a9b02
commit 321bfe7
Show file tree

Hide file tree

Showing 19 changed files with 53 additions and 131 deletions.
diff --git a/src/ast/cairoNodes/cairoAssert.ts b/src/ast/cairoNodes/cairoAssert.ts
diff --git a/src/ast/cairoNodes/export.ts b/src/ast/cairoNodes/export.ts
@@ -1,4 +1,3 @@
-export * from './cairoAssert';
 export * from './cairoFunctionDefinition';
 export * from './cairoContract';
 export * from './cairoTempVarStatement';
diff --git a/src/ast/cairoNodes/index.ts b/src/ast/cairoNodes/index.ts
@@ -1,4 +1,3 @@
-export * from './cairoAssert';
 export * from './cairoContract';
 export * from './cairoGeneratedFunctionDefinition';
 export * from './cairoFunctionDefinition';

diff --git a/src/ast/visitor.ts b/src/ast/visitor.ts
@@ -65,12 +65,7 @@ import {
   StatementWithChildren,
   ASTNodeWithChildren,
 } from 'solc-typed-ast';
-import {
-  CairoAssert,
-  CairoContract,
-  CairoFunctionDefinition,
-  CairoTempVarStatement,
-} from './cairoNodes';
+import { CairoContract, CairoFunctionDefinition, CairoTempVarStatement } from './cairoNodes';
 
 import { AST } from './ast';
 import { CairoGeneratedFunctionDefinition } from './cairoNodes/cairoGeneratedFunctionDefinition';
@@ -106,7 +101,6 @@ export abstract class ASTVisitor<T> {
     // Expression
     else if (node instanceof Assignment) res = this.visitAssignment(node, ast);
     else if (node instanceof BinaryOperation) res = this.visitBinaryOperation(node, ast);
-    else if (node instanceof CairoAssert) res = this.visitCairoAssert(node, ast);
     else if (node instanceof Conditional) res = this.visitConditional(node, ast);
     else if (node instanceof ElementaryTypeNameExpression)
       res = this.visitElementaryTypeNameExpression(node, ast);
@@ -349,9 +343,6 @@ export abstract class ASTVisitor<T> {
   visitSourceUnit(node: SourceUnit, ast: AST): T {
     return this.visitASTNodeWithChildren(node, ast);
   }
-  visitCairoAssert(node: CairoAssert, ast: AST): T {
-    return this.visitExpression(node, ast);
-  }
   visitTypeName(node: TypeName, ast: AST): T {
     return this.commonVisit(node, ast);
   }

diff --git a/src/cairoWriter/index.ts b/src/cairoWriter/index.ts
@@ -58,7 +58,6 @@ import {
   WhileStatement,
 } from 'solc-typed-ast';
 import {
-  CairoAssert,
   CairoContract,
   CairoFunctionDefinition,
   CairoGeneratedFunctionDefinition,
@@ -69,7 +68,6 @@ import {
   AssignmentWriter,
   BinaryOperationWriter,
   BlockWriter,
-  CairoAssertWriter,
   CairoContractWriter,
   CairoFunctionDefinitionWriter,
   CairoTempVarWriter,
@@ -104,7 +102,6 @@ export const CairoASTMapping = (ast: AST, throwOnUnimplemented: boolean) =>
     [BinaryOperation, new BinaryOperationWriter(ast, throwOnUnimplemented)],
     [Block, new BlockWriter(ast, throwOnUnimplemented)],
     [Break, new NotImplementedWriter(ast, throwOnUnimplemented)],
-    [CairoAssert, new CairoAssertWriter(ast, throwOnUnimplemented)],
     [CairoContract, new CairoContractWriter(ast, throwOnUnimplemented)],
     [CairoFunctionDefinition, new CairoFunctionDefinitionWriter(ast, throwOnUnimplemented)],
     [

diff --git a/src/cairoWriter/writers/cairoAssertWriter.ts b/src/cairoWriter/writers/cairoAssertWriter.ts
diff --git a/src/cairoWriter/writers/expressionStatementWriter.ts b/src/cairoWriter/writers/expressionStatementWriter.ts
@@ -6,7 +6,6 @@ import {
   FunctionCallKind,
   SrcDesc,
 } from 'solc-typed-ast';
-import { CairoAssert } from '../../ast/cairoNodes';
 import { CairoASTNodeWriter } from '../base';
 import { getDocumentation } from '../utils';
 
@@ -17,8 +16,7 @@ export class ExpressionStatementWriter extends CairoASTNodeWriter {
     if (
       (node.vExpression instanceof FunctionCall &&
         node.vExpression.kind !== FunctionCallKind.StructConstructorCall) ||
-      node.vExpression instanceof Assignment ||
-      node.vExpression instanceof CairoAssert
+      node.vExpression instanceof Assignment
     ) {
       return [[documentation, `${writer.write(node.vExpression)};`].join('\n')];
     } else {

diff --git a/src/cairoWriter/writers/index.ts b/src/cairoWriter/writers/index.ts
@@ -1,7 +1,6 @@
 export * from './assignmentWriter';
 export * from './binaryOperationWriter';
 export * from './blockWriter';
-export * from './cairoAssertWriter';
 export * from './cairoContractWriter';
 export * from './cairoFunctionDefinitionWriter';
 export * from './cairoGeneratedFunctionDefinitionWriter';

diff --git a/src/passes/builtinHandler/require.ts b/src/passes/builtinHandler/require.ts
@@ -4,13 +4,16 @@ import {
   ExpressionStatement,
   ExternalReferenceType,
   FunctionCall,
+  Identifier,
   Literal,
+  LiteralKind,
   Return,
 } from 'solc-typed-ast';
 import { AST } from '../../ast/ast';
-import { CairoAssert } from '../../ast/cairoNodes';
 import { ASTMapper } from '../../ast/mapper';
+import { cloneASTNode } from '../../utils/cloning';
 import { createBoolLiteral } from '../../utils/nodeTemplates';
+import { toHexString } from '../../utils/utils';
 
 export class Require extends ASTMapper {
   // Function to add passes that should have been run before this pass
@@ -28,7 +31,7 @@ export class Require extends ASTMapper {
       return;
     }
 
-    // Since the cairoAssert is not null, we have a require/revert/assert function call at hand
+    // Since cairoAssert is not null, this is a require/revert/assert function call
     assert(expressionNode instanceof FunctionCall);
 
     ast.replaceNode(node, cairoAssert);
@@ -44,42 +47,49 @@ export class Require extends ASTMapper {
   }
 
   requireToCairoAssert(expression: Expression | undefined, ast: AST): ExpressionStatement | null {
-    if (!(expression instanceof FunctionCall)) return null;
-    if (expression.vFunctionCallType !== ExternalReferenceType.Builtin) {
-      return null;
-    }
+    if (
+      expression instanceof FunctionCall &&
+      expression.vFunctionCallType === ExternalReferenceType.Builtin &&
+      ['assert', 'require', 'revert'].includes(expression.vIdentifier)
+    ) {
+      // TODO: The identifier node generated by the solc-typed-ast has different typestrings
+      //       and referencedDeclaration number for assert, require and revert Solidity functions.
+      //       Check typestring when updating solc-typed-ast version.
+      const assertIdentifier = cloneASTNode(expression.vExpression, ast);
+      assert(assertIdentifier instanceof Identifier);
+      assertIdentifier.name = 'assert';
 
-    if (expression.vIdentifier === 'require' || expression.vIdentifier === 'assert') {
-      const requireMessage =
-        expression.vArguments[1] instanceof Literal ? expression.vArguments[1].value : null;
+      const args: Expression[] = [];
+      if (expression.vIdentifier === 'revert') args.push(createBoolLiteral(false, ast));
+      args.push(...expression.vArguments);
+      if (args.length < 2) {
+        const message = 'Assertion error';
+        args.push(
+          new Literal(
+            ast.reserveId(),
+            '',
+            `literal_string "${message}"`,
+            LiteralKind.String,
+            toHexString(message),
+            message,
+          ),
+        );
+      }
 
       return new ExpressionStatement(
         ast.reserveId(),
         expression.src,
-        new CairoAssert(
+        new FunctionCall(
           ast.reserveId(),
-          expression.src,
-          expression.vArguments[0],
-          requireMessage,
-          expression.raw,
-        ),
-      );
-    } else if (expression.vIdentifier === 'revert') {
-      const revertMessage =
-        expression.vArguments[0] instanceof Literal ? expression.vArguments[0].value : null;
-
-      return new ExpressionStatement(
-        ast.reserveId(),
-        expression.src,
-        new CairoAssert(
-          ast.reserveId(),
-          expression.src,
-          createBoolLiteral(false, ast),
-          revertMessage,
-          expression.raw,
+          '',
+          expression.typeString,
+          expression.kind,
+          assertIdentifier,
+          args,
         ),
       );
     }
+
     return null;
   }
 }
diff --git a/src/passes/references/expectedLocationAnalyser.ts b/src/passes/references/expectedLocationAnalyser.ts
@@ -24,7 +24,6 @@ import {
   VariableDeclarationStatement,
 } from 'solc-typed-ast';
 import { AST } from '../../ast/ast';
-import { CairoAssert } from '../../ast/cairoNodes';
 import { ASTMapper } from '../../ast/mapper';
 import { locationIfComplexType } from '../../cairoUtilFuncGen/base';
 import { printNode } from '../../utils/astPrinter';
@@ -139,9 +138,9 @@ export class ExpectedLocationAnalyser extends ASTMapper {
 
     const parameterTypes = getParameterTypes(node, ast);
     // When calling `push`, the function receives two parameters nonetheless the argument is just one
-    // This does not explode because javascript does not gives an index out of range exception
+    // This does not explode because javascript does not give an index out of range exception
     node.vArguments.forEach((arg, index) => {
-      // Solc 0.7.0 types push and pop as you would expect, 0.8.0 adds an extra initial argument
+      // Solc 0.7.0 types push and pop as expected, 0.8.0 adds an extra initial argument
       const paramIndex = index + parameterTypes.length - node.vArguments.length;
       const t = parameterTypes[paramIndex];
       if (t instanceof PointerType) {
@@ -320,11 +319,6 @@ export class ExpectedLocationAnalyser extends ASTMapper {
     this.visitStatement(node, ast);
   }
 
-  visitCairoAssert(node: CairoAssert, ast: AST): void {
-    this.expectedLocations.set(node.vExpression, DataLocation.Default);
-    this.visitExpression(node, ast);
-  }
-
   visitIfStatement(node: IfStatement, ast: AST): void {
     this.expectedLocations.set(node.vCondition, DataLocation.Default);
     this.visitStatement(node, ast);

diff --git a/src/solWriter.ts b/src/solWriter.ts
@@ -9,7 +9,6 @@ import {
   SrcDesc,
 } from 'solc-typed-ast';
 import {
-  CairoAssert,
   CairoContract,
   CairoFunctionDefinition,
   CairoGeneratedFunctionDefinition,
@@ -108,21 +107,12 @@ class CairoImportFunctionDefinitionSolWriter extends ASTNodeWriter {
   }
 }
 
-class CairoAssertSolWriter extends ASTNodeWriter {
-  writeInner(node: CairoAssert, writer: ASTWriter): SrcDesc {
-    const result: SrcDesc = [];
-    result.push(`<cairo information> assert ${writer.write(node.vExpression)} = 1`);
-    return result;
-  }
-}
-
 const CairoExtendedASTWriterMapping = (printStubs: boolean) =>
   new Map<ASTNodeConstructor<ASTNode>, ASTNodeWriter>([
     [CairoContract, new CairoContractSolWriter()],
     [CairoFunctionDefinition, new CairoFunctionDefinitionSolWriter(printStubs)],
     [CairoGeneratedFunctionDefinition, new CairoGeneratedFunctionDefinitionSolWriter()],
     [CairoImportFunctionDefinition, new CairoImportFunctionDefinitionSolWriter()],
-    [CairoAssert, new CairoAssertSolWriter()],
   ]);
 
 export const CairoToSolASTWriterMapping = (printStubs: boolean) =>

diff --git a/src/utils/astChecking.ts b/src/utils/astChecking.ts
@@ -64,14 +64,14 @@ import {
 } from 'solc-typed-ast';
 import { pp } from 'solc-typed-ast/dist/misc/index';
 import { AST } from '../ast/ast';
-import { CairoAssert, CairoTempVarStatement } from '../ast/cairoNodes';
+import { CairoTempVarStatement } from '../ast/cairoNodes';
 import { ASTMapper } from '../ast/mapper';
 import { printNode } from './astPrinter';
 import { InsaneASTError } from './errors';
 import { safeGetNodeType } from './nodeTypeProcessing';
 import { isNameless } from './utils';
 
-// This is the solc-typed-ast AST checking code, with additions for CairoAssert and CairoContract
+// This is the solc-typed-ast AST checking code, with additions for CairoContract
 
 /**
  * Helper function to check if the node/nodes `arg` is in the `ASTContext` `ctx`.
@@ -667,8 +667,6 @@ export function checkSane(unit: SourceUnit, ctx: ASTContext): void {
     } else if (node instanceof UnaryOperation) {
       checkVFieldCtx(node, 'vSubExpression', ctx);
       checkDirectChildren(node, 'vSubExpression');
-    } else if (node instanceof CairoAssert) {
-      checkDirectChildren(node, 'vExpression');
     } else if (node instanceof CairoTempVarStatement) {
       // Not being checked because this node does not get affected by any
       // other ast pass

diff --git a/src/utils/cloning.ts b/src/utils/cloning.ts
@@ -44,7 +44,7 @@ import {
   WhileStatement,
 } from 'solc-typed-ast';
 import { AST } from '../ast/ast';
-import { CairoAssert, CairoFunctionDefinition } from '../ast/cairoNodes';
+import { CairoFunctionDefinition } from '../ast/cairoNodes';
 import { printNode } from './astPrinter';
 import { NotSupportedYetError, TranspileFailedError } from './errors';
 import { createParameterList } from './nodeTemplates';
@@ -104,14 +104,6 @@ function cloneASTNodeImpl<T extends ASTNode>(
       cloneASTNodeImpl(node.vRightExpression, ast, remappedIds),
       node.raw,
     );
-  } else if (node instanceof CairoAssert) {
-    newNode = new CairoAssert(
-      replaceId(node.id, ast, remappedIds),
-      node.src,
-      cloneASTNodeImpl(node.vExpression, ast, remappedIds),
-      node.assertMessage,
-      node.raw,
-    );
   } else if (node instanceof ElementaryTypeNameExpression) {
     newNode = new ElementaryTypeNameExpression(
       replaceId(node.id, ast, remappedIds),

diff --git a/src/utils/nodeTypeProcessing.ts b/src/utils/nodeTypeProcessing.ts
@@ -38,7 +38,6 @@ import { TranspileFailedError } from './errors';
 import { error } from './formatting';
 import { getContainingSourceUnit } from './utils';
 import { getNodeType, getNodeTypeInCtx } from './typeStrings/typeStringParser';
-import { CairoAssert } from '../ast/cairoNodes'; // eslint-disable-line
 
 /*
 Normal function calls and struct constructors require different methods for
@@ -298,9 +297,6 @@ export function safeGetNodeType(
   // if (node instanceof Literal) {
   //   return getNodeType(node, inference);
   // }
-  // if (node instanceof CairoAssert){
-  //   return new TupleType([]);
-  // }
   // if (node instanceof VariableDeclaration) {
   //   return inference.variableDeclarationToTypeNode(node);
   // }

diff --git a/tests/compilation/compilation.test.ts b/tests/compilation/compilation.test.ts
@@ -45,9 +45,9 @@ const expectedResults = new Map<string, ResultType>(
     ['deleteUses.sol', 'Success'],
     ['enums.sol', 'Success'],
     ['enums7.sol', 'Success'],
-    ['errorHandling/assert.sol', 'Success'],
-    ['errorHandling/require.sol', 'Success'],
-    ['errorHandling/revert.sol', 'Success'],
+    ['errorHandling/assertHandling.sol', 'Success'],
+    ['errorHandling/requireHandling.sol', 'Success'],
+    ['errorHandling/revertHandling.sol', 'Success'],
     ['events.sol', 'Success'],
     ['expressionSplitter/assignSimple.sol', 'Success'],
     ['expressionSplitter/funcCallSimple.sol', 'Success'],

diff --git a/...lation/contracts/errorHandling/assert.sol → ...ontracts/errorHandling/assertHandling.sol b/...lation/contracts/errorHandling/assert.sol → ...ontracts/errorHandling/assertHandling.sol
diff --git a/...ation/contracts/errorHandling/require.sol → ...ntracts/errorHandling/requireHandling.sol b/...ation/contracts/errorHandling/require.sol → ...ntracts/errorHandling/requireHandling.sol
diff --git a/...lation/contracts/errorHandling/revert.sol → ...ontracts/errorHandling/revertHandling.sol b/...lation/contracts/errorHandling/revert.sol → ...ontracts/errorHandling/revertHandling.sol
diff --git a/tests/compilation/passingContracts.ts b/tests/compilation/passingContracts.ts
@@ -35,9 +35,10 @@ export const passingContracts = [
   // 'tests/compilation/contracts/deleteUses.sol',
   'tests/compilation/contracts/enums.sol',
   // 'tests/compilation/contracts/enums7.sol',
-  // 'tests/compilation/contracts/errorHandling/assert.sol',
-  // 'tests/compilation/contracts/errorHandling/require.sol',
-  // 'tests/compilation/contracts/errorHandling/revert.sol',
+  'tests/compilation/contracts/errorHandling/assertHandling.sol',
+  'tests/compilation/contracts/errorHandling/requireHandling.sol',
+  // TODO: uncomment, when input checks are handled
+  // 'tests/compilation/contracts/errorHandling/revertHandling.sol',
   // 'tests/compilation/contracts/events.sol',
   // 'tests/compilation/contracts/expressionSplitter/assignSimple.sol',
   // 'tests/compilation/contracts/expressionSplitter/funcCallSimple.sol',