diff --git a/packages/cli/src/commands/push.ts b/packages/cli/src/commands/push.ts index ade2b2268..2286f9744 100644 --- a/packages/cli/src/commands/push.ts +++ b/packages/cli/src/commands/push.ts @@ -3,6 +3,7 @@ import { ZWeb3 } from '@openzeppelin/upgrades'; import add from './add'; import push from '../scripts/push'; +import { PushParams } from '../scripts/interfaces'; import Session from '../models/network/Session'; import { compile } from '../models/compiler/Compiler'; import Dependency from '../models/dependency/Dependency'; @@ -42,6 +43,7 @@ async function commandActions(options: any): Promise { async function action(options: any): Promise { const { + contractAlias, force, deployDependencies, reset: reupload, @@ -66,7 +68,7 @@ async function action(options: any): Promise { }); const promptDeployDependencies = await promptForDeployDependencies(deployDependencies, network, interactive); - const pushArguments = { + const pushArguments: PushParams = { deployProxyAdmin, deployProxyFactory, force, @@ -76,7 +78,10 @@ async function action(options: any): Promise { ...promptDeployDependencies, }; - if (!options.skipTelemetry) await Telemetry.report('push', pushArguments, interactive); + if (contractAlias) pushArguments.contractAliases = [contractAlias]; + + if (!options.skipTelemetry) + await Telemetry.report('push', (pushArguments as unknown) as Record, interactive); await push(pushArguments); if (!options.dontExitProcess && process.env.NODE_ENV !== 'test') process.exit(0); } @@ -89,9 +94,9 @@ async function runActionIfRequested(externalOptions: any): Promise { return action(options); } -async function runActionIfNeeded(contractName: string, network: string, options: any): Promise { +async function runActionIfNeeded(contractAlias: string, network: string, options: any): Promise { if (!options.interactive) return; - await action({ ...options, dontExitProcess: true, skipTelemetry: true }); + await action({ ...options, dontExitProcess: true, skipTelemetry: true, contractAlias }); } async function promptForDeployDependencies( diff --git a/packages/cli/src/models/TestHelper.ts b/packages/cli/src/models/TestHelper.ts index 06c894d4c..2a24ce272 100644 --- a/packages/cli/src/models/TestHelper.ts +++ b/packages/cli/src/models/TestHelper.ts @@ -14,7 +14,7 @@ export default async function( ): Promise { const controller = new NetworkController('test', txParams, networkFile); await controller.deployDependencies(); - await controller.push(false, true); + await controller.push(undefined, { reupload: false, force: true }); return controller.project; } diff --git a/packages/cli/src/models/network/NetworkController.ts b/packages/cli/src/models/network/NetworkController.ts index 007283d27..042252df1 100644 --- a/packages/cli/src/models/network/NetworkController.ts +++ b/packages/cli/src/models/network/NetworkController.ts @@ -122,13 +122,13 @@ export default class NetworkController { } // DeployerController - public async push(reupload = false, force = false): Promise { + public async push(aliases: string[] | undefined, { reupload = false, force = false } = {}): Promise { const changedLibraries = this._solidityLibsForPush(!reupload); - const contracts = this._contractsListForPush(!reupload, changedLibraries); + const contractObjects = this._contractsListForPush(aliases, !reupload, changedLibraries); const buildArtifacts = getBuildArtifacts(); // ValidateContracts also extends each contract class with validation errors and storage info - if (!this.validateContracts(contracts, buildArtifacts) && !force) { + if (!this.validateContracts(contractObjects, buildArtifacts) && !force) { throw Error( 'One or more contracts have validation errors. Please review the items listed above and fix them, or run this command again with the --force option.', ); @@ -140,11 +140,11 @@ export default class NetworkController { this.checkNotFrozen(); await this.uploadSolidityLibs(changedLibraries); - await Promise.all([this.uploadContracts(contracts), this.unsetContracts()]); + await Promise.all([this.uploadContracts(contractObjects), this.unsetContracts()]); await this._unsetSolidityLibs(); - if (isEmpty(contracts) && isEmpty(changedLibraries)) { + if (isEmpty(contractObjects) && isEmpty(changedLibraries)) { Loggy.noSpin(__filename, 'push', `after-push`, `All contracts are up to date`); } else { Loggy.noSpin(__filename, 'push', `after-push`, `All contracts have been deployed`); @@ -179,10 +179,16 @@ export default class NetworkController { } // Contract model - private _contractsListForPush(onlyChanged = false, changedLibraries: Contract[] = []): [string, Contract][] { + private _contractsListForPush( + aliases: string[] | undefined, + onlyChanged = false, + changedLibraries: Contract[] = [], + ): [string, Contract][] { const newVersion = this._newVersionRequired(); - return toPairs(this.projectFile.contracts) + aliases = aliases || Object.keys(this.projectFile.contracts); + return aliases + .map(alias => [alias, this.projectFile.contracts[alias]]) .map(([contractAlias, contractName]): [string, Contract] => [contractAlias, Contracts.getFromLocal(contractName)]) .filter( ([contractAlias, contract]) => diff --git a/packages/cli/src/scripts/interfaces.ts b/packages/cli/src/scripts/interfaces.ts index 5dc3bb7b3..761504ce5 100644 --- a/packages/cli/src/scripts/interfaces.ts +++ b/packages/cli/src/scripts/interfaces.ts @@ -108,6 +108,7 @@ export interface UnpackParams { } export interface PushParams extends Network { + contractAliases?: string[]; force?: boolean; reupload?: boolean; deployDependencies?: boolean; diff --git a/packages/cli/src/scripts/push.ts b/packages/cli/src/scripts/push.ts index 4aa747837..eb3336b2e 100644 --- a/packages/cli/src/scripts/push.ts +++ b/packages/cli/src/scripts/push.ts @@ -2,6 +2,7 @@ import NetworkController from '../models/network/NetworkController'; import { PushParams } from './interfaces'; export default async function push({ + contractAliases, network, deployDependencies, deployProxyAdmin, @@ -17,7 +18,7 @@ export default async function push({ if (deployDependencies) await controller.deployDependencies(); if (deployProxyAdmin) await controller.deployProxyAdmin(); if (deployProxyFactory) await controller.deployProxyFactory(); - await controller.push(reupload, force); + await controller.push(contractAliases, { reupload, force }); const { appAddress } = controller; } finally { controller.writeNetworkPackageIfNeeded(); diff --git a/packages/cli/src/telemetry/index.ts b/packages/cli/src/telemetry/index.ts index b5ed761f6..a5febe6e2 100644 --- a/packages/cli/src/telemetry/index.ts +++ b/packages/cli/src/telemetry/index.ts @@ -33,7 +33,7 @@ export interface UserEnvironment { export default { DISABLE_TELEMETRY: !!process.env.OPENZEPPELIN_DISABLE_TELEMETRY, - async report(commandName: string, params: { [key: string]: unknown }, interactive: boolean): Promise { + async report(commandName: string, params: Record, interactive: boolean): Promise { const telemetryOptions = await checkOptIn(interactive); if (telemetryOptions === undefined || !telemetryOptions.optIn) return; diff --git a/packages/cli/test/scripts/push.test.js b/packages/cli/test/scripts/push.test.js index 6ca88eb69..90be9e832 100644 --- a/packages/cli/test/scripts/push.test.js +++ b/packages/cli/test/scripts/push.test.js @@ -1,7 +1,9 @@ 'use strict'; require('../setup'); -const zosLib = require('@openzeppelin/upgrades'); // eslint-disable-line @typescript-eslint/no-var-requires +import { expect } from 'chai'; + +const upgrades = require('@openzeppelin/upgrades'); // eslint-disable-line @typescript-eslint/no-var-requires import { ZWeb3, Contracts, App, Package, ProxyAdmin, ProxyFactory } from '@openzeppelin/upgrades'; import { accounts } from '@openzeppelin/test-environment'; @@ -169,7 +171,7 @@ describe('push script', function() { }); it('should refuse to redeploy a contract if validation throws', async function() { - sinon.stub(zosLib, 'validate').throws(new Error('Stubbed error during contract validation')); + sinon.stub(upgrades, 'validate').throws(new Error('Stubbed error during contract validation')); await push({ networkFile: this.networkFile, network, @@ -179,7 +181,7 @@ describe('push script', function() { }); it('should redeploy contract skipping errors', async function() { - sinon.stub(zosLib, 'validate').throws(new Error('Stubbed error during contract validation')); + sinon.stub(upgrades, 'validate').throws(new Error('Stubbed error during contract validation')); await push({ force: true, networkFile: this.networkFile, @@ -195,6 +197,152 @@ describe('push script', function() { }); }; + const shouldDeployOnlySpecifiedContracts = function() { + describe('when contracts specified explicitly', function() { + beforeEach('loading previous addresses', function() { + this.previousAddress = this.networkFile.contract('Impl').address; + this.withLibraryPreviousAddress = this.networkFile.contract('WithLibraryImpl').address; + }); + + describe('when a NetworkFile is empty', function() { + beforeEach('purging NetworkFile', function() { + this.networkFile.data.contracts = {}; + this.networkFile.data.proxies = {}; + }); + + it('should record contracts in network file', async function() { + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + + const contract = this.networkFile.contract('Impl'); + contract.address.should.be.nonzeroAddress; + contract.localBytecodeHash.should.not.be.empty; + contract.storage.should.not.be.empty; + contract.types.should.not.be.empty; + const deployed = await ImplV1.at(contract.address); + (await deployed.methods.say().call()).should.eq('V1'); + }); + + it('should not record not specified contracts in network file', async function() { + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + + const contract = this.networkFile.contract('WithLibraryImpl'); + expect(contract).to.be.undefined; + }); + + it('should deploy contract instance', async function() { + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + + const address = this.networkFile.contract('Impl').address; + const deployed = await ImplV1.at(address); + (await deployed.methods.say().call()).should.eq('V1'); + }); + + it('should deploy required libraries', async function() { + await push({ contractAliases: ['WithLibraryImpl'], networkFile: this.networkFile, network, txParams }); + + const address = this.networkFile.solidityLib('UintLib').address; + const code = await ZWeb3.eth.getCode(address); + const uintLib = Contracts.getFromLocal('UintLib'); + code.length.should.eq(uintLib.schema.deployedBytecode.length).and.be.greaterThan(40); + }); + + it('should deploy and link contracts that require libraries', async function() { + await push({ contractAliases: ['WithLibraryImpl'], networkFile: this.networkFile, network, txParams }); + + const address = this.networkFile.contract('WithLibraryImpl').address; + const deployed = await WithLibraryImplV1.at(address); + const result = await deployed.methods.double(10).call(); + result.should.eq('20'); + }); + }); + + describe('on a redeploy', function() { + it('should not deploy contracts if unmodified', async function() { + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + this.networkFile.contract('Impl').address.should.eq(this.previousAddress); + }); + + it('should deploy unmodified contract if forced', async function() { + await push({ + contractAliases: ['Impl'], + networkFile: this.networkFile, + network, + txParams, + reupload: true, + }); + this.networkFile.contract('Impl').address.should.not.eq(this.previousAddress); + }); + + it('should deploy contracts if modified', async function() { + modifyBytecode.call(this, 'Impl'); + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + this.networkFile.contract('Impl').address.should.not.eq(this.previousAddress); + }); + + it('should not deploy contracts if library is modified', async function() { + modifyLibraryBytecode.call(this, 'UintLib'); + await push({ contractAliases: ['Impl'], networkFile: this.networkFile, network, txParams }); + this.networkFile.contract('WithLibraryImpl').address.should.eq(this.withLibraryPreviousAddress); + }); + }); + + context('validations', function() { + beforeEach('modifying contracts', function() { + modifyBytecode.call(this, 'Impl'); + modifyStorageInfo.call(this, 'Impl'); + }); + + it('should refuse to deploy a contract if storage is incompatible', async function() { + await push({ + contractAliases: ['Impl'], + networkFile: this.networkFile, + network, + txParams, + }).should.be.rejectedWith(/have validation errors/); + this.networkFile.contract('Impl').address.should.eq(this.previousAddress); + }); + + it('should deploy contract ignoring warnings', async function() { + await push({ + contractAliases: ['Impl'], + force: true, + networkFile: this.networkFile, + network, + txParams, + }); + this.networkFile.contract('Impl').address.should.not.eq(this.previousAddress); + }); + + it('should refuse to deploy a contract if validation throws', async function() { + sinon.stub(upgrades, 'validate').throws(new Error('Stubbed error during contract validation')); + await push({ + contractAliases: ['Impl'], + networkFile: this.networkFile, + network, + txParams, + }).should.be.rejectedWith(/have validation errors/); + this.networkFile.contract('Impl').address.should.eq(this.previousAddress); + }); + + it('should deploy contract skipping errors', async function() { + sinon.stub(upgrades, 'validate').throws(new Error('Stubbed error during contract validation')); + await push({ + contractAliases: ['Impl'], + force: true, + networkFile: this.networkFile, + network, + txParams, + }); + this.networkFile.contract('Impl').address.should.not.eq(this.previousAddress); + }); + + afterEach(function() { + sinon.restore(); + }); + }); + }); + }; + const shouldValidateContracts = function() { describe('validations', function() { beforeEach('capturing log output', function() { @@ -531,6 +679,7 @@ describe('push script', function() { shouldDeployApp(); shouldDeployProvider(); shouldDeployContracts(); + shouldDeployOnlySpecifiedContracts(); shouldRegisterContractsInDirectory(); shouldRedeployContracts(); shouldValidateContracts(); @@ -687,6 +836,7 @@ describe('push script', function() { }); shouldDeployContracts(); + shouldDeployOnlySpecifiedContracts(); shouldValidateContracts(); shouldRedeployContracts(); shouldDeleteContracts({ unregisterFromDirectory: false });