YAS-197 Require Message Bug

packages/rollup-dev-tools/src/tools/transpiler/transpiler.ts

@@ -9,7 +9,12 @@ import {
   bufferToBytecode,
   getPCOfEVMBytecodeIndex,
 } from '@eth-optimism/rollup-core'
-import { getLogger, bufToHexString, add0x } from '@eth-optimism/core-utils'
+import {
+  getLogger,
+  bufToHexString,
+  add0x,
+  bufferUtils,
+} from '@eth-optimism/core-utils'
 
 import BigNum = require('bn.js')
 
@@ -28,6 +33,7 @@ import {
 } from '../../types/transpiler'
 import { accountForJumps } from './jump-replacement'
 import { createError } from './util'
+import { format } from 'path'
 
 const log = getLogger('transpiler-impl')
 
@@ -82,12 +88,14 @@ export class TranspilerImpl implements Transpiler {
     )
     const startOfDeployedBytecode: number = bytecode.indexOf(deployedBytecode)
     if (startOfDeployedBytecode === -1) {
-      log.debug(
-        `WARNING: Could not find deployed bytecode (${bufToHexString(
-          deployedBytecode
-        )}) within the original bytecode (${bufToHexString(
-          bytecode
-        )}).  If you are using a custom compiler, this may break.`
+      errors.push(
+        TranspilerImpl.createError(
+          0,
+          TranspilationErrors.MISSING_DEPLOYED_BYTECODE_ERROR,
+          `WARNING: Could not find deployed bytecode (${bufToHexString(
+            deployedBytecode
+          )}) within the original bytecode (${bufToHexString(bytecode)}).`
+        )
       )
     }
 
@@ -151,6 +159,13 @@ export class TranspilerImpl implements Transpiler {
       errors
     )
 
+    // problem is after here?
+    log.debug(
+      `final transpiled deployed bytecode: \n${formatBytecode(
+        finalTranspiledDeployedBytecode
+      )}`
+    )
+
     // **** DETECT AND TAG USES OF CODECOPY IN CONSTRUCTOR BYTECODE AND TRANSPILE ****
 
     let taggedOriginalConstructorInitLogic: EVMBytecode
@@ -270,7 +285,7 @@ export class TranspilerImpl implements Transpiler {
           newConstantOffset
         ).toBuffer('be', op.opcode.programBytesConsumed)
         log.debug(
-          `fixing CODECOPY(constant) ad PC 0x${getPCOfEVMBytecodeIndex(
+          `fixing CODECOPY(constant) at PC 0x${getPCOfEVMBytecodeIndex(
             index,
             taggedBytecode
           ).toString(16)}.  Setting new index to 0x${bufToHexString(
@@ -284,28 +299,28 @@ export class TranspilerImpl implements Transpiler {
   }
 
   // Finds and tags the PUSHN's which are detected to be associated with CODECOPYing deployed bytecode which is returned during CREATE/CREATE2.
-  // Tags based on the pattern:
-  // PUSH2 // codecopy's and RETURN's length
-  // DUP1 // DUPed to use twice, for RETURN and CODECOPY both
-  // PUSH2 // codecopy's offset
-  // PUSH1 codecopy's destOffset
-  // CODECOPY // copy
-  // PUSH1 0 // RETURN offset
-  // RETURN // uses above RETURN offset and DUP'ed length above
   // See https://github.com/ethereum-optimism/optimistic-rollup/wiki/CODECOPYs for more details.
   private findAndTagDeployedBytecodeReturner(
     bytecode: EVMBytecode
   ): EVMBytecode {
     for (let index = 0; index < bytecode.length - 6; index++) {
       const op: EVMOpcodeAndBytes = bytecode[index]
+      // Tags based on the pattern used for deploying non-library contracts:
+      // PUSH2 // codecopy's and RETURN's length
+      // DUP1 // DUPed to use twice, for RETURN and CODECOPY both
+      // PUSH2 // codecopy's offset
+      // PUSH1 codecopy's destOffset
+      // CODECOPY // copy
+      // PUSH1 0 // RETURN offset
+      // RETURN // uses above RETURN offset and DUP'ed length above
       if (
         Opcode.isPUSHOpcode(op.opcode) &&
         Opcode.isPUSHOpcode(bytecode[index + 2].opcode) &&
         bytecode[index + 4].opcode === Opcode.CODECOPY &&
         bytecode[index + 6].opcode === Opcode.RETURN
       ) {
         log.debug(
-          `detected a [CODECOPY(deployed bytecode)... RETURN] (CREATE/2 deployment logic) pattern starting at PC: 0x${getPCOfEVMBytecodeIndex(
+          `detected a NON-LIBRARY [CODECOPY(deployed bytecode)... RETURN] (CREATE/2 deployment logic) pattern starting at PC: 0x${getPCOfEVMBytecodeIndex(
             index,
             bytecode
           ).toString(16)}. Tagging the offset and size...`
@@ -329,6 +344,45 @@ export class TranspilerImpl implements Transpiler {
           },
         }
       }
+      // Tags based on the pattern used for deploying library contracts:
+      // PUSH2: 0x0158  [0x0158] // deployed bytecode length
+      // PUSH2: 0x0026  [0x0026, 0x0158] // deployed bytecode start
+      // PUSH1: 0x0b  [0x0b, 0x0026, 0x0158] // destoffset of code to copy
+      // DUP3
+      // DUP3
+      // DUP3  [0x0b, 0x0026, 0x0158, 0x0b, 0x0026, 0x0158]
+      // CODECOPY  [0x0b, 0x0026, 0x0158]
+      else if (
+        Opcode.isPUSHOpcode(op.opcode) &&
+        Opcode.isPUSHOpcode(bytecode[index + 1].opcode) &&
+        Opcode.isPUSHOpcode(bytecode[index + 2].opcode) &&
+        bytecode[index + 6].opcode === Opcode.CODECOPY
+      ) {
+        log.debug(
+          `detected a LIBRARY [CODECOPY(deployed bytecode)... RETURN] (library deployment logic) pattern starting at PC: 0x${getPCOfEVMBytecodeIndex(
+            index,
+            bytecode
+          ).toString(16)}. Tagging the offset and size...`
+        )
+        bytecode[index] = {
+          opcode: op.opcode,
+          consumedBytes: op.consumedBytes,
+          tag: {
+            padPUSH: true,
+            reasonTagged: IS_DEPLOY_CODE_LENGTH,
+            metadata: undefined,
+          },
+        }
+        bytecode[index + 1] = {
+          opcode: bytecode[index + 1].opcode,
+          consumedBytes: bytecode[index + 1].consumedBytes,
+          tag: {
+            padPUSH: true,
+            reasonTagged: IS_DEPLOY_CODECOPY_OFFSET,
+            metadata: undefined,
+          },
+        }
+      }
     }
     return bytecode
   }
@@ -497,6 +551,30 @@ export class TranspilerImpl implements Transpiler {
     let lastOpcode: EVMOpcode
     let insideUnreachableCode: boolean = false
 
+    const numCodes = bytecode.length
+    const lastOpcodeAndConsumedBytes = bytecode[numCodes - 1]
+    if (
+      Opcode.isPUSHOpcode(lastOpcodeAndConsumedBytes.opcode) &&
+      lastOpcodeAndConsumedBytes.consumedBytes.byteLength <
+        lastOpcodeAndConsumedBytes.opcode.programBytesConsumed
+    ) {
+      // todo: handle with warnings[] separate from errors[]?
+      const message: string = `Final input opcode: ${
+        lastOpcodeAndConsumedBytes.opcode.name
+      } consumes ${
+        lastOpcodeAndConsumedBytes.opcode.programBytesConsumed
+      }, but only consumes 0x${bufToHexString(
+        lastOpcodeAndConsumedBytes.consumedBytes
+      )} at the end of input bytecode.  Padding with zeros under the assumption that this arises from a constant at EOF...`
+      log.debug(message)
+      bytecode[numCodes - 1].consumedBytes = bufferUtils.padRight(
+        lastOpcodeAndConsumedBytes.consumedBytes,
+        lastOpcodeAndConsumedBytes.opcode.programBytesConsumed
+      )
+    }
+    // todo remove this weak log
+    log.debug(`after padding: ${formatBytecode(bytecode)}`)
+
     const bytecodeBuf: Buffer = bytecodeToBuffer(bytecode)
     // todo remove once confirmed with Kevin?
     let seenJump: boolean = false
@@ -543,16 +621,6 @@ export class TranspilerImpl implements Transpiler {
           pc += opcode.programBytesConsumed
           continue
         }
-        if (
-          !TranspilerImpl.enoughBytesLeft(
-            opcode,
-            bytecodeBuf.length,
-            pc,
-            errors
-          )
-        ) {
-          break
-        }
       }
       if (insideUnreachableCode && !opcode) {
         const unreachableCode: Buffer = bytecodeBuf.slice(pc, pc + 1)
@@ -696,38 +764,6 @@ export class TranspilerImpl implements Transpiler {
     return true
   }
 
-  /**
-   * Returns whether or not there are enough bytes left in the bytecode for the provided Opcode.
-   * If it is not, it creates a new TranpilationError and appends it to the provided list.
-   *
-   * @param opcode The opcode in question.
-   * @param bytecodeLength The length of the bytecode being transpiled.
-   * @param pc The current program counter value.
-   * @param errors The cumulative errors list.
-   * @returns True if enough bytes are left for the Opcode to consume, False otherwise.
-   */
-  private static enoughBytesLeft(
-    opcode: EVMOpcode,
-    bytecodeLength: number,
-    pc: number,
-    errors: TranspilationError[]
-  ): boolean {
-    if (pc + opcode.programBytesConsumed >= bytecodeLength) {
-      const bytesLeft: number = bytecodeLength - pc - 1
-      const message: string = `Opcode: ${opcode.name} consumes ${
-        opcode.programBytesConsumed
-      }, but ${!!bytesLeft ? 'only ' : ''}${bytesLeft} ${
-        bytesLeft !== 1 ? 'bytes are' : 'byte is'
-      } left in input bytecode.`
-      log.debug(message)
-      errors.push(
-        createError(pc, TranspilationErrors.INVALID_BYTES_CONSUMED, message)
-      )
-      return false
-    }
-    return true
-  }
-
   /**
    * Util function to create TranspilationErrors.
    *

packages/rollup-dev-tools/src/types/transpiler/errors.ts

@@ -37,24 +37,6 @@ export class InvalidInitcodeError extends Error {
   }
 }
 
-export class MissingConstantError extends Error {
-  constructor(msg?: string) {
-    super(msg)
-  }
-}
-
-export class DetectedConstantOutOfBoundsError extends Error {
-  constructor(msg?: string) {
-    super(msg)
-  }
-}
-
-export class SubTranspilationError extends Error {
-  constructor(msg?: string) {
-    super(msg)
-  }
-}
-
 export class TranspilationErrors {
   public static readonly UNSUPPORTED_OPCODE: number = 0
   public static readonly OPCODE_NOT_WHITELISTED: number = 1
@@ -63,5 +45,6 @@ export class TranspilationErrors {
   public static readonly INVALID_INITCODE: number = 4
   public static readonly MISSING_CONSTANT_ERROR: number = 5
   public static readonly DETECTED_CONSTANT_OOB: number = 6
-  public static readonly SUB_TRANSPILATION_ERROR: number = 6
+  public static readonly SUB_TRANSPILATION_ERROR: number = 7
+  public static readonly MISSING_DEPLOYED_BYTECODE_ERROR: number = 8
 }

packages/rollup-dev-tools/test/contracts/constants-transpilation/ConstantGetter.sol

@@ -4,7 +4,7 @@ contract ConstantGetter {
 
     bytes32 public constant bytes32Constant = 0xABCDEF34ABCDEF34ABCDEF34ABCDEF34ABCDEF34ABCDEF34ABCDEF34ABCDEF34;
     bytes public constant bytesMemoryConstantA = hex"AAAdeadbeefAAAAAAdeadbeefAAAAAAdeadbeefAAAAAAdeadbeefAAAAAAdeadbeefAAAAAAdeadbeefAAAAAAdeadbeefAAA";
-    bytes public constant bytesMemoryConstantB = hex"BBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBB";
+    bytes public constant bytesMemoryConstantB = "this should pass but the error message is much longer";
 
     constructor(bytes memory _param) public {
         map[420] = _param;

packages/rollup-dev-tools/test/contracts/library-transpilation/SafeMathUser.sol

@@ -0,0 +1,9 @@
+pragma solidity ^0.5.0;
+
+import {SimpleSafeMath} from  './SimpleSafeMath.sol';
+
+contract SafeMathUser {
+  function use() public returns (uint) {
+    return SimpleSafeMath.addUint(2, 3);
+  }
+}

packages/rollup-dev-tools/test/contracts/library-transpilation/SimpleSafeMath.sol

@@ -0,0 +1,18 @@
+pragma solidity ^0.5.0;
+
+library SimpleSafeMath {
+
+    function subUint(uint a, uint b) public  returns(uint){
+
+        require(a >= b); // Make sure it doesn't return a negative value.
+        return a - b;
+
+    }
+    function addUint(uint a , uint b) public pure returns(uint){
+
+        uint c = a + b;
+
+        require(c >= a);   // Makre sure the right computation was made
+        return c;
+    }
+}

packages/rollup-dev-tools/test/transpiler/constants-transpilation.spec.ts

@@ -145,8 +145,8 @@ describe('Solitity contracts should have constants correctly accessible when usi
     retrievedBytesMemoryAVal.should.deep.equal(encodedBytesMemoryConstA)
   })
 
-  const bytesMemoryConstB: Buffer = hexStrToBuf(
-    'BBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBBBBBbeedfeedBBB'
+  const bytesMemoryConstB: Buffer = Buffer.from(
+    `this should pass but the error message is much longer`
   )
   it('should work for the first bytes memory constant', async () => {
     const retrievedBytesMemoryBVal: Buffer = await getGetterReturnedVal(