Add namespace to Upgrades #300
Conversation
| Proxy_only_admin() | ||
| Proxy_set_implementation(new_implementation) | ||
| Proxy._only_admin() | ||
| Proxy.set_implementation(new_implementation) |
There was a problem hiding this comment.
since it's up to the devs to protect the set_implementation function, shouldn't we signal this by making it a type B function aka a building block/not meant to be exported as-is, and name it _set_implementation?
There was a problem hiding this comment.
otoh this is going to happen a lot (libraries with many B functions), I'm doubtful now
There was a problem hiding this comment.
Hmm I'm actually in favor of that, but yes, it definitely is going to happen a lot. I'll update using the type B syntax. If there's objections after seeing it, we can discuss/reassess further
| # Initializer | ||
| # | ||
|
|
||
| func initializer{ |
There was a problem hiding this comment.
I think we should call it constructor like the rest of the namespaced libraries, no? Or at least open yet another discussion :p
There was a problem hiding this comment.
Haha so many discussions! Yeah, using constructor is probably best. I was thinking this was a special case, but it really operates in the same way the other constructor/initializers do
There was a problem hiding this comment.
In the latest commits, the Proxy library's initializer is changed to constructor, but I kept the initializer function name in the implementation contracts. My thought process is to ensure users don't think to use initializer in the decorated @constructor. See example here. Since it must be invoked after deployment, I think initializer is more appropriate
There was a problem hiding this comment.
Definitely, I had something similar in mind.
| # Guards | ||
| # | ||
|
|
||
| func _only_admin{ |
There was a problem hiding this comment.
what do you think of prepending guard-like functions with assert_? like assert_only_admin. I think it's pretty clear what it does, and it would be consistent across the language and with other codebases.
There was a problem hiding this comment.
I was actually thinking that the guard functions look a little naked when they're used. Prepending with assert_ is a really good idea
This PR integrates namespace to the Upgrades library, Proxy contract, and mock proxy contracts. This PR also proposes to reorganize the Proxy methods. Finally, this PR proposes to update the Proxy docs with the changes.