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.

Sign up for GitHub

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

Jump to bottom

Add arg configuration to proxyAdmin and allow name to act as artifact #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add arg configuration to proxyAdmin and allow name to act as artifact #142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4240b67
Add arg configuration to proxyAdmin and allow name to act as artifact
outdoteth Jun 25, 2021
0200d4b
Remove log
outdoteth Jul 12, 2021
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
5 changes: 3 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,8 +1080,9 @@ type ProxyOptionsBase = {
viaAdminContract?: // allow to specify a contract that act as a middle man to perform upgrades. Useful and Recommended for Transparent Proxies
| string
| {
name: string;
artifact?: string | ArtifactData;
name: string; // Custom name - If no artifact name is specified, it will also act as the artifact name
artifact?: string | ArtifactData; // Custom artifact name
args?: any[]; // Arguments that will be passed to the AdminContract constructor
};
};

Expand Down
25 changes: 21 additions & 4 deletions src/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,11 @@ export function addHelpers(
let checkABIConflict = true;
let viaAdminContract:
| string
| {name: string; artifact?: string | ArtifactData}
| {
name: string;
artifact?: string | ArtifactData;
args?: any[];
}
| undefined;
if (typeof options.proxy === 'object') {
upgradeIndex = options.proxy.upgradeIndex;
Expand Down Expand Up @@ -1065,9 +1069,16 @@ Note that in this case, the contract deployment will not behave the same if depl
} else {
proxyAdminName = viaAdminContract.name;
if (!viaAdminContract.artifact) {
proxyAdminDeployed = await partialExtension.get(proxyAdminName);
try {
proxyAdminDeployed = await partialExtension.get(proxyAdminName);
} catch (e) {
console.log(
Copy link
Owner

Choose a reason for hiding this comment

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

it should not be a console.log as this would show up everywhere including test output

Copy link
Author

Choose a reason for hiding this comment

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

Ok I removed the log completely as I see you have done in here:

Screenshot 2021-07-12 at 13 51 51

Copy link
Author

Choose a reason for hiding this comment

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

@wighawag just pinging.

Copy link
Owner

Choose a reason for hiding this comment

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

Hi Dylan, sorry did not find the time yet to fully review it, but in principle it looks good. Hopefully I ll find some time before I am back from my travel.

`ProxyAdmin deploy not found - Deploying new ProxyAdmin: ${proxyAdminName}`
);
}
}
proxyAdminArtifactNameOrContract = viaAdminContract.artifact;
proxyAdminArtifactNameOrContract =
viaAdminContract.artifact || viaAdminContract.name;
}

let proxyAdminContract: ExtendedArtifact | undefined;
Expand All @@ -1092,6 +1103,12 @@ Note that in this case, the contract deployment will not behave the same if depl
}

if (!proxyAdminDeployed) {
const args =
typeof options?.proxy === 'object' &&
typeof options.proxy.viaAdminContract === 'object'
? options.proxy.viaAdminContract?.args || [owner]
: [owner];

proxyAdminDeployed = await _deployOne(proxyAdminName, {
from: options.from,
autoMine: options.autoMine,
Expand All @@ -1102,7 +1119,7 @@ Note that in this case, the contract deployment will not behave the same if depl
contract: proxyAdminContract,
deterministicDeployment: options.deterministicDeployment,
skipIfAlreadyDeployed: true,
args: [owner],
args,
});
}

Expand Down
1 change: 1 addition & 0 deletions types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ type ProxyOptionsBase = {
| {
name: string;
artifact?: string | ArtifactData;
args: any[];
};
};

Expand Down