Skip to content

Commit

Permalink
ACP-77: Update SecondsUntil to prevent under-charging (#3395)
Browse files Browse the repository at this point in the history
  • Loading branch information
@StephenButtolph
StephenButtolph authored Sep 18, 2024
1 parent 69efca3 commit d660979
Show file tree
Hide file tree
Showing 2 changed files with 164 additions and 84 deletions.
67 changes: 18 additions & 49 deletions vms/platformvm/validators/fee/fee.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -91,77 +91,46 @@ 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,
// monotonically increase it, or monotonically decrease it. If it is
// 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
}
181 changes: 146 additions & 35 deletions vms/platformvm/validators/fee/fee_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const (
hour = 60 * minute
day = 24 * hour
week = 7 * day
year = 365 * day

minPrice = 2_048

Expand Down Expand Up @@ -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,
Expand All @@ -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,
},
}
)
Expand Down Expand Up @@ -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),
)
})
}
Expand Down Expand Up @@ -299,31 +416,31 @@ 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,
targetCost uint64,
) 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 {
b.Run(test.name, func(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)
}
})
}
Expand Down Expand Up @@ -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),
Expand All @@ -371,15 +488,15 @@ 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),
uint64(test.state.Excess),
uint64(test.config.Target),
uint64(test.config.MinPrice),
uint64(test.config.ExcessConversionConstant),
uint64(year),
uint64(hour),
test.expectedCost,
)
}
Expand All @@ -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),
)
},
)
Expand All @@ -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
Expand Down

0 comments on commit d660979

Please sign in to comment.