Consider using `BorshStorageKey` to allow for iterative upgrades/migrations

[mikedotexe]

Description

Before we hit the security audit, a relatively small win might be to set up the mpc-recovery contract to be migrated/updated/overwritten by new versions of the contract.

I recognize lines like this as the "original technique" we used back in the day, where we're assigning the contract to a hardcoded prefix key m.

https://github.com/near/mpc-recovery/blob/e89ff439ead0c2fc8e53043625241ece38dad2b8/contract/src/lib.rs#L75

As this project matures, if we decide to update the pending_requests struct to include more fields or something, it would become problematic. That's because when a contract method is called, the first thing that happens is the blockchain storage is retrieved and essentially marshalled into the nested structs with marked with the [near_bindgen] macro. If any of those nested structs is altered, the state data cannot fit nicely and it freaks out. (We also have handy macros like #[init(ignore_state)] that intentionally skip the state loading step.)

This looks like a great reference on this topic:

https://github.com/near-examples/update-migrate-rust/tree/b03e75c313a8cfbbb62e71758d2e6f49480f3861/enum-updates/base#guest-book-contract---versioned-messages

Some ideas from docs on this, too:

https://docs.near.org/tutorials/examples/update-contract-migrate-state

I'd recommend using the SDK's BorshStorageKey which is a goodie made for us in this situation:

https://docs.rs/near-sdk/latest/near_sdk/derive.BorshStorageKey.html

So instead of b"m" it'll look like what Jacob has here:

supported_assets_oracle_asset_ids: UnorderedMap::new(StorageKey::SupportedAssets),

https://github.com/near/multichain-gas-station-contract/blob/51c7d941c719977bd2ae887fbdff9e39741af3e6/gas_station/src/lib.rs#L248

[volovyks]

Nice suggestion, we worked on this here: https://github.com/near/mpc-recovery/issues/545 Ping us if you will have suggestions on implementation.

[mikedotexe]

Thanks, it looks like we're not exactly following the recommendations from docs in the approach I'm seeing linked and merged.

The BorshStorageKey isn't used, and I think it'd be helpful. What we'd like to avoid are lines like this:

https://github.com/near/mpc-recovery/blob/4d19bec2db80205ed9cdde4a740f4bca17d9a252/contract/src/lib.rs#L106

Because that means the prefix m is being hardcoded in state. What BorshStorageKey does is provide an alternative to hardcoding that eases in upgrades/migrations.

I think the eventual solution will remove all hardcoded state keys like we have it currently. (There are two hardcoded "m" keys to address, btw.)

^ https://docs.near.org/sdk/rust/contract-structure/nesting#error-prone-patterns