Merge pull request #2763 from oasislabs/ptrus/fix/comission-check

staking/api/commission: fix possible panic in validation check
oasisprotocol · Mar 13, 2020 · 8001e4d · 8001e4d
2 parents fe064e6 + ddcf8ee
commit 8001e4d
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 2 deletions.
diff --git a/.changelog/2763.bugfix.md b/.changelog/2763.bugfix.md
@@ -0,0 +1,4 @@
+staking/api/commission: fix possible panic in validation check
+
+The validation check would panic whenever the number of bound steps was
+greater than `rate_steps + 2`.
diff --git a/go/staking/api/commission.go b/go/staking/api/commission.go
@@ -64,8 +64,8 @@ func (cs *CommissionSchedule) validateNondegenerate(rules *CommissionScheduleRul
 		if step.Start%rules.RateChangeInterval != 0 {
 			return fmt.Errorf("bound step %d start epoch %d not aligned with commission rate change interval %d", i, step.Start, rules.RateChangeInterval)
 		}
-		if i > 0 && step.Start <= cs.Rates[i-1].Start {
-			return fmt.Errorf("bound step %d start epoch %d not after previous step start epoch %d", i, step.Start, cs.Rates[i-1].Start)
+		if i > 0 && step.Start <= cs.Bounds[i-1].Start {
+			return fmt.Errorf("bound step %d start epoch %d not after previous step start epoch %d", i, step.Start, cs.Bounds[i-1].Start)
 		}
 		if step.RateMin.Cmp(CommissionRateDenominator) > 0 {
 			return fmt.Errorf("bound step %d minimum rate %v/%v over unity", i, step.RateMin, CommissionRateDenominator)

diff --git a/go/staking/api/commission_test.go b/go/staking/api/commission_test.go
@@ -687,6 +687,58 @@ func TestCommissionSchedule(t *testing.T) {
 	}
 	require.NoError(t, cs.PruneAndValidateForGenesis(&rules, 0), "simultaneous rate and bound change")
 
+	cs = CommissionSchedule{
+		Rates: []CommissionRateStep{
+			{
+				Start: 0,
+				Rate:  mustInitQuantity(t, 50_000),
+			},
+		},
+		Bounds: []CommissionRateBoundStep{
+			{
+				Start:   0,
+				RateMin: mustInitQuantity(t, 0),
+				RateMax: mustInitQuantity(t, 100_000),
+			},
+			{
+				Start:   10,
+				RateMin: mustInitQuantity(t, 0),
+				RateMax: mustInitQuantity(t, 90_000),
+			},
+			{
+				Start:   20,
+				RateMin: mustInitQuantity(t, 0),
+				RateMax: mustInitQuantity(t, 80_000),
+			},
+		},
+	}
+	require.NoError(t, cs.PruneAndValidateForGenesis(&rules, 0), "valid where len(rates) < len(bounds)")
+
+	cs = CommissionSchedule{
+		Rates: []CommissionRateStep{
+			{
+				Start: 0,
+				Rate:  mustInitQuantity(t, 50_000),
+			},
+			{
+				Start: 10,
+				Rate:  mustInitQuantity(t, 60_000),
+			},
+			{
+				Start: 20,
+				Rate:  mustInitQuantity(t, 70_000),
+			},
+		},
+		Bounds: []CommissionRateBoundStep{
+			{
+				Start:   0,
+				RateMin: mustInitQuantity(t, 0),
+				RateMax: mustInitQuantity(t, 100_000),
+			},
+		},
+	}
+	require.NoError(t, cs.PruneAndValidateForGenesis(&rules, 0), "valid where len(bounds) < len(rates)")
+
 	cs = CommissionSchedule{
 		Rates: []CommissionRateStep{
 			{