Add BaseControllerV2 state metadata

[Gudahtt]

State metadata has been added to the new BaseController constructor as a required constructor parameter. The metadata describes how to derive the state that should be persisted, and how to derive an 'anonymized' representation of the controller state.

The metadata describes top-level properties only, but it allows you to define a function to derive the anonymized or persistent state for each property. The only requirement of this derivation function is that the output is also valid JSON.

This is part of the controller redesign (#337).

[Gudahtt]

This depends upon #366

[Gudahtt]

I believe this type includes all valid JSON, but it also includes invalid JSON. As soon as any Record/object type is included in this union type, all sorts of unserializable things like Date objects and classes are allowed as well, for reasons that are beyond me.

So this is used as the basis of the return type, but the isJsonable<> type is still used to wrap it to guarantee the result is valid JSON.

[Gudahtt]

I don't think the Partial is required around that Record<never, never> 🤔 I'll double-check this.

That was meant to cover the empty object case, as I didn't know how to adjust { [prop: string]: Json } to allow zero keys.

[brad-decker]

So the goal with that last type is to throw an error if none of the other cases match first, right?. I think you can just wrap your object with Partial instead of the Record.

[brad-decker]

or, you could add | {} to explicitly allow a empty object.

[Gudahtt]

I tried testing this type with the same TypeScript playground test suite used to validate the IsJsonable type, and discovered that an empty object is already accepted because of the { [prop: string]: Json } entry.

https://www.typescriptlang.org/play?#code/C4TwDgpgBAUgzgewHZQLxSQVwDbagHygCMEFsIBDFQrAWyIgCcCo5hGBLJAcxfmQDaAXRYBvKALCMEYAFyt2XbkPn8UAXwDcAWABQegGaYkAY2AdkUYBDYAeACoA+ABQArRElUeAlPIBuCBwAJlCielBQAPSRUPY2wFAccIkowAAW0CYItGAc2BTmyHrqenpc1owGFCbQAIKh4VAU8nQMjDq6Jfq6JvlwyQBCDboRFACELZj0TB1dZUgVVTVQAGrDowD8k9PtxaW65UxL0AAa603yACIFELP7h5XV0ACa5xRbUNfWd90Px1AALTe2zaLGMQQgBi4ECCP3miyeUAA6m8Ps5vGhHE0kCA4T1kGxsSA0BgcHgKMkqLj9tEoFkcnkmFAAPIAaT01jYziwuG8HU5wGcVWwcAgfI58WcAAZxboBc4AEQK2Xy4QqyWidTqrkCKVCbWC8TNKBSqBa-mSqlNZK1A3OK0UqADO1G+TiIjydiYCAAGjpbvU6j9QXkAk1foATH6FWkOAqhOaJTrw6FjTG40HSbg-QBmfUdPS0+m5cjMdLSADucCTgqQEArnxu6LtAjrDa+EHR+ZrzldGHrjes6LNdodyRWo5x1qgJ0nxMdzzn04BS8dSLt4Mh0KC3h7AFZp60mYRN1C6zuOkA

Though weirdly, without {} or Record included, two of the test cases are showing as invalid because of the error "Index signature is missing". Also it looks like this last entry in the type union was wholly responsible for allowing Date objects and other non-JSON things. 🤔

[Gudahtt]

Oh wow, I think IsJsonable is wrong. It doesn't check for key types that aren't supported by JSON. That error is legitimate.

[Gudahtt]

I'll clean this up in a later PR. There are a few changes needed to these types I think.

[Gudahtt]

This was cleaned up here: #373

[brad-decker]

So this PR is just adding the metadata methods and signatures, but I'm assuming that in a new PR that deriveStateFromMetadata will be called when writing state to a persistent store? Will, there be a need for a deanonmyzier method to restore the state from its persisted anonymized version?

[Gudahtt]

To add a bit more background on how this compromise was reached: I did want to have a metadata format that was fully declarative, but this proved to be difficult. It's declarative for top-level properties, but any fine-grained control over whether nested properties are included requires writing a function.

e.g. for this state:

const state = {
  txMeta: {
    hash: '0x123',
    history: [
      {
        hash: '0x123',
        value: 9,
      },
    ],
    value: 10,
  },
};

If I wanted to keep everything but the balance in each history object, I'd need to write a function to strip it out, like this:

const schema = {
  txMeta: {
    anonymous: false,
    persist: (txMeta) => {
      return {
        history: txMeta.history.map((entry) => {
          return { hash: entry.hash };
        }),
        value: txMeta.value,
      };
    },
  },
};

I would like to revisit the idea of a declarative API that would allow specifying this without using a function, e.g. something like this:

const schema = {
  txMeta: {
    history: {
      '*': {
        hash: {
          anonymous: true.
          persist: true
        },
        value: {
          anonymous: false,
          persist: false,
        },
      },
    },
    value: {
      anonymous: true,
      persist: true,
    },
  },
};

But I abandoned that effort for now because handling this arbitrarily-nested metadata structure proved to be difficult. I did have it working (with some caveats) until I tried adding wildcard support (to apply rules for any entries/properties in a particular object), and then the types totally fell apart.

[Gudahtt]

I'm assuming that in a new PR that deriveStateFromMetadata will be called when writing state to a persistent store?

Exactly, yes. This won't be implemented until the first controller based upon this one is integrated into a product, as the state persistence is handled separately in each product.

Will, there be a need for a deanonmyzier method to restore the state from its persisted anonymized version?

I don't think so, no. The anonymous state isn't meant to be persisted - it's for sending off to Sentry, and for generating state logs for support. The persisted state would not be anonymized. If the anonymization could be reversed, it would not be effective.

[brad-decker]

I don't think so, no. The anonymous state isn't meant to be persisted - it's for sending off to Sentry, and for generating state logs for support. The persisted state would not be anonymized. If the anonymization could be reversed, it would not be effective.

Ah that makes total sense, I erroneously thought both transforms were done before persisting.