diff --git a/vms/platformvm/validators/fee/fee.go b/vms/platformvm/validators/fee/fee.go index be0eae544ea2..23705d8b25a2 100644 --- a/vms/platformvm/validators/fee/fee.go +++ b/vms/platformvm/validators/fee/fee.go @@ -43,7 +43,7 @@ func (s State) AdvanceTime(target gas.Gas, seconds uint64) State { } // CostOf calculates how much to charge based on the dynamic fee mechanism for -// [seconds]. +// seconds. // // This implements the ACP-77 cost over time formula: func (s State) CostOf(c Config, seconds uint64) uint64 { @@ -91,35 +91,24 @@ func (s State) CostOf(c Config, seconds uint64) uint64 { return cost } -// SecondsUntil calculates the number of seconds that it would take to charge at -// least [targetCost] based on the dynamic fee mechanism. The result is capped -// at [maxSeconds]. -func (s State) SecondsUntil(c Config, maxSeconds uint64, targetCost uint64) uint64 { +// SecondsRemaining calculates the maximum number of seconds that a validator +// can pay fees before their fundsRemaining would be exhausted based on the +// dynamic fee mechanism. The result is capped at maxSeconds. +func (s State) SecondsRemaining(c Config, maxSeconds uint64, fundsRemaining uint64) uint64 { // Because this function can divide by prices, we need to sanity check the // parameters to avoid division by 0. if c.MinPrice == 0 { - if targetCost == 0 { - return 0 - } return maxSeconds } // If the current and target are the same, the price is constant. if s.Current == c.Target { - price := gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant) - return secondsUntil( - uint64(price), - maxSeconds, - targetCost, - ) + price := uint64(gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant)) + seconds := fundsRemaining / price + return min(seconds, maxSeconds) } - var ( - cost uint64 - seconds uint64 - err error - ) - for cost < targetCost && seconds < maxSeconds { + for seconds := uint64(0); seconds < maxSeconds; seconds++ { s = s.AdvanceTime(c.Target, 1) // Advancing the time is going to either hold excess constant, @@ -127,41 +116,21 @@ func (s State) SecondsUntil(c Config, maxSeconds uint64, targetCost uint64) uint // equal to 0 after performing one of these operations, it is guaranteed // to always remain 0. if s.Excess == 0 { - zeroExcessCost := targetCost - cost - secondsWithZeroExcess := secondsUntil( - uint64(c.MinPrice), - maxSeconds, - zeroExcessCost, - ) - + secondsWithZeroExcess := fundsRemaining / uint64(c.MinPrice) totalSeconds, err := safemath.Add(seconds, secondsWithZeroExcess) - if err != nil || totalSeconds >= maxSeconds { + if err != nil { + // This is technically unreachable, but makes the code more + // clearly correct. return maxSeconds } - return totalSeconds + return min(totalSeconds, maxSeconds) } - seconds++ - price := gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant) - cost, err = safemath.Add(cost, uint64(price)) - if err != nil { + price := uint64(gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant)) + if price > fundsRemaining { return seconds } + fundsRemaining -= price } - return seconds -} - -// Calculate the number of seconds that it would take to charge at least [cost] -// at [price] every second. The result is capped at [maxSeconds]. -func secondsUntil(price uint64, maxSeconds uint64, cost uint64) uint64 { - // Directly rounding up could cause an overflow. Instead we round down and - // then check if we should have rounded up. - secondsRoundedDown := cost / price - if secondsRoundedDown >= maxSeconds { - return maxSeconds - } - if cost%price == 0 { - return secondsRoundedDown - } - return secondsRoundedDown + 1 + return maxSeconds } diff --git a/vms/platformvm/validators/fee/fee_test.go b/vms/platformvm/validators/fee/fee_test.go index 059fbad61c51..f99be8682bb6 100644 --- a/vms/platformvm/validators/fee/fee_test.go +++ b/vms/platformvm/validators/fee/fee_test.go @@ -20,7 +20,6 @@ const ( hour = 60 * minute day = 24 * hour week = 7 * day - year = 365 * day minPrice = 2_048 @@ -212,7 +211,7 @@ var ( expectedExcess: math.MaxUint64, // Should not overflow }, { - name: "excess=0, current>>target, 11 seconds", + name: "excess=0, current>>target, 10 seconds", state: State{ Current: math.MaxUint32, Excess: 0, @@ -222,9 +221,9 @@ var ( MinPrice: minPrice, ExcessConversionConstant: excessConversionConstant, }, - expectedSeconds: 11, - expectedCost: math.MaxUint64, // Should not overflow - expectedExcess: math.MaxUint32 * 11, + expectedSeconds: 10, + expectedCost: 1_948_429_840_780_833_612, + expectedExcess: math.MaxUint32 * 10, }, } ) @@ -256,13 +255,131 @@ func TestStateCostOf(t *testing.T) { } } -func TestStateSecondsUntil(t *testing.T) { +func TestStateCostOfOverflow(t *testing.T) { + const target = 10_000 + config := Config{ + Target: target, + MinPrice: minPrice, + ExcessConversionConstant: excessConversionConstant, + } + + tests := []struct { + name string + state State + seconds uint64 + }{ + { + name: "current > target", + state: State{ + Current: math.MaxUint32, + Excess: 0, + }, + seconds: math.MaxUint64, + }, + { + name: "current == target", + state: State{ + Current: target, + Excess: 0, + }, + seconds: math.MaxUint64, + }, + { + name: "current < target", + state: State{ + Current: 0, + Excess: 0, + }, + seconds: math.MaxUint64, + }, + { + name: "current < target and reasonable excess", + state: State{ + Current: 0, + Excess: target + 1, + }, + seconds: math.MaxUint64/minPrice + 1, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal( + t, + uint64(math.MaxUint64), // Should not overflow + test.state.CostOf(config, test.seconds), + ) + }) + } +} + +func TestStateSecondsRemaining(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { require.Equal( t, test.expectedSeconds, - test.state.SecondsUntil(test.config, year, test.expectedCost), + test.state.SecondsRemaining(test.config, week, test.expectedCost), + ) + }) + } +} + +func TestStateSecondsRemainingLimit(t *testing.T) { + const target = 10_000 + tests := []struct { + name string + state State + minPrice gas.Price + costLimit uint64 + }{ + { + name: "zero price", + state: State{ + Current: math.MaxUint32, + Excess: 0, + }, + minPrice: 0, + costLimit: 0, + }, + { + name: "current > target", + state: State{ + Current: target + 1, + Excess: 0, + }, + minPrice: minPrice, + costLimit: math.MaxUint64, + }, + { + name: "current == target", + state: State{ + Current: target, + Excess: 0, + }, + minPrice: minPrice, + costLimit: minPrice * (week + 1), + }, + { + name: "current < target", + state: State{ + Current: 0, + Excess: 0, + }, + minPrice: minPrice, + costLimit: minPrice * (week + 1), + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := Config{ + Target: target, + MinPrice: test.minPrice, + ExcessConversionConstant: excessConversionConstant, + } + require.Equal( + t, + uint64(week), + test.state.SecondsRemaining(config, week, test.costLimit), ) }) } @@ -299,10 +416,10 @@ func BenchmarkStateCostOf(b *testing.B) { } } -func BenchmarkStateSecondsUntil(b *testing.B) { +func BenchmarkStateSecondsRemaining(b *testing.B) { benchmarks := []struct { - name string - secondsUntil func( + name string + secondsRemaining func( s State, c Config, maxSeconds uint64, @@ -310,12 +427,12 @@ func BenchmarkStateSecondsUntil(b *testing.B) { ) uint64 }{ { - name: "unoptimized", - secondsUntil: State.unoptimizedSecondsUntil, + name: "unoptimized", + secondsRemaining: State.unoptimizedSecondsRemaining, }, { - name: "optimized", - secondsUntil: State.SecondsUntil, + name: "optimized", + secondsRemaining: State.SecondsRemaining, }, } for _, test := range tests { @@ -323,7 +440,7 @@ func BenchmarkStateSecondsUntil(b *testing.B) { for _, benchmark := range benchmarks { b.Run(benchmark.name, func(b *testing.B) { for i := 0; i < b.N; i++ { - benchmark.secondsUntil(test.state, test.config, year, test.expectedCost) + benchmark.secondsRemaining(test.state, test.config, week, test.expectedCost) } }) } @@ -361,7 +478,7 @@ func FuzzStateCostOf(f *testing.F) { MinPrice: gas.Price(minPrice), ExcessConversionConstant: gas.Gas(max(excessConversionConstant, 1)), } - seconds = min(seconds, year) + seconds = min(seconds, hour) require.Equal( t, s.unoptimizedCostOf(c, seconds), @@ -371,7 +488,7 @@ func FuzzStateCostOf(f *testing.F) { ) } -func FuzzStateSecondsUntil(f *testing.F) { +func FuzzStateSecondsRemaining(f *testing.F) { for _, test := range tests { f.Add( uint64(test.state.Current), @@ -379,7 +496,7 @@ func FuzzStateSecondsUntil(f *testing.F) { uint64(test.config.Target), uint64(test.config.MinPrice), uint64(test.config.ExcessConversionConstant), - uint64(year), + uint64(hour), test.expectedCost, ) } @@ -403,11 +520,11 @@ func FuzzStateSecondsUntil(f *testing.F) { MinPrice: gas.Price(minPrice), ExcessConversionConstant: gas.Gas(max(excessConversionConstant, 1)), } - maxSeconds = min(maxSeconds, year) + maxSeconds = min(maxSeconds, hour) require.Equal( t, - s.unoptimizedSecondsUntil(c, maxSeconds, targetCost), - s.SecondsUntil(c, maxSeconds, targetCost), + s.unoptimizedSecondsRemaining(c, maxSeconds, targetCost), + s.SecondsRemaining(c, maxSeconds, targetCost), ) }, ) @@ -432,25 +549,19 @@ func (s State) unoptimizedCostOf(c Config, seconds uint64) uint64 { return cost } -// unoptimizedSecondsUntil is a naive implementation of SecondsUntil that is -// used for differential fuzzing. -func (s State) unoptimizedSecondsUntil(c Config, maxSeconds uint64, targetCost uint64) uint64 { - var ( - cost uint64 - seconds uint64 - err error - ) - for cost < targetCost && seconds < maxSeconds { +// unoptimizedSecondsRemaining is a naive implementation of SecondsRemaining +// that is used for differential fuzzing. +func (s State) unoptimizedSecondsRemaining(c Config, maxSeconds uint64, fundsRemaining uint64) uint64 { + for seconds := uint64(0); seconds < maxSeconds; seconds++ { s = s.AdvanceTime(c.Target, 1) - seconds++ - price := gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant) - cost, err = safemath.Add(cost, uint64(price)) - if err != nil { + price := uint64(gas.CalculatePrice(c.MinPrice, s.Excess, c.ExcessConversionConstant)) + if price > fundsRemaining { return seconds } + fundsRemaining -= price } - return seconds + return maxSeconds } // floatToGas converts f to gas.Gas by truncation. `gas.Gas(f)` is preferred and