We should be able to attach to update calls (query calls, too) a hash, so that if this hash does not match the WASM hash of the canister, the update or query call should fail.
This is to prevent the race condition of calling an updated canister, while old version (or nothing) should be called.
It is especially useful in context of Seb’s project of “owned canisters”, because the user may update his canister at any moment and external (not user owned) canisters may rely on it.
Consider a canister that worked well in the past but now updated (by a malicious person, for instance) to block updates of the calling canister by not returning from any called method. In this case checking by “application logic” makes no sense and is even harmful.
Race condition: start upgrading a canister, query “application logic”, upgrading finishes (to a malicious code), the application is called. So, by a race condition we called a malicious upgraded canister while thinking it’s yet a previous version.
How do you determine that the update is malicious in this case? When does your canister start trusting the new module hash?
That race condition exists if the module hash checking is implemented in your canister, but not if it’s implemented in the 3rd party canister. Implementing it in the 3rd party canister is subject to point number 2 though.
Thanks for the additional context, I think this could be an interesting feature to have on the protocol level.
I’m not convinced that what is suggested here actually solves the original concern that was shared.
OP mentions:
“It is especially useful in context of Seb’s project of “owned canisters”, because the user may update his canister at any moment and external (not user owned) canisters may rely on it.”.
If interfaces are changing, Candid is changing as well and therefore a call with an outdated payload would already fail no? So to some extension, getting an issue because of invalid payload or because a invalid hash, at the end of the day, it’s kind of same same.
We would hold a list of checked non-malicious hashes, to ensure that a malicious code isn’t called. It starts trusting new hash after a code review and our DAO deciding to trust it.
Implementing in 3rd party canister probably really avoids the race condition, I didn’t notice this. But also note that using a third-party canister changes the principal to which the checked canister needs to trust, what is undesirable, (and also is probably rather slow).
I see, that’s a good point. How would you generate a hash just for the code of that function that changed though? You can generate the hash for the entire WASM, but is it possible to do that just for a function and what’s used within that function? If not automatic, if the developer has to provide the hash manually, I’m not convinced it should be part of the protocol I would say sponaneously.
Of course, we generate hash for the entire WASM of a canister. It is already done in dfx deploy to decide whether to deploy or deployment is skipped because the same WASM is already deployed.
I don’t follow. If your idea is to use the hash of the entire WASM, how do you differentiate one particular function that has an unchanged interface but changed its logic from another function that remains the same? Or is your idea to prevent access to any functions, even if they did not change, if the provided hash is not the current hash?
My idea is to use the hash of the entire WASM. I don’t differentiate one particular function that has an unchanged interface but changed its logic from another function that remains the same. My idea is prevent access to any functions, even if they did not change, if the provided hash is not the current hash.
This would mean that the canister is broken until the new hash of the 3rd party canister is approved. I think there might be better ways to handle this.
If we’re talking about user owned canisters, then maybe we need to look at some kind of compatibility matrix. So if a user owns canisters A and B, A relies on B. If B has a new update, the user is not able to update A until there is an update to A that gives it compatibility with B.
For other scenarios, I think immutable smart contracts can handle this in a nice way. Deploy a canister and then black hole it. When there’s a new version, create a new canister and then black hole that one two. When dependendant canisters are ready to move from a previous to a new version, then can switch from the previous canister to the new canister.
Well then, in that case, I believe this feature might not be suited to be implemented at the protocol level, or at least it is not something I would use I think. In my humble opinion, while it is true that APIs can be fully deprecated, such as moving from API v1 to v2, there are typically several iterations between major versions. These iterations usually occur at the function level rather than the global level.
For instance, in Juno, I often deprecate specific functions and rarely change their behavior significantly. When I do make changes, they are always at the function level, not for the entire canister - i.e. I definitely do not want to prevent a global access to an entire canister. I’m more approaching development with the most backwards compatibility as possible. Similarly, in then open, I would also not want to restrict access based on the overall version of a contract but, I guess that’s probably personal opinion.
In summary, this seems to be a particular requirement that I do not share, therefore thinking it might be better to solve it particularly and not on a protocol level. Of course, this is just my two cents. If the majority find it beneficial, I am for sure not against to it as long as it remains an optional feature.
@peterparker I don’t see the logic: As I understood you, you critique other variants (such as Juno’s one) of doing this and then reject my variant. Why? It is also fully backward compatible.
In other words maybe: that’s not an approach I use or want to use. Therefore, I’m not convinced of its necessity at a global level, and I don’t think it should be available at a protocol level. Instead, I believe it’s something that should be developed by individual projects. However, if most people see the need for it, I am certainly not against it.
This is to protect against security vulnerabilities that may be worse that simply an outage. For this reason, my proposal temporarily disables access to the canister.
We can improve my proposal to avoid outages, passing several hashes and succeeding the call if one of them matches the current WASM hash. This allows to prepare for a new hash in advance.