Add helper methods to compare risks (#1763)

dydxprotocol · Jun 25, 2024 · f7984a6 · f7984a6
1 parent 568e95a
commit f7984a6
Show file tree

Hide file tree

Showing 3 changed files with 170 additions and 8 deletions.
diff --git a/protocol/lib/margin/risk.go b/protocol/lib/margin/risk.go
@@ -53,6 +53,34 @@ func (a *Risk) IsLiquidatable() bool {
 	return a.MMR.Sign() > 0 && a.MMR.Cmp(a.NC) > 0
 }
 
+// Cmp compares the risks of two accounts.
+// Returns -1 if a is less risky than b, 0 if they are equally risky, and 1 if a is more risky than b.
+
+// Note that here we are effectively checking that
+// `a.NetCollateral / a.MaintenanceMargin >= b.NetCollateral / b.MaintenanceMargin`.
+// However, to avoid rounding errors, we factor this as
+// `a.NetCollateral * b.MaintenanceMargin >= b.NetCollateral * a.MaintenanceMargin`.
+func (a *Risk) Cmp(b Risk) int {
+	ANcMultBMmr := new(big.Int).Mul(a.NC, b.MMR)
+	BNcMultAMmr := new(big.Int).Mul(b.NC, a.MMR)
+
+	result := BNcMultAMmr.Cmp(ANcMultBMmr)
+
+	// Special case: if the ratios are the same, compare the net collateral and maintenance margin
+	// for strict ordering.
+	if result == 0 {
+		if a.MMR.Sign() == 0 && b.MMR.Sign() == 0 {
+			// If both MMRs are zero, then the account with less net collateral is more
+			// risky.
+			return b.NC.Cmp(a.NC)
+		}
+
+		// otherwise, the account with the higher maintenance margin is more risky.
+		return a.MMR.Cmp(b.MMR)
+	}
+	return result
+}
+
 func mustExist(i *big.Int) *big.Int {
 	if i == nil {
 		return new(big.Int)

diff --git a/protocol/lib/margin/risk_test.go b/protocol/lib/margin/risk_test.go
@@ -220,3 +220,144 @@ func TestRisk_IsLiquidatable(t *testing.T) {
 		})
 	}
 }
+
+func TestRisk_Cmp(t *testing.T) {
+	tests := map[string]struct {
+		firstNC  *big.Int
+		firstMMR *big.Int
+
+		secondNC  *big.Int
+		secondMMR *big.Int
+
+		expected int
+	}{
+		// Normal cases: different ratios.
+		"first is less risky than second": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"first is less risky than second - second has zero TNC": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(0),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"first is less risky than second - second has negative TNC": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(-100),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"first is less risky than second - both have negative TNC": {
+			firstNC:   big.NewInt(-100),
+			firstMMR:  big.NewInt(150),
+			secondNC:  big.NewInt(-100),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"first is more risky than second": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(150),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  1,
+		},
+		"first is more risky than second - first has zero TNC": {
+			firstNC:   big.NewInt(0),
+			firstMMR:  big.NewInt(150),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  1,
+		},
+		"first is more risky than second - first has negative TNC": {
+			firstNC:   big.NewInt(-100),
+			firstMMR:  big.NewInt(150),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  1,
+		},
+		"first is more risky than second - both hahave negative TNC": {
+			firstNC:   big.NewInt(-100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(-100),
+			secondMMR: big.NewInt(100),
+			expected:  1,
+		},
+		"first is less risky than second - first has zero MMR": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(0),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"first is more risky than second - second has zero MMR": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(100),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(0),
+			expected:  1,
+		},
+		// special cases: ratio is the same
+		"special case: equally risky": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(100),
+			secondNC:  big.NewInt(100),
+			secondMMR: big.NewInt(100),
+			expected:  0,
+		},
+		"special case: same ratio, tie break by MMR": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(200),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"special case: first with zero NC and second with zero NC, tie break by MMR": {
+			firstNC:   big.NewInt(0),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(0),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"special case: first with zero NC and MMR, tie break by MMR": {
+			firstNC:   big.NewInt(0),
+			firstMMR:  big.NewInt(0),
+			secondNC:  big.NewInt(200),
+			secondMMR: big.NewInt(100),
+			expected:  -1,
+		},
+		"special case: second with zero NC and MMR, tie break by MMR": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(50),
+			secondNC:  big.NewInt(0),
+			secondMMR: big.NewInt(0),
+			expected:  1,
+		},
+		"special case: first with zero MMR and second with zero MMR, tie break by NC": {
+			firstNC:   big.NewInt(100),
+			firstMMR:  big.NewInt(0),
+			secondNC:  big.NewInt(200),
+			secondMMR: big.NewInt(0),
+			expected:  1,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			firstRisk := margin.Risk{
+				MMR: tc.firstMMR,
+				NC:  tc.firstNC,
+			}
+			secondRisk := margin.Risk{
+				MMR: tc.secondMMR,
+				NC:  tc.secondNC,
+			}
+			require.Equal(t, tc.expected, firstRisk.Cmp(secondRisk))
+		})
+	}
+}
diff --git a/protocol/x/subaccounts/lib/updates.go b/protocol/x/subaccounts/lib/updates.go
@@ -128,16 +128,9 @@ func IsValidStateTransitionForUndercollateralizedSubaccount(
 		return underCollateralizationResult
 	}
 
-	// Note that here we are effectively checking that
-	// `newNetCollateral / newMaintenanceMargin >= curNetCollateral / curMaintenanceMargin`.
-	// However, to avoid rounding errors, we factor this as
-	// `newNetCollateral * curMaintenanceMargin >= curNetCollateral * newMaintenanceMargin`.
-	newNcOldMmr := new(big.Int).Mul(riskNew.NC, riskCur.MMR)
-	oldNcNewMmr := new(big.Int).Mul(riskCur.NC, riskNew.MMR)
-
 	// The subaccount is not well-collateralized, and the state transition leaves the subaccount in a
 	// "more-risky" state (collateral relative to margin requirements is decreasing).
-	if oldNcNewMmr.Cmp(newNcOldMmr) > 0 {
+	if riskNew.Cmp(riskCur) > 0 {
 		return underCollateralizationResult
 	}