Description
Abstract
Proposing the addition of ink! messages which do not enforce reading and decoding the top level [ink(storage)] struct. This would provide additional flexibility to contract authors, and provides an "escape hatch" in case the data at the root key of the contract cannot be decoded into the top level storage struct.
Motivation
ink! messages are currently required to be methods with a &self or &mut self receiver. In both cases the top level contract storage struct is first loaded from storage and passed in as the self argument. For &mut self the storage struct is persisted following successful execution of the message logic.
One problem with this is that if the storage data at the root storage key of the contract cannot be successfully decoded into the storage struct, then the contract will be permanently* "bricked". This can happen either by the storage being corrupted via a direct write to set_contract_storage, or during migration to a new version of a contract via set_code_hash where the new version of the contract has an incompatible layout with the data at the root storage key.
The following example illustrates:
#[ink(storage)]
struct MyContract {
value: u32,
}
#[ink(message)]
fn migrate_storage(&self) {
const STORAGE_KEY: u32 = 0x00000000;
let new_value = self.value as u64;
ink::env::set_contract_storage(&STORAGE_KEY, &new_value);
}
#[ink(message)]
fn set_code(&mut self, code_hash: Hash) {
// this message cannot be called because the storage will fail to decode.
self.env().set_code_hash(&code_hash).unwrap()
}The call to migrate_storage writes directly to the contract storage, upgrading the value from a u32 to a u64. A subsequent call to set_code (or any other message) would now fail because the u64 cannot be decoded into the original u32 (see #1897). Note that decoding errors can also occur with other any other invalid/corrupted data.
Therefore we need a way to be able to call messages such as set_code which do not have the constraint of pre-loading the storage level struct.
* The "bricked" contract could be fixed by calling the priveleged set_code extrinsic on pallet_contracts, but this would require the contract author coordinating with the sudo owner or via governance.
Specification
Rust allows "associated functions" in impl blocks which do not take a self receiver. These can be used for messages which do not load the root contract storage before message invocation. Using the example above, the set_code function becomes:
#[ink(message)]
fn set_code(code_hash: Hash) {
ink::env::set_code_hash::<<Self as ink::env::ContractEnv>::Env>(&code_hash).unwrap()
}This allows the contract code to be updated without first requiring to load the contract state, which would fail.
Considerations
Does this pattern open up any security vulnerabilities, or otherwise introduce a footgun for contract authors? Should we allow contracts to be bricked, and encourage better testing to avoid the eventuality instead?
Alternatively we could encourage writing all contracts using Lazy and Mapping values at the top level, which could avoid some of the problems by not expecting any data to be stored at the top level. Of course this would still be affected by storage corruption if any data happens to be written directly to the root storage key.
Rationale
The layout of the contract storage can change, either intentionally for migration or accidentally/maliciously by directly writing to contract storage via ink::env::set_contract_storage. In this and other cases where we do not want or need to load the root contract state before executing a message, ink! messages which do not accept a self instance of the root contract storage should be provided.