90 upgradable contracts

[bal7hazar]

Fix #90 #94

[tekkac]

LGTM 👍

[cloudvenger]

huge job !
Look good for me into src, protostar upgrade and good initiative to insert testnet-2.
In addition, I like the rework of the test/integrations because there is less code duplication event if it creates a library file huge.

I approve of the PR because I have no complaints about the src section and the script and uprgade. Thus, in view of the structuring changes, this must be merged in order not to create a conflict for the other PRs.

However, here is an important note that I would like to discuss in this thread:

In the tests (unit like integrations), I did not find any trace of tests of the upgrade function
(from offseter, yielder, minter, project SC) in order to test the proper functioning and implementation of the Proxy pattern.

Maybe, we should create a directory tests/integrations/migration or tests/integrations/upgradable or tests/integrations/sc-management where we are testing :

• upgradable of smart-contract
• changing of ownership
• try to access/execute contracts without going through the proxy

What do you think @bal7hazar ?
I think, if we are agree to do this, put it in another PR because it only impacts the tests.

[bal7hazar]

There is not point to unit test the proxy since we didn't implemented any logic for this.
We are just relying on OZ features (check proxy.cairo), it would make sense if we add additional logic in a proxy/library.cairo lib, but there is no need.
So basically we trust in OZ unit tests (as we do for ERC721) and we don't do the exact same tests from our side.

[bal7hazar]

It can make sense to add some more integration tests such as upgrade a contract or change he admin.

However if a contract is deployed as an implementation through a proxy we cannot access it directly since there is no real contract. Since we want them upgradable, I don't think we need to e2e test them without proxy.

[cloudvenger]

Agreed about the non-necessity to redo unit tests in this specific case, because it would be, as you say, reimplement the OZ tests.

Understood about the impossibility of accessing a contract without going through the proxy.

Indeed, I was not talking about testing them without going through the proxy, but about making sure that the proxy is the owner of the contracts and that we cannot do anything with the contracts without going through the Proxy.

As I said in my preview comment, for me, we can merge in the PR state.
And we can discuss how to do these integration tests (upgrade, change of ownership).