Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Push only required contract during create command #1426

Merged
merged 4 commits into from
Feb 7, 2020
Merged

Push only required contract during create command #1426

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97cafff
Push only required contract during create command
ylv-io Feb 6, 2020
3a88323
Refactor NetworkController#push
ylv-io Feb 7, 2020
ec099d1
Add push tests
ylv-io Feb 7, 2020
f2284fc
Add push tests
ylv-io Feb 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/cli/src/commands/push.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -42,6 +43,7 @@ async function commandActions(options: any): Promise<void> {

async function action(options: any): Promise<void> {
const {
contractAlias,
force,
deployDependencies,
reset: reupload,
Expand All @@ -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,
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious: why was this cast needed when changing from { [key: string]: unknown } to Record<string, unknown> in the report method definition? I thought that both were semantically the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast is needed not because of the change of report method but because of change pushParams to type PushParams which doesn't have required index signature.

await push(pushArguments);
if (!options.dontExitProcess && process.env.NODE_ENV !== 'test') process.exit(0);
}
Expand All @@ -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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contractName was not even used, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

if (!options.interactive) return;
await action({ ...options, dontExitProcess: true, skipTelemetry: true });
await action({ ...options, dontExitProcess: true, skipTelemetry: true, contractAlias });
}

async function promptForDeployDependencies(
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/models/TestHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
21 changes: 14 additions & 7 deletions packages/cli/src/models/network/NetworkController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,13 @@ export default class NetworkController {
}

// DeployerController
public async push(reupload = false, force = false): Promise<void | never> {
public async push(aliases: string[], { reupload = false, force = false } = {}): Promise<void | never> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aliases should be string[] | undefined, and the same in _contractsListForPush. It's not necessary because we're not using strict mode yet, but I'd like us to be explicit about it so it will be easier to migrate to strict mode in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I was thinking there are all the null checks. I completely forgot about that our tsconfig is not in a strict mode. We should bump it to it as fast as possible.

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.',
);
Expand All @@ -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`);
Expand Down Expand Up @@ -179,10 +179,17 @@ export default class NetworkController {
}

// Contract model
private _contractsListForPush(onlyChanged = false, changedLibraries: Contract[] = []): [string, Contract][] {
private _contractsListForPush(
aliases: string[],
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]) =>
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/scripts/interfaces.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ export interface UnpackParams {
}

export interface PushParams extends Network {
contractAliases?: string[];
force?: boolean;
reupload?: boolean;
deployDependencies?: boolean;
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/scripts/push.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import NetworkController from '../models/network/NetworkController';
import { PushParams } from './interfaces';

export default async function push({
contractAliases,
network,
deployDependencies,
deployProxyAdmin,
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/telemetry/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down