Receipt and Action validation is error prone and is missing compile-time checks

[nagisa]

Today we have some data validation in various places of our system in the form of validate_receipt, validate_actions etc.

These functions are then invoked almost arbitrarily to check that the data types are indeed valid to work with further along the line.

The primary issue with how these validation are constructed is the fact that they take an already deserialized type they are supposed to validate (e.g. fn {validate_action} (.., &Action, ..) or fn {validate_receipt} (.., &Receipt, ..).) This poses a major flaw in the setup – both Actions and Receipts can exist in – and indeed be used by – any part of the code without the required verification steps being executed a-priori.

The other foot gun here is that producing these types can itself result in unexpected validation. Consider for example deserialization. If any of the types internal to Action or Receipt is “invalid”, the deserialization of these types can fail with a different error or in a different location, unintentionally causing a protocol change. Similarly there are parts of the codebase where the types stored have knowingly not been validated with an expectation that this is handled sometime in the future (e.g. DeleteAccountAction in near-vm-runner/src/logic/logic.rs,) however constructing those types in the first place requires to construct and store as fields types representing valid values!

---

I'm not quite sure how actionable this issue is really. In my mind the ideal setup would be to have multiple sets of types for each concept. For example Action:

/// Comes out of the database, network, contracts during execution, etc.
///
/// Stores literally a bag of bytes encoded in whatever format.
struct UnvalidatedAction(SerializedBagOfBytesOrWhatever);
/// Valid 
impl UnvalidatedAction {
    /// Deserialize an unvalidated action into its validated form.
    ///
    /// Implements any relevant checks (BEFORE constructing internal fields!)
    fn validate(&self) -> Action { ... }
}

/// Wherever this exists you know you can rely on this being valid.
enum Action { ... }

However it seems like it would be an extreme engineering challenge to actually achieve something like this. Primarily because implementing this would now require going through every affected data type and carefully vetting all the failure modes in every location they are constructed (which also means implicit locations as might be derived by deserialization types.)

Perhaps a slightly easier option would be to convert each of these types to an enum along the lines of

enum MaybeAction {
    Unverified(Action? SerializedBagOfBytes? Idk.)
    Verified(Action)
}

impl MaybeAction {
    fn unwrap(self) -> Action { let Self::Validated(a) = self else { panic!("boom!") }; a }
}

which then would at least check for these sorts of conditions at the runtime. We would still need to rewrite the validation code to operate on bytes rather than already-deserialized structures and to ensure that any errors previously emitted by deserialization continues being emitted in the same place… ARGH! Maybe just store the already deserialized type in either case…?