Skip to content

Commit

Permalink
Removes web3 from the owner script (#1336)
Browse files Browse the repository at this point in the history
* Using owner script in integration tests

* Removed web3 from owner script

* Manually setting isContract in owner script
  • Loading branch information
eternauta1337 authored Jun 15, 2021
1 parent 024675e commit 5b63890
Show file tree
Hide file tree
Showing 10 changed files with 207 additions and 137 deletions.
1 change: 0 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions publish/src/commands/nominate.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,7 @@ const nominate = async ({
privateKey,
providerUrl,
yes,
quiet,
}) => {
if (quiet) {
console.log = () => {};
}

ensureNetwork(network);
deploymentPath = deploymentPath || getDeploymentPathForNetwork({ network, useOvm });
ensureDeploymentPath(deploymentPath);
Expand Down Expand Up @@ -178,7 +173,6 @@ module.exports = {
'-v, --private-key [value]',
'The private key to deploy with (only works in local mode, otherwise set in .env).'
)
.option('--quiet', 'Dont print logs.')
.option('-y, --yes', 'Dont prompt, just reply yes.')
.option(
'-c, --contracts [value]',
Expand Down
94 changes: 53 additions & 41 deletions publish/src/commands/owner.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
'use strict';

const ethers = require('ethers');
const fs = require('fs');
const { gray, yellow, red, cyan, bgYellow, black } = require('chalk');
const w3utils = require('web3-utils');
const Web3 = require('web3');

const {
getUsers,
Expand Down Expand Up @@ -45,6 +44,8 @@ const owner = async ({
yes,
useOvm,
providerUrl,
useFork,
isContract,
}) => {
ensureNetwork(network);
deploymentPath = deploymentPath || getDeploymentPathForNetwork({ network, useOvm });
Expand All @@ -58,7 +59,7 @@ const owner = async ({
newOwner = getUsers({ network, useOvm, user: 'owner' }).address;
}

if (!w3utils.isAddress(newOwner)) {
if (!ethers.utils.isAddress(newOwner)) {
console.error(red('Invalid new owner to nominate. Please check the option and try again.'));
process.exit(1);
} else {
Expand All @@ -72,7 +73,9 @@ const owner = async ({

const { providerUrl: envProviderUrl, privateKey: envPrivateKey } = loadConnections({
network,
useFork,
});

if (!providerUrl) {
if (!envProviderUrl) {
throw new Error('Missing .env key of PROVIDER_URL. Please add and retry.');
Expand All @@ -85,10 +88,8 @@ const owner = async ({
privateKey = envPrivateKey;
}

const web3 = new Web3(new Web3.providers.HttpProvider(providerUrl));
const provider = new ethers.providers.JsonRpcProvider(providerUrl);

const code = await web3.eth.getCode(newOwner);
const isContract = code !== '0x';
if (!isContract && !yes) {
try {
await confirmAction(
Expand All @@ -102,13 +103,18 @@ const owner = async ({
}
}

web3.eth.accounts.wallet.add(privateKey);
const account = web3.eth.accounts.wallet[0].address;
console.log(gray(`Using account with public key ${account}`));
let wallet;
if (useFork) {
const account = getUsers({ network, user: 'owner' }).address; // protocolDAO
wallet = provider.getSigner(account);
} else {
wallet = new ethers.Wallet(privateKey, provider);
}
console.log(gray(`Using account with public key ${wallet.address}`));

if (!isContract && account.toLowerCase() !== newOwner.toLowerCase()) {
if (!isContract && wallet.address.toLowerCase() !== newOwner.toLowerCase()) {
throw new Error(
`New owner is ${newOwner} and signer is ${account}. The signer needs to be the new owner in order to be able to claim ownership and/or execute owner actions.`
`New owner is ${newOwner} and signer is ${wallet.address}. The signer needs to be the new owner in order to be able to claim ownership and/or execute owner actions.`
);
}

Expand All @@ -119,7 +125,7 @@ const owner = async ({
let currentSafeNonce;
if (isContract) {
// new owner should be gnosis safe proxy address
protocolDaoContract = getSafeInstance(web3, newOwner);
protocolDaoContract = getSafeInstance(providerUrl, newOwner);

// get protocolDAO nonce
currentSafeNonce = await getSafeNonce(protocolDaoContract);
Expand All @@ -129,9 +135,7 @@ const owner = async ({
process.exit();
}

console.log(
yellow(`Using Protocol DAO Safe contract at ${protocolDaoContract.options.address}`)
);
console.log(yellow(`Using Protocol DAO Safe contract at ${protocolDaoContract.address}`));
}

const confirmOrEnd = async message => {
Expand Down Expand Up @@ -159,7 +163,7 @@ const owner = async ({
// Load staged transactions
stagedTransactions = await getSafeTransactions({
network,
safeAddress: protocolDaoContract.options.address,
safeAddress: protocolDaoContract.address,
});
}

Expand Down Expand Up @@ -191,14 +195,15 @@ const owner = async ({
safeContract: protocolDaoContract,
data,
to: target,
sender: account,
sender: wallet.address,
network,
lastNonce,
});

// sign txHash to get signature
const sig = getSafeSignature({
signer: web3.eth.accounts.wallet[0],
privateKey,
providerUrl,
contractTxHash: txHash,
});

Expand All @@ -209,23 +214,23 @@ const owner = async ({
data,
nonce: newNonce,
to: target,
sender: account,
sender: wallet.address,
transactionHash: txHash,
signature: sig,
});

// track lastNonce submitted
lastNonce = newNonce;
} else {
const tx = await web3.eth.sendTransaction({
from: account,
const tx = await wallet.sendTransaction({
to: target,
gasPrice: w3utils.toWei(gasPrice, 'gwei'),
gas: gasLimit,
gasPrice: ethers.utils.parseUnits(gasPrice, 'gwei'),
gasLimit: ethers.BigNumber.from(gasLimit),
data,
});
const receipt = await tx.wait();

logTx(tx);
logTx(receipt);
}

entry.complete = true;
Expand All @@ -242,25 +247,25 @@ const owner = async ({
for (const contract of Object.keys(config)) {
const { address, source } = deployment.targets[contract];
const { abi } = deployment.sources[source];
const deployedContract = new web3.eth.Contract(abi, address);
const deployedContract = new ethers.Contract(address, abi, provider);

// ignore contracts that don't support Owned
if (!deployedContract.methods.owner) {
if (!deployedContract.functions.owner) {
continue;
}
const currentOwner = (await deployedContract.methods.owner().call()).toLowerCase();
const nominatedOwner = (await deployedContract.methods.nominatedOwner().call()).toLowerCase();
const currentOwner = (await deployedContract.owner()).toLowerCase();
const nominatedOwner = (await deployedContract.nominatedOwner()).toLowerCase();

if (currentOwner === newOwner) {
console.log(gray(`${newOwner} is already the owner of ${contract}`));
} else if (nominatedOwner === newOwner) {
const encodedData = deployedContract.methods.acceptOwnership().encodeABI();
const encodedData = deployedContract.interface.encodeFunctionData('acceptOwnership', []);

if (isContract) {
// Check if similar one already staged and pending
const existingTx = checkExistingPendingTx({
stagedTransactions,
target: deployedContract.options.address,
target: deployedContract.address,
encodedData,
currentSafeNonce,
});
Expand All @@ -279,15 +284,16 @@ const owner = async ({
const { txHash, newNonce } = await getNewTransactionHash({
safeContract: protocolDaoContract,
data: encodedData,
to: deployedContract.options.address,
sender: account,
to: deployedContract.address,
sender: wallet.address,
network,
lastNonce,
});

// sign txHash to get signature
const sig = getSafeSignature({
signer: web3.eth.accounts.wallet[0],
privateKey,
providerUrl,
contractTxHash: txHash,
});

Expand All @@ -297,24 +303,24 @@ const owner = async ({
network,
data: encodedData,
nonce: newNonce,
to: deployedContract.options.address,
sender: account,
to: deployedContract.address,
sender: wallet.address,
transactionHash: txHash,
signature: sig,
});

// track lastNonce submitted
lastNonce = newNonce;
} else {
const tx = await web3.eth.sendTransaction({
from: account,
to: deployedContract.options.address,
gasPrice: w3utils.toWei(gasPrice, 'gwei'),
gas: gasLimit,
const tx = await wallet.sendTransaction({
to: deployedContract.address,
gasPrice: ethers.utils.parseUnits(gasPrice, 'gwei'),
gasLimit: ethers.BigNumber.from(gasLimit),
data: encodedData,
});
const receipt = await tx.wait();

logTx(tx);
logTx(receipt);
}
} catch (err) {
console.log(
Expand Down Expand Up @@ -346,6 +352,12 @@ module.exports = {
'-o, --new-owner <value>',
'The address of protocolDAO proxy contract as owner (please include the 0x prefix)'
)
.option(
'-k, --use-fork',
'Perform the deployment on a forked chain running on localhost (see fork command).',
false
)
.option('--is-contract', 'Wether the new owner is a contract wallet or an EOA', false)
.option('-v, --private-key [value]', 'The private key of wallet to stage with.')
.option('-g, --gas-price <value>', 'Gas price in GWEI', DEFAULTS.gasPrice)
.option('-l, --gas-limit <value>', 'Gas limit', parseInt, DEFAULTS.gasLimit)
Expand Down
15 changes: 13 additions & 2 deletions publish/src/safe-utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const Web3 = require('web3');
const w3utils = require('web3-utils');
const axios = require('axios');
const { green, gray, red, yellow } = require('chalk');
Expand Down Expand Up @@ -63,7 +64,11 @@ const safeTransactionApi = ({ network, safeAddress }) => {
return `https://safe-transaction.${network}.gnosis.io/api/v1/safes/${address}/multisig-transactions/`;
};

const getSafeInstance = (web3, safeAddress) => {
const getSafeInstance = (web3, providerUrl, safeAddress) => {
if (!web3) {
web3 = new Web3(new Web3.providers.HttpProvider(providerUrl));
}

return new web3.eth.Contract(abi, safeAddress);
};

Expand Down Expand Up @@ -239,7 +244,13 @@ const getNewTransactionHash = async ({ safeContract, data, to, sender, network,
return { txHash, newNonce };
};

const getSafeSignature = ({ signer, contractTxHash }) => {
const getSafeSignature = ({ signer, privateKey, providerUrl, contractTxHash }) => {
if (!signer) {
const web3 = new Web3(new Web3.providers.HttpProvider(providerUrl));

signer = web3.eth.accounts.wallet.add(privateKey);
}

// sign txHash to get signature
const { signature } = signer.sign(contractTxHash);

Expand Down
82 changes: 0 additions & 82 deletions test/integration/behaviors/nominate.behavior.js

This file was deleted.

Loading

0 comments on commit 5b63890

Please sign in to comment.