From 67f28645018244a0aeda6404b7fd4ea33c67110f Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 20 Feb 2018 13:42:35 -0800 Subject: Address feedback --- .gitignore | 2 +- packages/deployer/CHANGELOG.md | 4 +-- packages/deployer/src/cli.ts | 1 + packages/deployer/src/compiler.ts | 62 ++++++++++++++++++------------------ packages/deployer/src/deployer.ts | 32 +++++++++++-------- packages/deployer/src/utils/types.ts | 4 +-- 6 files changed, 55 insertions(+), 50 deletions(-) diff --git a/.gitignore b/.gitignore index 2f1f9d9f7..49d0604ea 100644 --- a/.gitignore +++ b/.gitignore @@ -73,4 +73,4 @@ packages/website/public/bundle* bin/ # generated contract artifacts -packages/contracts/src/artifacts \ No newline at end of file +packages/contracts/src/artifacts diff --git a/packages/deployer/CHANGELOG.md b/packages/deployer/CHANGELOG.md index f1e9d38ee..a63d9cf3b 100644 --- a/packages/deployer/CHANGELOG.md +++ b/packages/deployer/CHANGELOG.md @@ -1,8 +1,8 @@ # CHANGELOG -## v0.2.0 - _??_ +## v0.2.0 - _TBD, 2018_ - * Check dependencies when determining if contracts should be recompiled. + * Check dependencies when determining if contracts should be recompiled (#408). ## v0.1.0 - _February 16, 2018_ diff --git a/packages/deployer/src/cli.ts b/packages/deployer/src/cli.ts index b093dd71b..ba156ac20 100644 --- a/packages/deployer/src/cli.ts +++ b/packages/deployer/src/cli.ts @@ -16,6 +16,7 @@ const DEFAULT_NETWORK_ID = 50; const DEFAULT_JSONRPC_PORT = 8545; const DEFAULT_GAS_PRICE = (10 ** 9 * 2).toString(); const DEFAULT_CONTRACTS_LIST = '*'; + /** * Compiles all contracts with options passed in through CLI. * @param argv Instance of process.argv provided by yargs. diff --git a/packages/deployer/src/compiler.ts b/packages/deployer/src/compiler.ts index 783bc0ea3..149ca5d6d 100644 --- a/packages/deployer/src/compiler.ts +++ b/packages/deployer/src/compiler.ts @@ -20,17 +20,17 @@ import { import { utils } from './utils/utils'; const ALL_CONTRACTS_IDENTIFIER = '*'; -const SOLIDITY_VERSION_REGEX = /(?:solidity\s\^?)([0-9]{1,2}[.][0-9]{1,2}[.][0-9]{1,2})/; +const SOLIDITY_VERSION_REGEX = /(?:solidity\s\^?)(\d+\.\d+\.\d+)/; const SOLIDITY_FILE_EXTENSION_REGEX = /(.*\.sol)/; const IMPORT_REGEX = /(import\s)/; -const DEPENDENCY_PATH_REGEX = /"([^"]+)"/; +const DEPENDENCY_PATH_REGEX = /"([^"]+)"/; // Source: https://github.com/BlockChainCompany/soljitsu/blob/master/lib/shared.js export class Compiler { private _contractsDir: string; private _networkId: number; private _optimizerEnabled: number; private _artifactsDir: string; - private _contractSourcesIfExists?: ContractSources; + private _contractSources?: ContractSources; private _solcErrors: Set = new Set(); private _specifiedContracts: Set = new Set(); private _contractSourceData: ContractSourceData = {}; @@ -82,10 +82,10 @@ export class Compiler { private static _getContractSpecificSourceData(source: string): ContractSpecificSourceData { const dependencies: string[] = []; const sourceHash = ethUtil.sha3(source); - const solc_version = Compiler._parseSolidityVersion(source); + const solcVersion = Compiler._parseSolidityVersion(source); const contractSpecificSourceData: ContractSpecificSourceData = { dependencies, - solc_version, + solcVersion, sourceHash, }; const lines = source.split('\n'); @@ -149,12 +149,12 @@ export class Compiler { */ public async compileAllAsync(): Promise { await this._createArtifactsDirIfDoesNotExistAsync(); - this._contractSourcesIfExists = await Compiler._getContractSourcesAsync(this._contractsDir); - _.forIn(this._contractSourcesIfExists, (source, fileName) => { + this._contractSources = await Compiler._getContractSourcesAsync(this._contractsDir); + _.forIn(this._contractSources, (source, fileName) => { this._contractSourceData[fileName] = Compiler._getContractSpecificSourceData(source); }); const fileNames = this._specifiedContracts.has(ALL_CONTRACTS_IDENTIFIER) - ? _.keys(this._contractSourcesIfExists) + ? _.keys(this._contractSources) : Array.from(this._specifiedContracts.values()); _.forEach(fileNames, fileName => { this._setSourceTreeHash(fileName); @@ -169,29 +169,29 @@ export class Compiler { * @param fileName Name of contract with '.sol' extension. */ private async _compileContractAsync(fileName: string): Promise { - if (_.isUndefined(this._contractSourcesIfExists)) { + if (_.isUndefined(this._contractSources)) { throw new Error('Contract sources not yet initialized'); } const contractSpecificSourceData = this._contractSourceData[fileName]; - const currentArtifact = (await this._getContractArtifactOrReturnAsync(fileName)) as ContractArtifact; + const currentArtifactIfExists = (await this._getContractArtifactIfExistsAsync(fileName)) as ContractArtifact; const sourceHash = `0x${contractSpecificSourceData.sourceHash.toString('hex')}`; - const sourceTreeHash = `0x${contractSpecificSourceData.sourceTreeHash.toString('hex')}`; + const sourceTreeHash = `0x${contractSpecificSourceData.sourceTreeHashIfExists.toString('hex')}`; const shouldCompile = - _.isUndefined(currentArtifact) || - currentArtifact.networks[this._networkId].optimizer_enabled !== this._optimizerEnabled || - currentArtifact.networks[this._networkId].source_tree_hash !== sourceTreeHash; + _.isUndefined(currentArtifactIfExists) || + currentArtifactIfExists.networks[this._networkId].optimizer_enabled !== this._optimizerEnabled || + currentArtifactIfExists.networks[this._networkId].source_tree_hash !== sourceTreeHash; if (!shouldCompile) { return; } - const fullSolcVersion = binPaths[contractSpecificSourceData.solc_version]; + const fullSolcVersion = binPaths[contractSpecificSourceData.solcVersion]; const solcBinPath = `./solc/solc_bin/${fullSolcVersion}`; const solcBin = require(solcBinPath); const solcInstance = solc.setupMethods(solcBin); utils.consoleLog(`Compiling ${fileName}...`); - const source = this._contractSourcesIfExists[fileName]; + const source = this._contractSources[fileName]; const input = { [fileName]: source, }; @@ -217,7 +217,7 @@ export class Compiler { const unlinked_binary = `0x${compiled.contracts[contractIdentifier].bytecode}`; const updated_at = Date.now(); const contractNetworkData: ContractNetworkData = { - solc_version: contractSpecificSourceData.solc_version, + solc_version: contractSpecificSourceData.solcVersion, keccak256: sourceHash, source_tree_hash: sourceTreeHash, optimizer_enabled: this._optimizerEnabled, @@ -227,11 +227,11 @@ export class Compiler { }; let newArtifact: ContractArtifact; - if (!_.isUndefined(currentArtifact)) { + if (!_.isUndefined(currentArtifactIfExists)) { newArtifact = { - ...currentArtifact, + ...currentArtifactIfExists, networks: { - ...currentArtifact.networks, + ...currentArtifactIfExists.networks, [this._networkId]: contractNetworkData, }, }; @@ -253,28 +253,28 @@ export class Compiler { * Sets the source tree hash for a file and its dependencies. * @param fileName Name of contract file. */ - private _setSourceTreeHash(fileName: string) { + private _setSourceTreeHash(fileName: string): void { const contractSpecificSourceData = this._contractSourceData[fileName]; if (_.isUndefined(contractSpecificSourceData)) { throw new Error(`Contract data for ${fileName} not yet set`); } - if (_.isUndefined(contractSpecificSourceData.sourceTreeHash)) { + if (_.isUndefined(contractSpecificSourceData.sourceTreeHashIfExists)) { const dependencies = contractSpecificSourceData.dependencies; if (dependencies.length === 0) { - contractSpecificSourceData.sourceTreeHash = contractSpecificSourceData.sourceHash; + contractSpecificSourceData.sourceTreeHashIfExists = contractSpecificSourceData.sourceHash; } else { _.forEach(dependencies, dependency => { this._setSourceTreeHash(dependency); }); const dependencySourceTreeHashes = _.map( dependencies, - dependency => this._contractSourceData[dependency].sourceTreeHash, + dependency => this._contractSourceData[dependency].sourceTreeHashIfExists, ); const sourceTreeHashesBuffer = Buffer.concat([ contractSpecificSourceData.sourceHash, ...dependencySourceTreeHashes, ]); - contractSpecificSourceData.sourceTreeHash = ethUtil.sha3(sourceTreeHashesBuffer); + contractSpecificSourceData.sourceTreeHashIfExists = ethUtil.sha3(sourceTreeHashesBuffer); } } } @@ -285,11 +285,11 @@ export class Compiler { * @return Import contents object containing source code of dependency. */ private _findImportsIfSourcesExist(importPath: string): ImportContents { - if (_.isUndefined(this._contractSourcesIfExists)) { - throw new Error('Contract sources not yet initialized'); - } const fileName = path.basename(importPath); - const source = this._contractSourcesIfExists[fileName]; + const source = this._contractSources[fileName]; + if (_.isUndefined(source)) { + throw new Error(`Contract source not found for ${fileName}`); + } const importContents: ImportContents = { contents: source, }; @@ -309,7 +309,7 @@ export class Compiler { * @param fileName Name of contract file. * @return Contract data on network or undefined. */ - private async _getContractArtifactOrReturnAsync(fileName: string): Promise { + private async _getContractArtifactIfExistsAsync(fileName: string): Promise { let contractArtifact; const contractName = path.basename(fileName, constants.SOLIDITY_FILE_EXTENSION); const currentArtifactPath = `${this._artifactsDir}/${contractName}.json`; @@ -322,7 +322,7 @@ export class Compiler { return contractArtifact; } catch (err) { utils.consoleLog(`Artifact for ${fileName} does not exist`); - return contractArtifact; + return undefined; } } } diff --git a/packages/deployer/src/deployer.ts b/packages/deployer/src/deployer.ts index b14401050..021645fd1 100644 --- a/packages/deployer/src/deployer.ts +++ b/packages/deployer/src/deployer.ts @@ -35,9 +35,11 @@ export class Deployer { * @return Deployed contract instance. */ public async deployAsync(contractName: string, args: any[] = []): Promise { - const contractArtifact: ContractArtifact = this._loadContractArtifactIfExists(contractName); - const contractData: ContractNetworkData = this._getContractDataFromArtifactIfExists(contractArtifact); - const data = contractData.unlinked_binary; + const contractArtifactIfExists: ContractArtifact = this._loadContractArtifactIfExists(contractName); + const contractNetworkDataIfExists: ContractNetworkData = this._getContractNetworkDataFromArtifactIfExists( + contractArtifactIfExists, + ); + const data = contractNetworkDataIfExists.unlinked_binary; const from = await this._getFromAddressAsync(); const gas = await this._getAllowableGasEstimateAsync(data); const txData = { @@ -46,7 +48,7 @@ export class Deployer { data, gas, }; - const abi = contractData.abi; + const abi = contractNetworkDataIfExists.abi; const web3ContractInstance = await this._deployFromAbiAsync(abi, args, txData); utils.consoleLog(`${contractName}.sol successfully deployed at ${web3ContractInstance.address}`); const contractInstance = new Contract(web3ContractInstance, this._defaults); @@ -100,19 +102,21 @@ export class Deployer { contractAddress: string, args: any[], ): Promise { - const contractArtifact: ContractArtifact = this._loadContractArtifactIfExists(contractName); - const contractData: ContractNetworkData = this._getContractDataFromArtifactIfExists(contractArtifact); - const abi = contractData.abi; + const contractArtifactIfExists: ContractArtifact = this._loadContractArtifactIfExists(contractName); + const contractNetworkDataIfExists: ContractNetworkData = this._getContractNetworkDataFromArtifactIfExists( + contractArtifactIfExists, + ); + const abi = contractNetworkDataIfExists.abi; const encodedConstructorArgs = encoder.encodeConstructorArgsFromAbi(args, abi); const newContractData = { - ...contractData, + ...contractNetworkDataIfExists, address: contractAddress, constructor_args: encodedConstructorArgs, }; const newArtifact = { - ...contractArtifact, + ...contractArtifactIfExists, networks: { - ...contractArtifact.networks, + ...contractArtifactIfExists.networks, [this._networkId]: newContractData, }, }; @@ -139,12 +143,12 @@ export class Deployer { * @param contractArtifact The contract artifact. * @return Network specific contract data. */ - private _getContractDataFromArtifactIfExists(contractArtifact: ContractArtifact): ContractNetworkData { - const contractData = contractArtifact.networks[this._networkId]; - if (_.isUndefined(contractData)) { + private _getContractNetworkDataFromArtifactIfExists(contractArtifact: ContractArtifact): ContractNetworkData { + const contractNetworkDataIfExists = contractArtifact.networks[this._networkId]; + if (_.isUndefined(contractNetworkDataIfExists)) { throw new Error(`Data not found in artifact for contract: ${contractArtifact.contract_name}`); } - return contractData; + return contractNetworkDataIfExists; } /** * Gets the address to use for sending a transaction. diff --git a/packages/deployer/src/utils/types.ts b/packages/deployer/src/utils/types.ts index 166fc15dd..a3f722976 100644 --- a/packages/deployer/src/utils/types.ts +++ b/packages/deployer/src/utils/types.ts @@ -71,9 +71,9 @@ export interface ContractSourceData { export interface ContractSpecificSourceData { dependencies: string[]; - solc_version: string; + solcVersion: string; sourceHash: Buffer; - sourceTreeHash?: Buffer; + sourceTreeHashIfExists?: Buffer; } export interface ImportContents { -- cgit v1.2.3