Implement Gating Requirements module

[jnaviask]

Description

The requirements module is the core of the gating feature. It is where the actual "gating" validation takes place -- involving querying a chain or checking against a hardcoded allow list. We will use our existing TBC (token balance cache) for performing checks against the chain.

The module can depend on the database for validating the "source" field (as needed for TBC), but ideally only via canonical identifiers, such as eth chain id and cosmos moniker (cf. #4951). It should otherwise strive to be as self-contained as possible.

Subtasks

Feel free to break out into separate tickets as needed.

• [x] #4958 Implement stub interfaces to unblock route implementation.
• [x] #4958 Implement unit tests for stubs.
• [x] Implement the specified logic.

Engineering Requirements

From tech spec, the module should provide:

• A function to validate a requirement definition against the specification.
  • Used by Create/UpdateGroup.
  • On failure, should return human-readable error about why a rule is invalid.
• A function to check a rule against a provided address.
  • Used by CheckGroupMember.
  • Should effectively run requirement check on rule + data (incl “source”) and return human-readable message if check fails.
  • Does not need caching at this time.

Additional context

Example of valid requirements object:

{
  "requirements": [
    {
      "rule": "threshold",
      "data": {
        "threshold": "1000000",
        "source": {
          "source_type": "erc20",
          "eth_chain_id": 1,
          "contract_address": "0xFEED"
        }
      }
    },
    {
      "rule": "threshold",
      "data": {
        "threshold": "1000000",
        "source": {
          "source_type": "erc721",
          "eth_chain_id": 1,
          "contract_address": "0xFEED2"
        }
      }
    },
    {
      "rule": "threshold",
      "data": {
        "threshold": "1000000",
        "source": {
          "source_type": "eth_native",
          "eth_chain_id": 1,
        }
      }
    },
    {
      "rule": "threshold",
      "data": {
        "threshold": "1000000",
        "source": {
          "source_type": "cosmos_native",
          "cosmos_chain_id": "osmosis-1",
          "token_symbol": "osmo"
        }
      }
    },
    {
      "rule": "allow",
      "data": {
        "allow": ["0x1234", "0x5667"]
      }
    }
  ]
}

Draft of JSON schema (we may want to modify the schema to only specify single requirements rather than an array of requirements):

{
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "type": "object",
  "version": "0.0.0",
  "properties": {
    "requirements": {
      "type": "array",
      "minItems": 1,
      "maxItems": 10,
      "items": { "$ref": "#/$defs/requirement" }
    }
  },
  "required": [
    "requirements"
  ],
  "additionalProperties": false,
  "$defs": {
    "requirement": {
      "oneOf": [
        /*
         * threshold rule:
         *   given an address, query an externally defined 
         *   attribute of the address, then check whether
         *   the attribute value is >= the provided threshold
         */
        {
          "type": "object",
          "properties": {
            "rule": { "const": "threshold" },
            "data": {
              "type": "object",
              "properties": {
                "threshold": {
                  "type": "string",
                  "pattern": "^\\d+$" /* numeric string */
                },
                "source": { "type": "object" }
              },
              "additionalProperties": false
            }
          },
          "additionalProperties": false
        },
        /*
         * allow list rule:
         *   given an address, check whether it is included in a 
         *   provided list of valid address strings
         */
        {
          "type": "object",
          "properties": {
            "rule": { "const": "allow" },
            "data": {
              "type": "object",
              "properties": {
                "allow": {
                  "type": "array",
                  "items": {
                    "type": "string"
                  },
                  "minItems": 1,
                  "maxItems": 100
                },
              "additionalProperties": false
              }
            }
          },
          "additionalProperties": false
        }
      ]
    }
  }
}

Open Questions

• Should schema include a "source_type" field? Or, what kind of validation can we do against source?
• Is it worth validating addresses in the allowlist at the schema level, as we have some guarantees about their shape? Or should we leave it entirely to our own logic?

[ianrowan]

A couple questions here:

1. Is it right to assume we want the requirements module to be located in packages/commonwealth/server? Or do we expect to use it on the client or even within other packages?

2. For Json validation I was just thinking about tradeoffs of using JSON schema validation vs typescript type assertion. I plan to build detailed types to support the requirements object(ie requirements types and data subtypes for the data field based on source) Given the json our server will be receiving won't be user generated(unlike templates) we have more of a guarantee the objects will fit our type from our client code, especially if we share the type to the client(like links, etc). So the question in summary is -- can types/assertation be used in place of a json schema given we can build the requirements with type assertations on the client as well and they wont be user generated/provided in their raw form?

CC @jnaviask @MuonShot

[jnaviask]

@ianrowan Great questions.

1. We should allow as much validation code as possible to live outside of a dependency on the server. This is because we should be able to surface errors on the client without making round-trips. However, we also will have certain server-side dependencies e.g. resolving the source field, so I think it's safe to start in the server folder (although attempting to minimize points of contact) and move it later if needed.

2. We should assume that a user can make an API call with any possible shape, so we can't make assumptions about the form of JSON received. However, I'm agnostic on the specific approach we use to validate the received JSON objects. Whichever makes the most sense to you is fine as long as it's secure.

[ianrowan]

@jnaviask

1. That makes sense, thanks. I think this is another case for at least sharing/duplicating types server/client. As long as the client is packing UI input into our requirement type we can surface errors/incompatibilities on the client before a request is made by enforcing typed requests(while also checking on the server for the API use case)

2. I think we'll be depending on types enough to validate the incoming json upon attempt to force it to the type. We'll have multiple lines of defense given the Groups table db model type is set up with a typed requirement column similar to links so naturally incorrect json shouldnt make it to the DB.

[ianrowan]

Running into a potential inefficiency with TBC here related to our goal of keeping requirements(chain id's specifically) agnostic to our database/data. This is mainly a cosmos issue but evm is still backwards.

TBC at a minimum requires nodeId for fetchUserBalance this by definition is the nodeId from our db so this is already creating a dependency on the db. For evm we can luckily pull the preloaded/cached list of nodes and find where eth_chain_id == requirement.eth_chain_id and then pull that nodeId to make the balance call.(this is still a backward way to do this imo)

On cosmos however, the goal was to hold the actual cosmos_chain_id in the requirement which does not have any correlation to a nodeId so the only way to(maybe b/c our chain_ids don't map 1:1 to actual cosmos chain ids) would be to query Chains and then find the chainNode of the chain and then use the nodeId for the TBC request.

All this said to support our goal of agnostic requirements with identifiers that are accurate outside of our database TBC will need a moderately complicated overhaul given the current design(What it needs to be able to do is easy, but manipulating the current arch to it is harder).

So I think we have 2 solutions here:

1. Scrap the plan to have chain identifiers in threshold requirements self contained and true to the actual chain. Store our ChainNode id instead.

2. Rework TBC to accept balanceTypes(ie cosmos, ethereum) and nodes we can generate on the fly and pass in(ie push any RPC endpoint in to a ChainNode object and call tbc)

   • There's also a consideration here if we do go this route if we want to store the cosmos/eth_chain_id or just store the RPC endpoint we should use. There's benefits to storing the RPC as this will have to be something we develop methods to look up EVERY time we do a requirement check, whereas the client/server calculating the RPC to use at the time of requirement creation could make this a one time thing and balance checks very straight forward. This is the solution I prefer and would just add server/route work to fetch a working RPC(most of the time just from our own data) per requirement created.

Update: To further expand on this the downstream requirements in tbc(ie the actual getBalance logic) from the ChainNode object for Ethereum is just the ChainNode.url || chainNode.private_url. Cosmos requires ChainNode.bech32 on top of the url + private_url. So this would be the minimal data required to use TBC from the module after also creating a tbc method that will accept a non-db derived IChainNode object

CC: @jnaviask @CowMuon

[jnaviask]

@ianrowan My understanding is that the intent of #4951 is to resolve the above issue (adding moniker to the ChainNode table so we can always perform "external id <> node" reference). Would this solve the problem?

[ianrowan]

@jnaviask yes that would solve most of the problem. We still have the consideration of adding a new method to TBC that allows us to pass canonical ids for evm/cosmos that resolves to the correct node within TBC. This would address the issue in the design of TBC where the fetchBalance methods always require a nodeId param which is the Id of the node in our DB. FYI this is the current method:

public async fetchUserBalance(
    network: ChainNetwork,
    nodeId: number, // id of the node in ChainNodes Table
    userAddress: string,
    contractAddress?: string
  )

I'm guessing there wouldnt be a problem with me extending tbc but just to pass it by: in a new method I'd just replace nodeId with a chainId string param and resolve to the correct node within the tbc method.

That said its important to note that for work in #4951 The IChainNode model will need to be updated in commonwealth/packages/token-balance-cache/src/types.ts to reflect the new model for use in the tbc. And this ticket(the cosmos portion) will be blocked in principle until #4951 is complete although most work can go through async