feat: access key mgmt for relayer

[vzctl]

This feature introduces an ability for the owner to add/remove function call keys for submit calls.
Calling submit using such function calls speeds up engine transactions by one block (~1 second)

In the future the owner will be replaced with a separate role using AccessControllable from near-plugins.

Scenario

1. A new "owner" keypair is generated by the relayer manager
2. The owner adds that key (1) as a FC key to the owner account with a permission to call add/remove_relayer_key method on aurora contract.
3. The owner account gets funded to process/relay aurora transactions
4. The relayer manager generates keypair(s) that would be used to relay transactions to aurora
5. The relayer manager uses the "owner" key (1) to call add_relayer_key method on aurora, passing a public key (4) with attached funds/allowance to process transactions from aurora account.
6. The relayer uses the key (4) to submit transactions from aurora account until the allowance gets exhausted
7. The relayer manager removes FC keys with exhausted allowance

Problem

The FC key that is added to the owner account on step (2) would need to attach NEAR deposit to the add_relayer_key call on step (5), which is not allowed (the allowance can only be used for gas, not for NEAR transfers).

Ways forward

In the medium/long term we will switch to a dynamic access management with a proper relayer manager role.

For the short term we have the following options:

1. Hardcode the list of relayer managers in a static array
2. Add relayer_manager_id to the engine config and implement state_migration for it. For backwards compatibility the config struct will inherit the owner_id value and will implement a getter/setter method for relayer_manager_id
   2.1) Handle the state migration as part of feat(engine): erc20 gas token #662
   2.2) Implement a separate state migration and handle it separately as part of this PR
3. Add a separate key to the storage, outside of the Config struct

If we go with 2) or 3) we would need to handle one more state migration later to deprecate unused state that would be replaced with AccessControllable of near-plugins. We probably need to do it anyway to remove owner_id.

Additional questions

Should we fix/improve the problems in public_key.rs code in this PR (increasing the drift from near-sdk) or should we accept the current state and work on these problems upstream?

[aleksuss]

Is the unwrap really safe here? I would add CurveType::Unknown, and if something is wrong, then return CurveType::Unknown.

[vzctl]

I'm trying to minimise the changes to the original code as much as possible:

https://github.com/near/near-sdk-rs/blob/master/near-sdk/src/types/public_key.rs#L104

Is there a less-invasive option to make it safer?

[aleksuss]

I guess, this call should be moved to the *_relayer_key functions. Because parse_public_key_args does not only the parsing key. The principle of single responsibility. And then, it would be easier to write a test for the parse_public_key_args.

[vzctl]

Renamed the function to be more explicit that the access checking is happening here too.

My idea is to be defensive and require authorization before any parsing takes place - preventing parsing vulns from being triggered by unauthorized accounts. Decoupling and duplicating these function calls in *_relayer_key could be more risky than keeping them together.

Also, this access checking must be migrated to AccessControllable asap anyway.

I don't have a strong opinion on this point - will split this up if you think it's necessary - your call.

[aleksuss]

It would be better to make this with TryFrom trait:

impl TryFrom<u8> for CurveType {
    type Error = ParsePublicKeyError;
   
    fn from(val: u8) -> Result<Self, Self::Error> {
         match val {
             0 => Ok(CurveType::ED25519),
             1 => Ok(CurveType::SECP256K1),
             _ => Err(ParsePublicKeyError {
                 kind: ParsePublicKeyErrorKind::UnknownCurve,
             }),
         }
    }
}

[joshuajbouw]

I'm confused; what does a relayer have anything to do with the management of the contract? This must be limited to admins.

[vzctl]

I'm confused; what does a relayer have anything to do with the management of the contract? This must be limited to admins.

After talking with Josh we decided to remove the hardcoded list of relayer manager accounts and reuse owner_id which is already a part of the engine state.

[aleksuss]

Implemented it #792

[joshuajbouw]

Closing in favour of #792