Proxy pattern - Good practices and SOC

[bal7hazar]

Hello, I hope this is a good place to discuss about this topic, else please do not hesitate to redirect me in the right place.

I'm learing and experiencing the upgradable features that you proposed.

Since I'm still learning best practices around smart contracts, I was wondering if there was some kind of advice to follow on where some specific functions should be positioned either in the implementation or in the proxy

Especially about a getImplementationHash function that is positionned in the implementation if we look at one of your proposition. However, from the "seperation of concept" pov, I was expecting a function dealing with implementation stuff to rather be in the proxy.

I don't know if there is a kind of rule saying that Proxy must only implement initializer and fallback functions to avoid any risk or smthg, so here is my point. Thank you for any advice about it 🙏

[ericnordelo]

You probably shouldn't implement functions related to the business logic of your dApp in the proxy because the general idea of it is to redirect calls (like in EIP-1167).

But, sometimes, you want some logic in the proxy related to extra functionality, like functions providing upgradeability of the implementation contract. Then you have a new issue: clashes between proxy and implementation functions. This can be solved in different ways, with some Pros and Cons that are out of the scope of this answer, but I recommend you to check Transparent Proxies and UUPS for upgrades.

getImplementationHash in the implementation results from using the UUPS proxy pattern.

[bal7hazar]

Thank you for this answer, here the resource I read to understand both patterns, if it can help someone else.

[martriay]

hey there @bal7hazar thanks for asking :)
@ericnordelo's answer is sufficient, but I wonder if the library should help users grasp this with better naming.

the lib today requires implementations to from oz.upgrades.library import Proxy and use/expose its functions. the idea behind it was for impls to be proxy-aware and "speak" to the proxy through this functions, but now I wonder if it wouldn't be clearer to call the module Proxiable/Proxied and help users understand those functions are meant to be used in the impl.

[bal7hazar]

Indeed, the library mixes both proxy and proxied logics which is ok I think if we consider the lib to handle all proxy stuff.

Accordng to your point, a preset of both proxy and implem would help to understand where each function must goes imho (and sufficient enough).
Also, if the logic is disgned to be used in UUPS proxy pattern, it could be specified somewhere (doc or comment) to avoid wrong usages.

[martriay]

Indeed, the library mixes both proxy and proxied logics which is ok I think if we consider the lib to handle all proxy stuff.

no it doesn't mix any logic. everything is implemented where it should. there's an argument to be made (i did above) to whether call the library Proxy or Proxied, but that's no logic issue.

Accordng to your point, a preset of both proxy and implem would help to understand where each function must goes imho (and sufficient enough)

that's exactly what we have now: Proxy (preset only, there's no lib to make/customize proxies), and implementation.

Also, if the logic is disgned to be used in UUPS proxy pattern, it could be specified somewhere (doc or comment) to avoid wrong usages.

it's on the docs

accidental misusage is really hard since we don't offer any other proxy preset not library. users following our docs can easily implement their implementations, and departing from there (e.g. implementing something in the proxy as you asked when opening this issue) is very stretch, you need to circumvent our lib API.

[bal7hazar]

no it doesn't mix any logic

Yes my bad, I just forgot what I read yesterday 🤭

it's on the docs

I have no excuse then for not having understood on my first attempt 😓
Everything seems to be well documented.

that's exactly what we have now: Proxy (preset only, there's no lib to make/customize proxies), and implementation.

What about to have a Implementation.cairo in proxy/presets implementing all functions (get_implementation, set_admin, get_admin, etc...), this ERC-20 preset only implements upgrade and this is why I came here first since I didn't know where I had to implement other functions (in proxy or implementation). But that's also because I didn't know much about proxy patterns, now it's very clear.

[martriay]

preset contracts are meant to be deployed, a simple implementation proxy would be of no use. as of what implementation contracts should do, it's also documented in here.

[mstrong-tech]

hey! could you help me to know how the proxy is upgraded?
here is an interesting example: https://goerli.voyager.online/contract/0x02cd30ebc9a05d795ea3c72e42c857e4105510a8c914935b0a825d048878dc7c#readContract
this is a proxy contract and its implementation is 0x2c77ea7fb7adae748be83767fe216acc31d882b7e18f16279f1143a2860f05a.
the problem is that i don't know how the proxy's implementation is upgraded to the above address.
i traced all the transactions and events of this proxy but couldn't find any place where the upgrade happened.
when this proxy contract is deployed, implementation parameter in the constructor was set as 0x0501fd03c9cdb3a15d8f7bfa5145a7bb131794af8caa50f6ff56b4800a059a79. i don't understand how this was changed to 0x2c77ea7fb7adae748be83767fe216acc31d882b7e18f16279f1143a2860f05a.

[martriay]

it's hard to analyze a non verified contract. at a first glance i don't see the event either, but cannot really tell.

here for example upgradeTo is being called but no event fired