Questions about the `.openzeppelin` upgrades file

Support Upgrades
1

Hi.. I have two questions about the .openzeppelin file that is created by the Upgrades plugin.

  1. Can I manually use the hash that it creates to identify an implementation to check to which contract corresponds? For example I would like to check on Etherscan a contract i deployed some time ago and be able to calculate this hash and then check in the openzeppelin file if I have that implementation properly registered, without relying on the code I have locally in the repo. Is there a way to do this?

  2. Is it possible to manually delete the proxies section of the internal .openzeppelin file ? In my case im not interested in tracking proxies. I would even prefer not to and just keep tracks of the deployed implementations.

Regards,
Julian

2

@ericglau I would really appreciate if you can clarify this doubts as you are the most experience person in this subject. Thanks!

3
  1. You can use the hashBytecodeWithoutMetadata(bytecode: string, constructorArgs = '') function from the @openzeppelin/upgrades-core package to calculate the implementation hash. The bytecode argument is the contract creation code, NOT the deployed contract bytecode. On Etherscan, you can get this from the Input Data field of the transaction that created the implementation contract (and by doing so, this Input Data field also includes the encoded constructor arguments, so you don't need anything else for constructorArgs).
  2. I don't suggest deleting the proxies, otherwise you won't be able to upgrade them unless you forceImport them again. The proxies section is used for tracking the kind of proxy (uups, transparent, or beacon).
4

Thanks for your answers! I realized I can do the whole upgradeability check just by using the implementations. So im doing forceImport passing the implementation address and use validateUpgrade also using the implementation address.

This way I dont have the proxies section in the file. I dont need that section because Im not doing the upgrades through the plugin but through governance. Also I dont "own" the proxies, they are deployed via a factory and owned by other people.

So just using the implementations seems like great for me. Do you see any downside?

5

Makes sense. In that case you don't even need forceImport -- you could just call validateUpgrade with both ethers contract factories directly e.g. validateUpgrade(oldContractFactory, newContractFactory)

6

Right. But I dont have the old contract factory anymore. It has been replaced by the new version of the code (imagine some months after) so I rely on previous implementations being force-imported before and available in the network file. Everytime I release a new version I deploy the implementations, forceImport them so they are in the network file.

And then in the future, when im doing a new version, I check with validateUpgrades just having the new code that is being developed in the repo (with the changes). Seems OK?

7

What you described sounds reasonable for tracking the storage layout. There are slight risks with this approach though:

  1. forceImport itself does not do any safety checks for the imported contract, so if an unsafe contract was already deployed, that would not catch it (unless you already did validations before deploying).
  2. forceImport does not check that the contract source code you are importing actually matches what was deployed at the address. So there is more possibility for manual error.

Consider deployImplementation (for the first version of a contract) and prepareUpgrade (for later versions). Those do multiple things: validation, deploys the implementation, adds it the network file without needing forceImport. Note prepareUpgrade does not actually perform the upgrade, so this is the recommended way to deploy a new implementation in preparation for upgrade through governance.

8

Nice. Very valuable info. I agree with your approach. The original implementations are already deployed and safe (we checked them manually). But I do see value in doing prepareUpgrade instead of a simple deploy for implementations. Avoiding forceImport seems pretty valuable.

So prepareUpgrade would be the replacement of simply deploying a new implementation? (not for the first time, but one that is intended to be used as new version in upgrade?)

Note: many of these functions were not available when we started so I didnt include them from the get go.

UPDATE: Just realized prepareUpgrade will not work for me as it needs a Proxy. And im only handling implementations. Is it difficult to make it accept an implementation address as the other functions?

1 Like
9

Opened https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/777 to allow prepareUpgrade to take an implementation address.

1 Like
10

Thanks a lot for the quick response and the action taken on the function. You rock!!

1 Like
12

Hi @julianmrodri, just sharing a few additional details here that may help:

  • The plugin makes some read-only calls to the given address to check if it's a proxy or beacon, so some reverts are expected during this process and the plugin should catch these internally. If it determines that it's not a proxy or beacon, then it treats it as an implementation contract.

  • If you’re using mainnet forks for testing, check if you are using Hardhat 2.12.3 or newer. The plugin makes use of this for better handing of the network file as described in https://docs.openzeppelin.com/upgrades-plugins/1.x/network-files#temporary-files and https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/726 — it will automatically copy the mainnet network file into a temporary directory instead of using .openzeppelin/unknown-31337.json.

1 Like
13

Cool!. I still see the reverts so that explains it. Really cool that feature about the mainnet network file! I'll include it. The hardhat version may explain the hardhat_metadata Method not supported

Another odd thing im seeing is that in some cases its not deploying the new implementation and just returning the same address of the current implementation. Does the plugin skip deployment if there are no changes?

14

Another odd thing im seeing is that in some cases its not deploying the new implementation and just returning the same address of the current implementation. Does the plugin skip deployment if there are no changes?

Yes, it skips deployment and returns the existing implementation address if the implementation is identical to what was previously deployed.

1 Like
15

Bumped Hardhat and fixed the metadata error plus I dont need to rely on unknown-31337.json anymore. Thanks! Seems like its working as expected now!

1 Like
16

Eric just a final thing on this, I found.

With both functions deployImplementation and prepareUpgrade I am using 2 contracts that use external libraries, and Im adding the unsafeAllow external library linking.

What surprises me is that I get the warning message for only ONE of those 2 contracts. The other one I don't get the warning.

What could be the reason one of them does not even show the warning and the other one does?

By looking at my code one of them uses the libwith using MyLib... and the other one just references it like MyLibrary.function... . so maybe the plugin its not detecting im using an external lib in the second case? Seems like this might be an issue?