Deployed all contracts and verified to Mainnet #371

stevieraykatz · 2023-09-08T03:18:36Z

Explanation of the solution

Instructions on making this work

run yarn or yarn install to install npm dependencies

linear · 2023-09-08T03:18:38Z

stevieraykatz · 2023-09-08T03:21:27Z

utils/deploy.ts

@misicnenad I added this dumb delay to avoid some odd race conditions I was hitting on mainnet.

My leading theory is that Alchemy doesn't know about the new published block at the time that we submit the transcation to link the proxy to the implementation. This causes the proxy deployment to fail because the "target address isn't a contract". I was also sporadically running into other odd errors that weren't repeatable further hinting at race conditions.

This delay fixed all of the inconsistencies but I also know that it's not the cleanest solution. Open to any and all permanent suggestions.

stevieraykatz · 2023-09-08T03:22:53Z

hardhat.config.ts

+      gasPrice: 120*10**9,
+      gas: 15*10**6,


This was necessary since gas prices were fluctuating wildly.

And now I think that we may want to define them dynamically so that we can accommodate network conditions when we're about to run tasks on Mainnet

This won't work, see #371 (comment)

stevieraykatz · 2023-09-08T04:12:28Z

tasks/helpers/submitMultiSigTx.ts

+  owner: SignerWithAddress | Wallet,
  destination: string,
  data: BytesLike
 ): Promise<boolean> {
  logger.out(`Submitting transaction to Multisig at address: ${msAddress}...`);
  const multisig = IMultiSigGeneric__factory.connect(msAddress, owner);
-  const tx = await multisig.submitTransaction(destination, 0, data, "0x");
+  const tx = await multisig.submitTransaction(destination, 0, data, "0x", {gasPrice: 120*10**9});


This was necessary since Hardhat throws Account ... is not managed by the node you are connected to in its current form on Mainnet. However, we don't need the signer to be a SignerWithAddress to submit txs. Luckily this works! But the manual/static gas price is gross. Open to suggestions or I will clean it up tomorrow to match the hardhat network config opts.

Seems that Hardhat does not support setting project-wide gas configurations when running with ethers, see:
https://hardhat.org/hardhat-network/docs/reference#gas
NomicFoundation/hardhat#2406

There's an open issue to make this possible NomicFoundation/hardhat#3496

This means we need to hardcode/calculate gas values ourselves 😑 We could create a helper func that wraps this, and pass its result to all "send Tx" calls.

P.S. it maybe works with ethers-v6, but we can't use it yet, so not useful to us
NomicFoundation/hardhat#3496 (comment)

0xNeshi · 2023-09-08T09:01:11Z

utils/deploy.ts

@@ -23,6 +23,7 @@ export async function deploy<T extends ContractFactory>(
  try {
    const contract = await factory.deploy(...(constructorArguments ?? []));
    await contract.deployed();
+    await delay(1000)


What issues popped up here? 😄
Whatever they were, delaying by a certain number of blocks instead might be more helpful in avoiding the issue 🤔

Ah I see your explanation above now #371 (comment)

If this works then it's fine by me, but a more "lasting" solution could still be waiting a certain number of blocks, we can use:

await contract.deployTransaction.wait(2) // waits for Tx to be included in 2 blocks

Note: I see some people recommending 3 or even 5 blocks, which might be safer than 2.
Note 2: this is not an issue with ethers-v6, too bad we can't use it 😑 (I forget why we can't, but it's something related to one of our packages not supporting it yet)

I like that wait(n) blocks idea better than delay. 👍 It does work though!

0xNeshi · 2023-09-08T09:01:41Z

utils/signers/connectSignerFromPkey.ts

-  const signer = new Wallet(pkey, hre.ethers.provider);
-  return hre.ethers.getSigner(signer.address);
+): Promise<Wallet> {
+  return new Wallet(pkey, hre.ethers.provider);


I think this is even better as it's more generic than using SignerWithAddress

0xNeshi · 2023-09-08T09:02:11Z

tasks/helpers/submitMultiSigTx.ts

@@ -13,13 +14,13 @@ import {filterEvents, logger} from "utils";
 */
 export async function submitMultiSigTx(
  msAddress: string,
-  owner: SignerWithAddress,
+  owner: SignerWithAddress | Wallet,


We can switch all SignerWithAddress uses to Signer as it's more generic in some followup PR (Signer is inherited by both SignerWithAddress and Wallet), created an issue for that https://linear.app/angel-protocol/issue/AP-781/switch-from-using-signerwithaddress-to-signer-in-all-scriptstasks

0xNeshi · 2023-09-08T10:01:53Z

tasks/helpers/submitMultiSigTx.ts

+  owner: SignerWithAddress | Wallet,
  destination: string,
  data: BytesLike
 ): Promise<boolean> {
  logger.out(`Submitting transaction to Multisig at address: ${msAddress}...`);
  const multisig = IMultiSigGeneric__factory.connect(msAddress, owner);
-  const tx = await multisig.submitTransaction(destination, 0, data, "0x");
+  const tx = await multisig.submitTransaction(destination, 0, data, "0x", {gasPrice: 120*10**9});


Seems that Hardhat does not support setting project-wide gas configurations when running with ethers, see:
https://hardhat.org/hardhat-network/docs/reference#gas
NomicFoundation/hardhat#2406

There's an open issue to make this possible NomicFoundation/hardhat#3496

This means we need to hardcode/calculate gas values ourselves 😑 We could create a helper func that wraps this, and pass its result to all "send Tx" calls.

P.S. it maybe works with ethers-v6, but we can't use it yet, so not useful to us
NomicFoundation/hardhat#3496 (comment)

Deployed all contracts and verified to Mainnet

27ff609

stevieraykatz commented Sep 8, 2023

View reviewed changes

stevieraykatz self-assigned this Sep 8, 2023

Added Wallet as a valid output to getAPSigner

7537491

stevieraykatz commented Sep 8, 2023

View reviewed changes

0xNeshi suggested changes Sep 8, 2023

View reviewed changes

0xNeshi force-pushed the AP-723-Mainnet-Deploy branch from 02146d5 to 7537491 Compare September 8, 2023 10:36

Fix failing CI (#372)

52b4370

SovereignAndrey requested review from SovereignAndrey and 0xNeshi September 8, 2023 11:29

SovereignAndrey merged commit c817a3e into master Sep 8, 2023
1 check passed

SovereignAndrey deleted the AP-723-Mainnet-Deploy branch September 8, 2023 12:16

linear bot mentioned this pull request Sep 12, 2023

Resolves AP-638, AP-784: Update setStratParams to run both on Polygon (main Registrar) and strategy's network (LocalRegistrar)" #375

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deployed all contracts and verified to Mainnet #371

Deployed all contracts and verified to Mainnet #371

stevieraykatz commented Sep 8, 2023

linear bot commented Sep 8, 2023

stevieraykatz Sep 8, 2023

stevieraykatz Sep 8, 2023

stevieraykatz Sep 8, 2023

0xNeshi Sep 8, 2023

stevieraykatz Sep 8, 2023

0xNeshi Sep 8, 2023

0xNeshi Sep 8, 2023

0xNeshi Sep 8, 2023 •

edited

Loading

SovereignAndrey Sep 8, 2023

0xNeshi Sep 8, 2023

0xNeshi Sep 8, 2023 •

edited

Loading

0xNeshi Sep 8, 2023

Deployed all contracts and verified to Mainnet #371

Deployed all contracts and verified to Mainnet #371

Conversation

stevieraykatz commented Sep 8, 2023

Explanation of the solution

Instructions on making this work

linear bot commented Sep 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xNeshi Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xNeshi Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xNeshi Sep 8, 2023 •

edited

Loading

0xNeshi Sep 8, 2023 •

edited

Loading