Use initializable library inside other libraries

[ericnordelo]

For example in upgrades library:

https://github.com/OpenZeppelin/cairo-contracts/blob/b764660da224ee757b831ea62bf7a4c63ee898fb/src/openzeppelin/upgrades/library.cairo#L44-L55

Wouldn't be better to import and using here the initializable library instead of rewriting the initialization logic? I think importing libraries from libraries is not against the Extensibility Pattern.

[martriay]

The reason I did not use Initializable was to isolate the initialization of the proxy from the implementation. If it did, initializing the proxy would automatically initialize the implementation, they share the storage variable.

[ericnordelo]

I don't get it, let me know what I'm missing:

This Proxy.initializer is meant to be called from the code of the implementation right? And the implementation is declared (but not deployed) so it can't be initialized. Initializing the proxy would use the implementation initializer through a library_call, and this initializer would use the Proxy.initializer from the library to set the proxy_admin. But the name of the storage variable to handle "already initialized" is not relevant right?

I'm suggesting something like this in the Proxy library initializer:

// "./openzeppelin/upgrades/library.cairo"

func initializer{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( 
     proxy_admin: felt 
 ) { 
     Initializable.initialize();

     _set_admin(proxy_admin); 
     return (); 
 } 

[martriay]

But the name of the storage variable to handle "already initialized" is not relevant right

Yes it is. The storage address of a given storage variable is a function of its name, therefore all library calls to Initializable.initialize() would be sharing the Initializable_initialized storage variable which can only be set once. This is why it shouldn't be called from any other library initializer, otherwise it would stall the initialization process (if any initializer also checks for uninitialization).

That being said, I'm not being able to remember why not simply following the extensibility pattern and removing the initialization check completely, since the pattern expects the checks (if any) to be performed in the constructor as a whole, not in each initializer. Resulting in:

// "./openzeppelin/upgrades/library.cairo"

func initializer{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( 
     proxy_admin: felt 
 ) {
     _set_admin(proxy_admin); 
     return (); 
 } 

And users (if following the extensibility pattern) shouldn't call this function from another initializer, nor from another function aside the constructor, and just once.

[martriay]

I think importing libraries from libraries is not against the Extensibility Pattern.

All in all, is against the pattern to use initializers anywhere but in constructors, this includes (i.e. excludes :p) library modules. So you can import libraries, but you shouldn't call initializers.

[ericnordelo]

Now makes sense. I wasn't considering that a contract could extend from different libraries simultaneously (of course).

With this said:

And users (if following the extensibility pattern) shouldn't call this function from another initializer, nor from another function aside the constructor, and just once.

In the UUPS upgradeable implementation version we are providing from the Wizard, we have an external initializer (acting as the constructor) which calls the Proxy.initializer inside. If we don't check initialization then the proxy pointing to this implementation could be re-initialized

That being said, I'm not being able to remember why not simply following the extensibility pattern and removing the initialization check completely

This could be the why. I understand why the proxy needs this design, but this goes against the extensibility pattern's current definition, maybe we should add an entry for this exception in the docs?

[martriay]

Closing this since it will not be relevant anymore after the ongoing Cairo 1.0 migration. If you think this is a mistake, feel free to open a new issue.