anoma / namada

Rust implementation of Namada, a Proof-of-Stake L1 for interchain asset-agnostic privacy
https://namada.net
GNU General Public License v3.0
2.4k stars 960 forks source link

Ungovernable wasm code keys #3061

Open grarco opened 7 months ago

grarco commented 7 months ago

Seems like the wasm keys in storage do not fall under any address and therefore have no associated VP.

https://github.com/anoma/namada/blob/7d70e5a7a4ef50bdad2df9f665ffa39998c328c5/crates/core/src/storage.rs#L698-L732

Since we prevent modification to non-protected keys in storage, this means that no governance proposal can effectively modify these keys, which is required if we want to extend the allowlist for example.

We should place these keys under some VP to allow this and also add a check in this vp when a new wasm code is added to storage by running the validate_untrusted_wasm function to make sure that the tx doesn't contain unwanted features (this can be run by the community members before voting on the proposal but I believe a check in protocol would add more safety). On top of that we can also check that all the required keys are updated accordingly, e.g. if we add a new wasm code we should also insert the relative name, length and hash keys, and if we want to remove one from the allowlist we should do it in the way we designed (just remove from the allowlist and keep everything else for execution).

grarco commented 7 months ago

After #3100 we only need to update the parameters vp to check the correctness of the updates coming from a proposal

cwgoes commented 6 months ago

These checks are important in general, but not critical for phase 1 - we can take extra care for the first proposals.

brentstone commented 6 months ago

why reopened?

grarco commented 6 months ago

Second part of this issue hasn't been addressed yet. As per Chris's last comment we are leaving it for later