From ddcf8ee3d37e4e8a3c0102cb38c09de1f57c76ea Mon Sep 17 00:00:00 2001 From: ptrus Date: Fri, 13 Mar 2020 10:51:12 +0100 Subject: [PATCH] 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`. --- .changelog/2763.bugfix.md | 4 +++ go/staking/api/commission.go | 4 +-- go/staking/api/commission_test.go | 52 +++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 .changelog/2763.bugfix.md diff --git a/.changelog/2763.bugfix.md b/.changelog/2763.bugfix.md new file mode 100644 index 00000000000..4638f874370 --- /dev/null +++ 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 index 11a2d35bf43..58fad617635 100644 --- 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 index d12b27b21d1..2fd2c57d2e5 100644 --- 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{ {