Closed obumnwabude closed 1 month ago
You could use the same contract name in a differently named Solidity file, then refer to that contract using its fully qualified contract name in the format FILE_PATH:CONTRACT_NAME
. For example, "contracts/BoxV2.sol:Box"
The purpose of requiring both versions of the contract is to allow storage layout validations to be run.
Would it be possible to download the source from etherscan/polygonscan/etc and use that automagically as a comparison? Similar to what the truffle debugger did with the --fetch-external flag. I mean, would something like that somehow be considered as an addition to a roadmap?
@tomw1808 I think that makes sense as an enhancement and we will consider that as part of this issue -- we have a similar enhancement in mind for Hardhat in https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/598
ohhh so basically right now we need both contracts.
i was expecting it to run a comparison with the previous implementation, but it does make sense since if the contract has never been deployed there would be nothing to check against, same with using the same contract.
i think a cache/artifact system eill be a good idea to store previous versions to check against for upgrades where it stores the previous versions that oz foundry would then use to compare the current iteration (this artifact MUST be included in the commit since it is needed in future upgrades), doing it like this will make it easier to have fewer files while also allowing the oz checks to work.
now the problem here would be the changes mid development if something changed and created a conflict with the old builds (uncommited local)
ex.
if so there should be a command to revert the changes for the artifacts to the commited build to clear the conflicts (which should be doable with git)
yeah now that i think about this enough it would be complicated to implement although it would be easier for the development side since there would be fewer files
what i was doing was just adding a public constant var for the contract version instead of a new implementationV2 and hoping the check would be smart enough to compare the changes from the previous contract state 😂which is foolish hahaha
So, I know that is very specific to our own project, but I made a sort of prototype now for a release pipeline. Not that we have so many releases, but I still like to have a functioning reassurance that I'm not overwriting anything in production that shouldn't be overwritten. I also know that doesn't apply to every project, ours is a bit bigger and dates back before hardhat even existed, so there was a lot of porting going on over the years. Anyways I ended up with something like this:
So that would be what I will end up with. The main scripts are actually really not a lot, however its finicky and of course if would be better if there was some sort of toolchain that supported this somehow with a standard workflow...
Here's the script to download the polygon contracts (mind our own folder structure comes from truffle before): https://github.com/Morpher-io/MorpherProtocol/blob/account-abstraction/helpers/chainfork/dl_source.js
and here's the script that runs everything, the script, then anvil and then overwrites the storage and then runs forge script to validate the contracts (and then run the unit tests in the future - work in progress): https://github.com/Morpher-io/MorpherProtocol/blob/account-abstraction/helpers/chainfork/start.sh
May it help someone out there...
@tomw1808 I think that makes sense as an enhancement and we will consider that as part of this issue -- we have a similar enhancement in mind for Hardhat in https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/598
if it's going to be fetched from an external source i think adding a custom natspec that indicates the chain, address and or name of the reference contract implementation would be good since not only will the deployed next version implementation would have a reference pointing to that contract itself meaning people will be able to see previous versions of the contract with clear reference, it would also make it easier on the development side since developers would just need to add a natspec for it without adding more contracts for new implementation versions.
the problem i am seeing with this is which would be used to check the code?
address().code (i don't know if it is possible to use this for storage layout verification)
or
block explorer api for fetching the source code (this would cost money since it is usually under pro tier feature)
maybe there are other ways to do it too
@Metaxona
i think a cache/artifact system eill be a good idea to store previous versions to check against for upgrades
For Hardhat, we do exactly that. Hardhat Upgrades uses network files to save the storage layout of deployed contracts, which are used for comparison during later upgrades. We decided not to use this approach for Foundry for the following reasons:
forge script
can optionally broadcast some of the transactions that were simulated. From the Foundry script, I am not aware of a way to properly determine if the current part of the code is being broadcast (and even if it is, the script may still fail at a later step). Therefore, we cannot conclusively know when a deployment has occurred to a real network before saving the storage layout.if it's going to be fetched from an external source i think adding a custom natspec that indicates the chain, address and or name of the reference contract implementation would be good
Supposedly the chain and address can already be determined from how the user is calling the upgrade function (via the connected network and the address parameter), but perhaps which specific explorer to get source code from needs to be provided somewhere.
@tomw1808
I made a sort of prototype now for a release pipeline
Thanks for sharing. Another possible future approach may be to keep previous releases in a different branch, and when we support comparisons with other build info dirs, the storage layout from the build info of the previous branch can be used for reference.
All the examples make it look like to upgrade a contract, you are changing the proxy to a new implementation address whose Contract name is different from the old implementation. (BoxV2 against Box).
But how do you make changes to the old implementation without changing its name and successfully upgrade the proxy?
For example, you fix a simple logic in one line of code in Box and then you want the proxy to reflect this. But then following the provided steps don't work. Even after setting
unsafeSkipAllChecks
to false in opts.So how do you upgrade without changing name?