Push only required contract during create command (#1426)

* Push only required contract during create command

* Refactor NetworkController#push

* Add push tests

* Add push tests

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<void> {
 
 async function action(options: any): Promise<void> {
   const {
+    contractAlias,
     force,
     deployDependencies,
     reset: reupload,
@@ -66,7 +68,7 @@ async function action(options: any): Promise<void> {
   });
   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<void> {
     ...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<string, unknown>, 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<void> {
   return action(options);
 }
 
-async function runActionIfNeeded(contractName: string, network: string, options: any): Promise<void> {
+async function runActionIfNeeded(contractAlias: string, network: string, options: any): Promise<void> {
   if (!options.interactive) return;
-  await action({ ...options, dontExitProcess: true, skipTelemetry: true });
+  await action({ ...options, dontExitProcess: true, skipTelemetry: true, contractAlias });
 }
 
 async function promptForDeployDependencies(

packages/cli/src/models/TestHelper.ts

@@ -14,7 +14,7 @@ export default async function(
 ): Promise<ProxyAdminProject | AppProject> {
   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;
 }

packages/cli/src/models/network/NetworkController.ts

@@ -122,13 +122,13 @@ export default class NetworkController {
   }
 
   // DeployerController
-  public async push(reupload = false, force = false): Promise<void | never> {
+  public async push(aliases: string[] | undefined, { reupload = false, force = false } = {}): Promise<void | never> {
     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]) =>

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;

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();

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<void> {
+  async report(commandName: string, params: Record<string, unknown>, interactive: boolean): Promise<void> {
     const telemetryOptions = await checkOptIn(interactive);
     if (telemetryOptions === undefined || !telemetryOptions.optIn) return;
 

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 });