Namespaces are a very powerful (although undocumented) way to write Cairo modules. It allows scoping function definitions under an identifier, helping prevent collisions when importing functions from multiple modules.
Nevertheless, the current implementation does not allow to scope storage variables (or functions representing them) within namespaces, opening the door for storage collisions in the case of two modules defining the same storage variable, like this:
# module_a.cairo
@storage_var
func counter() -> (foo: felt):
end
# module_b.cairo
@storage_var
func counter() -> (foo: felt):
end
You can try this working example by @koloz based on @andrewâs previous research. Note that because of how storage variables work in StarkNet, these two modules will be writing and reading from the same storage slot potentially resulting in bugs or losses.
This is the case even if the user of the modules doesnât manually import any of these storage variables, since any @external or @storage_var directive gets executed and effectively âimportedâ when importing any identifier from a module.
Proposal
Support @storage_var definitions within the scope of a namespace
To make it easy for devs and tooling to reason about storage location, it can be internally computed as:
Stop âauto-importingâ storage variables and external functions that are not explicitly imported. It is a bug. Smart contract development is a critical task and itâs already hard enough, letâs minimize surprises, letâs aim for a predictable execution and programming environment.
I have run into some issues when using bound usage patterns and support both parts of the proposal.
The issue I ran into involved wanting to define a contract with âhookâ functions that should be overridden by derivative contracts.
I had a contract A.cairo which defined some âhookâ function extensible(). In this contract I had a function user() that calls extensible(). Afterwards, I created a contract B.cairo that imported the function user() but wrote a different implementation of function extensible(). Unfortunately, contrary to how it would work in a contract inheritance pattern, this led to the old version of extensible() being used in user() since usage relationships are bound on a function level.
In general, I believe in incremental proposals that move towards more familiar inheritance/multiple inheritance patterns and this is a great example.
I 100% agree with the proposal. It removes two major footguns from the language that unless removed are bound to result in vulnerabilities and hacks.
Though, if this proposal is not implemented then itâs very feasible for a static analysis tool to discover and expose potential issues created by storage variable name collisions and accidental imports of external calls. (especially namespace collisions could be a static check in the compiler)
On not auto-importing, does it make sense to spin off a separate discussion for this? There is a danger of go down a rabbithole about how to approach composition, and how to do it well .
Final note: with plain not-auto importing external calls there is a danger that you use (âextendâ) an external library and forget to import all the external calls.
Doesnât this couple the storage var address to where its code location (where itâs defined in the code)?
Itâs not necessarily a bad thing, but I can imagine a case where some contract treats the storage var of another contract as something it might want to rely on (essentially making the storage var an API of the defining contract).
Another way to overcome name collision would be to include the name qualification explicitly in the storage var name.
i.e. a storage var s1 in namespace n1 could be called n1_s1, so another variable named s1 in a separate namespace wonât collide with it.
This isnât perfect. Itâs arguably less elegant (itâs like always referring to the storage var by its fully qualified name), and it relies on convention.
But it solves the name collision, and allows use case where the youâd want the storage var address to not rely on its location in the code.
I think that relying on autoimporting is not an acceptable extensibility pattern.
This is what we currently do but itâs one more thing to be mindful about, another point of failure. Thereâs too many security considerations and we need to minimize the mental burden for developers to focus on business logic.
Sure but thatâs just another convention ÂŻ\(ă)/ÂŻ . External parties can rely on ERC20.decimals instead of ERC20_decimals.
Oh, for sure. Just thinking about what could go wrong if you turn off auto importing straight away without a nice extensibility pattern to put in place.
I feel like implicit storage aliasing will be a really difficult thing to do right even if you do use it intentionally.
iâm in favor. the less we can rely on people to have a standard the less bugs that will potentially pop up overall and people wonât have to think about it when writing contracts (especially new devs who may not know about how variables are stored)
If this idea is not adopted, a static analysis tool might easily find and reveal potential problems brought on by storage variable name conflicts and unintentional imports of external functions. Namespace clashes in particular might be subject to a static compiler check.
.