Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Defender deployments to use Defender SDK #894

Merged
merged 7 commits into from
Oct 12, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
4 changes: 4 additions & 0 deletions packages/plugin-hardhat/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- Update OpenZeppelin Defender deployments to use Defender SDK ([#888](https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/888))

## 2.3.2 (2023-10-11)

- Fix Hardhat compile error when using Solidity 0.5.x. ([#892](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/892))
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-hardhat/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"dependencies": {
"@openzeppelin/defender-admin-client": "^1.48.0",
"@openzeppelin/defender-base-client": "^1.48.0",
Comment on lines 39 to 40
Copy link
Member

Choose a reason for hiding this comment

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

Are we still actively using these? I see even proposals are included in the new SDK but don't know about the status what are the Defender libraries we kept.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are used for Defender v1 and are not deprecated yet, so we can keep them for now.

"@openzeppelin/platform-deploy-client": "^0.10.0",
"@openzeppelin/defender-sdk-deploy-client": "^1.2.0",
"@openzeppelin/upgrades-core": "^1.30.1",
"chalk": "^4.1.0",
"debug": "^4.1.1",
Expand Down
8 changes: 4 additions & 4 deletions packages/plugin-hardhat/src/defender/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { CompilerInput, CompilerOutputContract, HardhatRuntimeEnvironment } from

import { parseFullyQualifiedName } from 'hardhat/utils/contract-names';

import { DeploymentResponse, SourceCodeLicense } from '@openzeppelin/platform-deploy-client';
import { DeploymentResponse, SourceCodeLicense } from '@openzeppelin/defender-sdk-deploy-client';
import {
Deployment,
RemoteDeploymentId,
Expand All @@ -19,7 +19,7 @@ import UpgradeableBeacon from '@openzeppelin/upgrades-core/artifacts/@openzeppel
import TransparentUpgradeableProxy from '@openzeppelin/upgrades-core/artifacts/@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol/TransparentUpgradeableProxy.json';
import ProxyAdmin from '@openzeppelin/upgrades-core/artifacts/@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol/ProxyAdmin.json';

import { getNetwork, getDefenderClient } from './utils';
import { getNetwork, getDeployClient } from './utils';
import { DeployTransaction, DefenderDeployOptions, UpgradeOptions } from '../utils';
import debug from '../utils/debug';
import { getDeployData } from '../utils/deploy-impl';
Expand Down Expand Up @@ -60,7 +60,7 @@ export async function defenderDeploy(
opts: UpgradeOptions & DefenderDeployOptions,
...args: unknown[]
): Promise<Required<Deployment & RemoteDeploymentId> & DeployTransaction> {
const client = getDefenderClient(hre);
const client = getDeployClient(hre);

const constructorArgs = [...args] as (string | number | boolean)[];
const contractInfo = await getContractInfo(hre, factory, { constructorArgs, ...opts });
Expand All @@ -82,7 +82,7 @@ export async function defenderDeploy(

let deploymentResponse: DeploymentResponse;
try {
deploymentResponse = await client.Deployment.deploy({
deploymentResponse = await client.deployContract({
contractName: contractInfo.contractName,
contractPath: contractInfo.sourceName,
network: network,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { HardhatRuntimeEnvironment } from 'hardhat/types';

import { getNetwork, getDefenderClient } from './utils';
import { getNetwork, getDeployClient } from './utils';

export interface ApprovalProcess {
approvalProcessId: string;
Expand All @@ -11,10 +11,10 @@ export type GetDefaultApprovalProcessFunction = () => Promise<ApprovalProcess>;

export function makeGetDefaultApprovalProcess(hre: HardhatRuntimeEnvironment): GetDefaultApprovalProcessFunction {
return async function getDefaultApprovalProcess() {
const client = getDefenderClient(hre);
const client = getDeployClient(hre);
const network = await getNetwork(hre);

const response = await client.Upgrade.getApprovalProcess(network);
const response = await client.getUpgradeApprovalProcess(network);

if (response.network !== network) {
// This should not happen
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { ContractFactory, ethers } from 'ethers';
import { HardhatRuntimeEnvironment } from 'hardhat/types';
import { DefenderDeployOptions, UpgradeOptions } from '../utils';
import { getNetwork, enableDefender, getDefenderClient } from './utils';
import { getNetwork, enableDefender, getDeployClient } from './utils';
import { deployImplForUpgrade } from '../prepare-upgrade';

export interface UpgradeProposalResponse {
Expand All @@ -34,7 +34,7 @@ export function makeProposeUpgradeWithApproval(
return async function proposeUpgradeWithApproval(proxyAddress, contractNameOrImplFactory, opts = {}) {
opts = enableDefender(hre, defenderModule, opts);

const client = getDefenderClient(hre);
const client = getDeployClient(hre);
const network = await getNetwork(hre);

if (await isBeaconProxy(hre.network.provider, proxyAddress)) {
Expand Down Expand Up @@ -64,7 +64,7 @@ export function makeProposeUpgradeWithApproval(
const txResponse = deployedImpl.txResponse;
const newImplementation = deployedImpl.impl;

const upgradeProposalResponse = await client.Upgrade.upgrade({
const upgradeProposalResponse = await client.upgradeContract({
proxyAddress: proxyAddress,
proxyAdminAddress: proxyAdmin,
newImplementationABI: abi,
Expand Down
23 changes: 5 additions & 18 deletions packages/plugin-hardhat/src/defender/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ import {
} from '@openzeppelin/upgrades-core';

import { Network, fromChainId } from '@openzeppelin/defender-base-client';
import {
BlockExplorerApiKeyClient,
DeploymentClient,
DeploymentConfigClient,
PlatformClient,
UpgradeClient,
} from '@openzeppelin/platform-deploy-client';
import { DeployClient } from '@openzeppelin/defender-sdk-deploy-client';

import { HardhatDefenderConfig } from '../type-extensions';
import { DefenderDeploy } from '../utils';
Expand Down Expand Up @@ -92,15 +86,8 @@ export function disableDefender(
}
}

interface DefenderClient {
Deployment: DeploymentClient;
DeploymentConfig: DeploymentConfigClient;
BlockExplorerApiKey: BlockExplorerApiKeyClient;
Upgrade: UpgradeClient;
}

export function getDefenderClient(hre: HardhatRuntimeEnvironment): DefenderClient {
return PlatformClient(getDefenderApiKey(hre));
export function getDeployClient(hre: HardhatRuntimeEnvironment): DeployClient {
return new DeployClient(getDefenderApiKey(hre));
}

/**
Expand All @@ -115,9 +102,9 @@ export async function getRemoteDeployment(
hre: HardhatRuntimeEnvironment,
remoteDeploymentId: string,
): Promise<RemoteDeployment | undefined> {
const client = getDefenderClient(hre);
const client = getDeployClient(hre);
try {
return (await client.Deployment.get(remoteDeploymentId)) as RemoteDeployment;
return (await client.getDeployedContract(remoteDeploymentId)) as RemoteDeployment;
} catch (e) {
const message = (e as any).response?.data?.message;
if (message?.match(/deployment with id .* not found\./)) {
Expand Down
26 changes: 8 additions & 18 deletions packages/plugin-hardhat/test/defender-deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,21 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Deployment: {
deploy: () => {
return {
txHash: TX_HASH,
deploymentId: DEPLOYMENT_ID,
address: ADDRESS,
};
},
},
DeploymentConfig: {},
BlockExplorerApiKey: {
list: () => [{ network: t.context.fakeChainId }],
create: () => {
return;
},
deployContract: () => {
return {
txHash: TX_HASH,
deploymentId: DEPLOYMENT_ID,
address: ADDRESS,
};
},
};
t.context.spy = sinon.spy(t.context.fakeDefenderClient.Deployment, 'deploy');
t.context.spy = sinon.spy(t.context.fakeDefenderClient, 'deployContract');

t.context.deploy = proxyquire('../dist/defender/deploy', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getAdminClient: () => t.context.fakeAdminClient,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
'../utils/etherscan-api': {
getEtherscanAPIConfig: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Upgrade: {
getApprovalProcess: sinon.stub(),
},
getUpgradeApprovalProcess: sinon.stub(),
};

t.context.getDefaultApprovalProcess = proxyquire('../dist/defender/get-default-approval-process', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
}).makeGetDefaultApprovalProcess(hre);
});
Expand All @@ -32,7 +30,7 @@ test.afterEach.always(() => {
test('get default approval process', async t => {
const { fakeChainId, fakeDefenderClient, getDefaultApprovalProcess } = t.context;

fakeDefenderClient.Upgrade.getApprovalProcess.returns({
fakeDefenderClient.getUpgradeApprovalProcess.returns({
approvalProcessId: APPROVAL_PROCESS_ID,
via: MULTISIG_ADDRESS,
network: fakeChainId,
Expand All @@ -44,13 +42,13 @@ test('get default approval process', async t => {
address: MULTISIG_ADDRESS,
});

sinon.assert.calledWithExactly(fakeDefenderClient.Upgrade.getApprovalProcess, fakeChainId);
sinon.assert.calledWithExactly(fakeDefenderClient.getUpgradeApprovalProcess, fakeChainId);
});

test('get default approval process - wrong network returned', async t => {
const { fakeDefenderClient, getDefaultApprovalProcess } = t.context;

fakeDefenderClient.Upgrade.getApprovalProcess.returns({
fakeDefenderClient.getUpgradeApprovalProcess.returns({
approvalProcessId: APPROVAL_PROCESS_ID,
via: MULTISIG_ADDRESS,
network: 'sepolia',
Expand All @@ -64,7 +62,7 @@ test('get default approval process - wrong network returned', async t => {
test('get default approval process - no address', async t => {
const { fakeChainId, fakeDefenderClient, getDefaultApprovalProcess } = t.context;

fakeDefenderClient.Upgrade.getApprovalProcess.returns({
fakeDefenderClient.getUpgradeApprovalProcess.returns({
approvalProcessId: APPROVAL_PROCESS_ID,
network: fakeChainId,
});
Expand Down
6 changes: 3 additions & 3 deletions packages/plugin-hardhat/test/defender-utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const test = require('ava');
const sinon = require('sinon');
const { getNetwork, getDefenderClient, disableDefender, enableDefender } = require('../dist/defender/utils');
const { getNetwork, disableDefender, enableDefender, getDeployClient } = require('../dist/defender/utils');

test.beforeEach(async t => {
t.context.fakeChainId = '0x05';
Expand Down Expand Up @@ -29,14 +29,14 @@ test('fails if chain id is not accepted', async t => {

test('fails if defender config is missing', async t => {
delete t.context.fakeHre.config.defender;
t.throws(() => getDefenderClient(t.context.fakeHre), {
t.throws(() => getDeployClient(t.context.fakeHre), {
message: /Missing OpenZeppelin Defender API key and secret in hardhat config/,
});
});

test('fails if defender api key is missing in config', async t => {
delete t.context.fakeHre.config.defender.apiKey;
t.throws(() => getDefenderClient(t.context.fakeHre), {
t.throws(() => getDeployClient(t.context.fakeHre), {
message: /Missing OpenZeppelin Defender API key and secret in hardhat config/,
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Upgrade: {
upgrade: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
upgradeContract: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
};

t.context.spy = sinon.spy(t.context.fakeDefenderClient.Upgrade, 'upgrade');
t.context.spy = sinon.spy(t.context.fakeDefenderClient, 'upgradeContract');

t.context.proposeUpgradeWithApproval = proxyquire('../dist/defender/propose-upgrade-with-approval', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
}).makeProposeUpgradeWithApproval(hre);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,22 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Upgrade: {
upgrade: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
upgradeContract: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
};

t.context.spy = sinon.spy(t.context.fakeDefenderClient.Upgrade, 'upgrade');
t.context.spy = sinon.spy(t.context.fakeDefenderClient, 'upgradeContract');

t.context.proposeUpgradeWithApproval = proxyquire('../dist/defender/propose-upgrade-with-approval', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
}).makeProposeUpgradeWithApproval(hre);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Upgrade: {
upgrade: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
upgradeContract: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
};

t.context.spy = sinon.spy(t.context.fakeDefenderClient.Upgrade, 'upgrade');
t.context.spy = sinon.spy(t.context.fakeDefenderClient, 'upgradeContract');

t.context.proposeUpgradeWithApproval = proxyquire('../dist/defender/propose-upgrade-with-approval', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
}).makeProposeUpgradeWithApproval(hre);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,22 @@ test.beforeEach(async t => {
t.context.fakeChainId = 'goerli';

t.context.fakeDefenderClient = {
Upgrade: {
upgrade: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
upgradeContract: () => {
return {
proposalId: proposalId,
externalUrl: proposalUrl,
transaction: {},
};
},
};

t.context.spy = sinon.spy(t.context.fakeDefenderClient.Upgrade, 'upgrade');
t.context.spy = sinon.spy(t.context.fakeDefenderClient, 'upgradeContract');

t.context.proposeUpgradeWithApproval = proxyquire('../dist/defender/propose-upgrade-with-approval', {
'./utils': {
...require('../dist/defender/utils'),
getNetwork: () => t.context.fakeChainId,
getDefenderClient: () => t.context.fakeDefenderClient,
getDeployClient: () => t.context.fakeDefenderClient,
},
}).makeProposeUpgradeWithApproval(hre);

Expand Down
Loading