moved the "join table" to a separate field in the entity schema

narval-xyz · Nov 12, 2024 · 5483285 · 5483285
1 parent 082544b
commit 5483285
Show file tree

Hide file tree

Showing 14 changed files with 389 additions and 155 deletions.
diff --git a/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts b/apps/policy-engine/src/open-policy-agent/core/__test__/unit/open-policy-agent.engine.spec.ts
@@ -76,6 +76,7 @@ describe('OpenPolicyAgentEngine', () => {
         accountGroups: [],
         userGroups: [],
         groups: [],
+        groupMembers: [],
         userAccounts: [],
         users: [],
         accountGroupMembers: [],

diff --git a/apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/evaluation.util.spec.ts b/apps/policy-engine/src/open-policy-agent/core/util/__test__/unit/evaluation.util.spec.ts
@@ -360,18 +360,17 @@ describe('toData', () => {
         )
       })
     })
-
     it('indexes legacy groups with members by lower case id', () => {
       expect.assertions(3)
 
       const { entities } = toData({
         ...FIXTURE.ENTITIES,
         userGroupMembers: [
-          ...FIXTURE.ENTITIES.userGroupMembers,
+          ...(FIXTURE.ENTITIES?.userGroupMembers || []),
           { userId: 'test-legacy-alice', groupId: 'test-legacy-group' }
         ],
         accountGroupMembers: [
-          ...FIXTURE.ENTITIES.accountGroupMembers,
+          ...(FIXTURE.ENTITIES?.accountGroupMembers || []),
           { accountId: 'test-legacy-account', groupId: 'test-legacy-group' }
         ],
         accountGroups: [{ id: 'test-legacy-group' }],

diff --git a/apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts b/apps/policy-engine/src/open-policy-agent/core/util/evaluation.util.ts
@@ -192,43 +192,23 @@ export const toInput = (params: {
 }
 
 export const toData = (entities: Entities): Data => {
-  const groups = new Map<string, Group>()
-  const userGroups = new Map<string, UserGroup>()
-  const accountGroups = new Map<string, AccountGroup>()
-
-  const {
-    userGroups: legacyUserGroupsEntities = [],
-    accountGroups: legacyAccountGroupsEntities = [],
-    groups: newGroups = []
-  } = entities
-
-  // Process user group members
-  entities.userGroupMembers.forEach(({ userId, groupId }) => {
-    const id = groupId.toLowerCase()
-    const isInNewGroups = newGroups.some((group) => group.id === id)
-    const isInLegacyGroups = legacyUserGroupsEntities.some((group) => group.id === id)
-
-    if (isInNewGroups) {
-      groups.get(id)?.users.push(userId) || groups.set(id, { id, users: [userId], accounts: [] })
-    }
-    if (isInLegacyGroups) {
-      userGroups.get(id)?.users.push(userId) || userGroups.set(id, { id, users: [userId] })
-    }
-  })
+  const userGroupMembers = entities.userGroupMembers || []
+  const accountGroupMembers = entities.accountGroupMembers || []
+  const groupMembers = entities.groupMembers || []
 
-  // Process account group members
-  entities.accountGroupMembers.forEach(({ accountId, groupId }) => {
+  const userGroups = userGroupMembers.reduce((groups, { userId, groupId }) => {
     const id = groupId.toLowerCase()
-    const isInNewGroups = newGroups.some((group) => group.id === id)
-    const isInLegacyGroups = legacyAccountGroupsEntities.some((group) => group.id === id)
+    const group = groups.get(id)
 
-    if (isInNewGroups) {
-      groups.get(id)?.accounts.push(accountId) || groups.set(id, { id, users: [], accounts: [accountId] })
-    }
-    if (isInLegacyGroups) {
-      accountGroups.get(id)?.accounts.push(accountId) || accountGroups.set(id, { id, accounts: [accountId] })
+    if (group) {
+      return groups.set(id, {
+        id: groupId,
+        users: group.users.concat(userId)
+      })
+    } else {
+      return groups.set(groupId, { id: groupId, users: [userId] })
     }
-  })
+  }, new Map<string, UserGroup>())
 
   const accountAssignees = entities.userAccounts.reduce((assignees, { userId, accountId }) => {
     const account = assignees.get(accountId)
@@ -245,15 +225,44 @@ export const toData = (entities: Entities): Data => {
     assignees: accountAssignees.get(account.id) || []
   }))
 
+  const accountGroups = accountGroupMembers.reduce((groups, { accountId, groupId }) => {
+    const group = groups.get(groupId)
+
+    if (group) {
+      return groups.set(groupId, {
+        id: groupId,
+        accounts: group.accounts.concat(accountId)
+      })
+    } else {
+      return groups.set(groupId, { id: groupId, accounts: [accountId] })
+    }
+  }, new Map<string, AccountGroup>())
+
+  const mergedGroups = groupMembers.reduce((groups, member) => {
+    const group = groups.get(member.groupId) ?? {
+      id: member.groupId,
+      users: [],
+      accounts: []
+    }
+
+    const newGroup = {
+      ...group,
+      users: member.type === 'user' ? [...group.users, member.userId] : group.users,
+      accounts: member.type === 'account' ? [...group.accounts, member.accountId] : group.accounts
+    }
+
+    return groups.set(member.groupId, newGroup)
+  }, new Map<string, Group>())
+
   const data: Data = {
     entities: {
       addressBook: indexBy('id', entities.addressBook),
       tokens: indexBy('id', entities.tokens),
       users: indexBy('id', entities.users),
-      groups: indexBy('id', Object.fromEntries(groups)),
-      accountGroups: Object.fromEntries(accountGroups),
       userGroups: Object.fromEntries(userGroups),
-      accounts: indexBy('id', accounts)
+      accounts: indexBy('id', accounts),
+      accountGroups: Object.fromEntries(accountGroups),
+      groups: Object.fromEntries(mergedGroups)
     }
   }
 

diff --git a/packages/armory-e2e-testing/src/resource/entity/defi-interaction.json b/packages/armory-e2e-testing/src/resource/entity/defi-interaction.json
@@ -228,24 +228,6 @@
       "decimals": 6
     }
   ],
-  "userGroupMembers": [
-    {
-      "groupId": "uniswap-trading",
-      "userId": "test-eric-user-uid"
-    },
-    {
-      "groupId": "uniswap-trading",
-      "userId": "test-bob-user-uid"
-    },
-    {
-      "groupId": "test-treasury-group-uid",
-      "userId": "test-dave-user-uid"
-    },
-    {
-      "groupId": "test-treasury-group-uid",
-      "userId": "test-carol-user-uid"
-    }
-  ],
   "userAccounts": [
     {
       "accountId": "eip155:eoa:0x76d1b7f9b3F69C435eeF76a98A415332084A856F",
@@ -290,22 +272,46 @@
       "role": "manager"
     }
   ],
-  "accountGroupMembers": [
+  "groupMembers": [
     {
+      "type": "account",
       "groupId": "uniswap-trading",
       "accountId": "eip155:eoa:0x9f38879167acCf7401351027EE3f9247A71cd0c5"
     },
     {
+      "type": "account",
       "groupId": "uniswap-trading",
       "accountId": "eip155:eoa:0x0f610AC9F0091f8F573c33f15155afE8aD747495"
     },
     {
+      "type": "account",
       "groupId": "test-treasury-group-uid",
       "accountId": "eip155:eoa:0x0301e2724a40E934Cce3345928b88956901aA127"
     },
     {
+      "type": "account",
       "groupId": "test-treasury-group-uid",
       "accountId": "eip155:eoa:0x76d1b7f9b3F69C435eeF76a98A415332084A856F"
+    },
+    {
+      "type": "user",
+      "groupId": "uniswap-trading",
+      "userId": "test-eric-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "uniswap-trading",
+      "userId": "test-bob-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-treasury-group-uid",
+      "userId": "test-dave-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-treasury-group-uid",
+      "userId": "test-carol-user-uid"
     }
   ],
   "groups": [

diff --git a/packages/armory-e2e-testing/src/resource/entity/test.default.json b/packages/armory-e2e-testing/src/resource/entity/test.default.json
@@ -198,28 +198,6 @@
       "decimals": 6
     }
   ],
-  "userGroupMembers": [
-    {
-      "groupId": "test-engineering-group-uid",
-      "userId": "test-alice-user-uid"
-    },
-    {
-      "groupId": "test-engineering-group-uid",
-      "userId": "test-carol-user-uid"
-    },
-    {
-      "groupId": "test-treasury-group-uid",
-      "userId": "test-bob-user-uid"
-    },
-    {
-      "groupId": "test-treasury-group-uid",
-      "userId": "test-eric-user-uid"
-    },
-    {
-      "groupId": "test-treasury-group-uid",
-      "userId": "test-dave-user-uid"
-    }
-  ],
   "userAccounts": [
     {
       "accountId": "eip155:eoa:0x76d1b7f9b3F69C435eeF76a98A415332084A856F",
@@ -264,22 +242,51 @@
       "role": "manager"
     }
   ],
-  "accountGroupMembers": [
+  "groupMembers": [
     {
+      "type": "account",
       "groupId": "test-engineering-group-uid",
       "accountId": "eip155:eoa:0x9f38879167acCf7401351027EE3f9247A71cd0c5"
     },
     {
+      "type": "account",
       "groupId": "test-engineering-group-uid",
       "accountId": "eip155:eoa:0x0f610AC9F0091f8F573c33f15155afE8aD747495"
     },
     {
+      "type": "account",
       "groupId": "test-treasury-group-uid",
       "accountId": "eip155:eoa:0x0301e2724a40E934Cce3345928b88956901aA127"
     },
     {
+      "type": "account",
       "groupId": "test-treasury-group-uid",
       "accountId": "eip155:eoa:0x76d1b7f9b3F69C435eeF76a98A415332084A856F"
+    },
+    {
+      "type": "user",
+      "groupId": "test-engineering-group-uid",
+      "userId": "test-alice-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-engineering-group-uid",
+      "userId": "test-carol-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-treasury-group-uid",
+      "userId": "test-bob-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-treasury-group-uid",
+      "userId": "test-eric-user-uid"
+    },
+    {
+      "type": "user",
+      "groupId": "test-treasury-group-uid",
+      "userId": "test-dave-user-uid"
     }
   ],
   "groups": [

diff --git a/...src/resource/policy/checkSpendingLimit/treasury-groupMember-can-transfer-1-eth-fixed.json b/...src/resource/policy/checkSpendingLimit/treasury-groupMember-can-transfer-1-eth-fixed.json
@@ -2,6 +2,10 @@
   "id": "treasury-members-can-transfer-1-eth",
   "description": "treasury group members can transfer 1 ETH",
   "when": [
+    {
+      "criterion": "checkPrincipalGroup",
+      "args": ["test-treasury-group-uid"]
+    },
     {
       "criterion": "checkSpendingLimit",
       "args": {

diff --git a/...c/resource/policy/checkSpendingLimit/treasury-groupMember-can-transfer-1-eth-rolling.json b/...c/resource/policy/checkSpendingLimit/treasury-groupMember-can-transfer-1-eth-rolling.json
@@ -2,6 +2,10 @@
   "id": "treasury-members-can-transfer-1-eth",
   "description": "treasury group members can transfer 1 ETH",
   "when": [
+    {
+      "criterion": "checkPrincipalGroup",
+      "args": ["test-treasury-group-uid"]
+    },
     {
       "criterion": "checkSpendingLimit",
       "args": {

diff --git a/...urce/policy/checkSpendingLimit/treasury-legacyGroupMember-can-transfer-1-eth-rolling.json b/...urce/policy/checkSpendingLimit/treasury-legacyGroupMember-can-transfer-1-eth-rolling.json
@@ -2,6 +2,10 @@
   "id": "treasury-members-can-transfer-1-eth",
   "description": "treasury group members can transfer 1 ETH",
   "when": [
+    {
+      "criterion": "checkPrincipalGroup",
+      "args": ["test-treasury-group-uid"]
+    },
     {
       "criterion": "checkSpendingLimit",
       "args": {
@@ -16,10 +20,6 @@
           "tokens": ["eip155:1/slip44:60"]
         }
       }
-    },
-    {
-      "criterion": "checkPrincipalGroup",
-      "args": ["test-treasury-group-uid"]
     }
   ],
   "then": "permit"