fix on calculateSpendings

narval-xyz · Nov 6, 2024 · 84b605f · 84b605f
1 parent ccb4e53
commit 84b605f
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 26 deletions.
diff --git a/...src/resource/open-policy-agent/rego/armory/criteria/__test__/checkSpendingLimit_test.rego b/...src/resource/open-policy-agent/rego/armory/criteria/__test__/checkSpendingLimit_test.rego
@@ -239,3 +239,34 @@ test_calculateCurrentSpendingsForUserOperationIntent if {
 
 	res == 2400000000000000000
 }
+
+test_calculateCurrentSpendingsDoesntAuthorizeUserNotInGroup if {
+	conditions = {
+		"timeWindow": {
+			"type": "rolling",
+			"value": (12 * 60) * 60,
+		},
+		"filters": {
+			"tokens": {"eip155:137/erc20:0x2791bca1f2de4661ed88a30c99a7a9449aa84174"},
+			"userGroup": {"I should not match at all"},
+		},
+	}
+
+	not calculateCurrentSpendings(conditions) with input as testData.requestWithEip1559Transaction with data.entities as testData.entities
+}
+
+test_spendingLimitByUserGroupDoesntAuthorizeEveryoneFromFunction if {
+	res := checkSpendingLimit({
+		"limit": "1000000000000000000",
+		"operator": "lte",
+		"timeWindow": {
+			"type": "rolling",
+			"value": 86400,
+		},
+		"filters": {
+			"perPrincipal": true,
+			"tokens": {"eip155:137/erc20:0x2791bca1f2de4661ed88a30c99a7a9449aa84174"},
+			"userGroups": {"I should not match at all"},
+		},
+	}) with input as testData.requestWithEip1559Transaction with data.entities as testData.entities
+}
diff --git a/...rce/open-policy-agent/rego/armory/criteria/__test__/policies/spendings_policies_test.rego b/...rce/open-policy-agent/rego/armory/criteria/__test__/policies/spendings_policies_test.rego
@@ -125,6 +125,27 @@ forbid[{"policyId": "spendingLimitByAccountGroup"}] := reason if {
 	}
 }
 
+permit[{"policyId": "spendingLimitByUserGroupNonExistentGroup"}] := reason if {
+	checkPrincipalRole("member")
+	checkSpendingLimit({
+		"limit": "5000000000",
+		"operator": "gt",
+		"currency": "fiat:usd",
+		"timeWindow": {
+			"type": "rolling",
+			"value": (12 * 60) * 60,
+		},
+		"filters": {"userGroups": {"non-existent-group"}},
+	})
+
+	reason = {
+		"type": "permit",
+		"policyId": "spendingLimitByUserGroupNonExistentGroup",
+		"approvalsSatisfied": [],
+		"approvalsMissing": [],
+	}
+}
+
 # If Alice transfers >$5k usd value of USDC in a 12 hour rolling window, then require approvals
 permit[{"policyId": "spendingLimitWithApprovals"}] := reason if {
 	checkAccountAssigned

diff --git a/...src/resource/open-policy-agent/rego/armory/criteria/__test__/policies/spendings_test.rego b/...src/resource/open-policy-agent/rego/armory/criteria/__test__/policies/spendings_test.rego
@@ -41,6 +41,10 @@ test_spendingLimitByAccountResource if {
 	}
 }
 
+test_spendingLimitByUserGroupDoesntAuthorizeEveryone if {
+	res = permit[{"policyId": "spendingLimitByUserGroupNonExistentGroup"}] with input as spendingLimitReq with data.entities as testData.entities
+}
+
 test_spendingLimitByUserGroup if {
 	spendingLimitByUserGroupReq = object.union(spendingLimitReq, {
 		"principal": {"userId": "test-bar-uid"},

diff --git a/...policy-engine/src/resource/open-policy-agent/rego/armory/criteria/checkSpendingLimit.rego b/...policy-engine/src/resource/open-policy-agent/rego/armory/criteria/checkSpendingLimit.rego
@@ -65,51 +65,32 @@ checkSpendingOperator(spendings, operator, limit) if {
 }
 
 # Check Spending Limit
-
 calculateCurrentSpendings(params) := result if {
 	conditions = object.union(spendingWildcardConditions, params)
 	timeWindow = conditions.timeWindow
 	filters = conditions.filters
 	transfers = array.concat(feeds.transferFeed, util.intentTransferObjects)
 
-	result = sum([spending |
+	validTransfers := [transfer |
 		some transfer in transfers
-
-		# filter by principal
 		util.checkTransferByPrincipal(transfer.initiatedBy, filters.perPrincipal)
-
-		# filter by tokens
 		util.checkTransferCondition(transfer.token, filters.tokens)
-
-		# filter by users
 		util.checkTransferCondition(transfer.initiatedBy, filters.users)
-
-		# filter by resource accounts
 		util.checkTransferCondition(transfer.resourceId, filters.resources)
-
-		# filter by destination accounts
 		util.checkTransferCondition(transfer.to, filters.destinations)
-
-		# filter by chains
 		util.checkTransferCondition(lib.numberToString(transfer.chainId), filters.chains)
-
-		# filter by user groups
 		util.checkTransferByUserGroups(transfer.initiatedBy, filters.userGroups)
-
-		# filter by account groups
 		util.checkTransferByAccountGroups(transfer.from, filters.accountGroups)
-
-		# filter by start date
 		util.checkTransferFromStartDate(transfer.timestamp, timeWindow)
-
-		# filter by end date
 		util.checkTransferToEndDate(transfer.timestamp, timeWindow)
-
-		# filter by time window type
 		util.checkTransferTimeWindow(transfer.timestamp, timeWindow)
+	]
+
+	# Rule will fail if no valid transfers found
+	count(validTransfers) > 0
 
-		spending = calculateTransferSpending(transfer, conditions.currency)
-	])
+	# Calculate sum only for valid transfers
+	result := sum([calculateTransferSpending(transfer, conditions.currency) | some transfer in validTransfers])
 }
 
 checkSpendingLimit(params) if {