sae: Support oscillating MinPrice configs by StephenButtolph · Pull Request #5279 · ava-labs/avalanchego

StephenButtolph · 2026-04-14T15:41:16Z

Why this should be merged

Resolves #5242

There has been increased desired to make the minimum price configurable on the C-chain: avalanche-foundation/ACPs#283

However, this requires the GasPriceConfig to be semi-untrusted. Currently, the implementation of AfterBlock assumes that the GasPriceConfig is set by some "trusted" entity.

While this makes sense for a subnet-evm instance, it doesn't work for the C-chain, where a malicious block producer may try to keep the gas price artificially low.

This PR fortifies the gastime.Time.AfterBlock against semi-malicious GasPriceConfig changes.

How this works

The ACP-176 mechanism calculates price as:

$$ P := M \cdot e^{\left(\frac{x}{K}\right)} $$

Currently, we support changing $M$ by tracking the previous price, and then binary searching for the closest excess such that the new price will be as close to the old price as possible.

This works, but only if $M$ is only rarely changed.

We can instead modify the price calculation to be:

$$ P := e^{\left(\frac{x}{K}\right)} $$

where

$$ x \geq \ln(M) \cdot K $$

This is equivalent, other than integer approximation differences, but allows us to never reduce $x$ during a min price change, and only increase $x$ when the min price is increased past the prior price.

Specifically, this change:

Removes MinPrice from gas.CalculatePrice and handles everything in the exponent.
Changes StaticPricing so that excess is always pinned to the minimum (rather than ignoring excess during Price.) TBH I feel like this could be considered a bug-fix in its own right, since excess is exposed publicly and not just an internal gasprice implementation detail.
Changes excess scaling to linearly scale based on the product of T and the Scaling constant, rather than binary searching when the scaling constant is changed.

How this was tested

Added a regression test for min price oscillations.
Replaced FuzzPriceInvarianceAfterBlock with a large test table asserting excess and price in TestAfterBlock
Added a new fuzz test for priceExcess (which calculates ln(p) * k).

Need to be documented in RELEASES.md?

No.

…ego into sae-price-manipulation

ARR4N · 2026-04-27T09:56:51Z

Aside: setting the lower bound on $x$ while removing $M$ entirely has the added benefit of a change in the minimum price not affecting the rate of change of price by scaling the entire $e^{\frac{x}{K}}$ component.

ARR4N

Still need to review acp176_test.go but sharing everything else early.

ARR4N · 2026-04-27T10:46:32Z

+// scaleExcess returns x * newT * newScale / (oldT * oldScale) rounded up and
+// capped to [math.MaxUint64].
+func scaleExcess(x, newT, newScale, oldT, oldScale gas.Gas) gas.Gas {
+	var (
+		newK uint256.Int
+		v    uint256.Int
+	)
+	newK.SetUint64(uint64(newT))
+	v.SetUint64(uint64(newScale))
+	newK.Mul(&newK, &v)
+
+	var oldK uint256.Int
+	oldK.SetUint64(uint64(oldT))
+	v.SetUint64(uint64(oldScale))
+	oldK.Mul(&oldK, &v)
+
+	v.SetUint64(uint64(x))
+	v.Mul(&v, &newK)
+	v.Add(&v, &oldK) // round up by adding oldK - 1
+	v.SubUint64(&v, 1)
+	v.Div(&v, &oldK)
+
+	if !v.IsUint64() {
+		return math.MaxUint64
+	}
+	return gas.Gas(v.Uint64())
+}


Reusing v everywhere is unclear. I think the following is easier to reason about:

Suggested change

// scaleExcess returns x * newT * newScale / (oldT * oldScale) rounded up and

// capped to [math.MaxUint64].

func scaleExcess(x, newT, newScale, oldT, oldScale gas.Gas) gas.Gas {

var (

newK uint256.Int

v uint256.Int

)

newK.SetUint64(uint64(newT))

v.SetUint64(uint64(newScale))

newK.Mul(&newK, &v)

var oldK uint256.Int

oldK.SetUint64(uint64(oldT))

v.SetUint64(uint64(oldScale))

oldK.Mul(&oldK, &v)

v.SetUint64(uint64(x))

v.Mul(&v, &newK)

v.Add(&v, &oldK) // round up by adding oldK - 1

v.SubUint64(&v, 1)

v.Div(&v, &oldK)

if !v.IsUint64() {

return math.MaxUint64

}

return gas.Gas(v.Uint64())

}

// scaleExcess returns oldX * newT * newScale / (oldT * oldScale) rounded up and

// capped to [math.MaxUint64].

func scaleExcess(oldX, newT, newScale, oldT, oldScale gas.Gas) gas.Gas {

newK := mulAsUint256(newT, newScale)

oldK := mulAsUint256(oldT, oldScale)

if newK.Eq(&oldK) {

return oldX

}

var x uint256.Int

x.SetUint64(uint64(oldX))

x.Mul(&x, &newK)

x.Add(&x, &oldK) // round up by adding oldK - 1

x.SubUint64(&x, 1)

x.Div(&x, &oldK)

if !x.IsUint64() {

return math.MaxUint64

}

return gas.Gas(x.Uint64())

}

func mulAsUint256[T ~uint64](a, b T) uint256.Int {

var x, y uint256.Int

x.SetUint64(uint64(a))

y.SetUint64(uint64(b))

x.Mul(&x, &y)

return x

}

(no action required) Out of interest, why don't you like the short-circuit equality check?

Definitely opinionated haha, but I always want to feel like code as a purpose. For me, this if statement would convey either:

This is an edge case we are guarding against (which it isn't)

This is a performance improvement (which should come with a benchmark)

This is trying to convey the behavior of the function (which would better live as a comment than changing the code imo).

If the intent of the suggestion was (2), which I think is probably valid (albeit likely a micro-optimization), then I think we'd want to add a benchmark so that we could understand how much it actually matters.

ARR4N

Review of TestAfterBlock().

ARR4N · 2026-04-27T12:21:41Z

-			newKonT: 0, // invalid
-			T:       1, M: 1, KonT: 1,
-			newM: 1,
+			name: "high_price_scaling_causes_price_increase",


Is the fact that it's an increase a property that we want/expect? If so, why? I was a little confused about the semantics of priceExcess() and I suspect this is related.

Is the fact that it's an increase a property that we want?

No.

Is the fact that it's an increase a property that we expect?

Yes.

This actually isn't relevant to priceExcess. The calculations for this test are as follows:

oldX := 1_802_924_127 newK := 50 * 1_000_000 oldK := 87 * 1_000_000 newX := oldX * newK / oldK = 1_802_924_127 * 50 / 87 ~= 1_036_163_291.379...

If we round the excess down to 1_036_163_291, the price would decrease to 999_999_983.
We instead, round up to 1_036_163_292, where the price is 1_000_000_002.

It isn't possible for us to maintain price 999_999_990, so we need to decide what to do. We've always maintained rounding up to avoid any weird edge cases w.r.t. trying to game the price lower by block proposers... So this test is really just verifying that we are rounding up.

I changed the name of the test from high_price_scaling_causes_price_increase to high_price_scaling_rounding_causes_price_increase.

ARR4N · 2026-04-27T12:23:18Z

+			name: "intermediate_scaling_overflow",
+			init: state{
+				target: 1_000_000,
+				excess: math.MaxUint64,
+				config: GasPriceConfig{
+					TargetToExcessScaling: math.MaxUint64,
+					MinPrice:              1,
+				},
+				price: 2,
+			},
+			new: state{
+				target: 1_000_000,
+				excess: 1,
+				config: GasPriceConfig{
+					TargetToExcessScaling: 1,
+					MinPrice:              1,
+				},
+				price: 1,
+			},
+		},
+		{
+			name: "intermediate_scaling_overflow_rounds_up",


Please can you explain these two intermediate scaling tests.

When scaling x, we calculate oldK and newK.

newX := oldX * newK / oldK newK := 1_000_000 oldK := 1_000_000 * MaxUint64

Previously, if oldK or newK exceeded MaxUint64, we capped them at MaxUint64.

So, previously, x would have been updated based on:

newX := oldX * newK / oldK newK := 1_000_000 oldK := min(1_000_000 * MaxUint64, MaxUint64) = MaxUint64

So previously newX would have been 1_000_000, rather than the (correct) 1 which it is now.

I renamed the tests to scaling_old_k_overflow and scaling_old_k_overflow_rounds_up. I also added scaling_new_k_overflow.

Gotcha; no loss of resolution due to clipping.

ARR4N · 2026-04-27T12:27:02Z

+			name: "invalid_target_override",
+			init: state{
+				target: 0,


This seems like a strange test. When would we ever be in such an invalid state and need to override it?

This is essentially testing the same case as tested in TestTargetClamping. We silently ignore a target=0, and clamp it to 0.

This shouldn't happen in practice, but we sanitize it here regardless

ARR4N

End of review. Sorry this took so long!

ARR4N · 2026-04-27T12:32:16Z

+		gotP := calculatePrice(x, k)
+		assert.LessOrEqual(t, gotP, p, "gotPrice <= wantPrice")
+
+		if gotP < p && x != math.MaxUint64 {


Confirming my understanding. gotP < p i.f.f. p is unrepresentable, so we assert that the smallest possible increase in excess skips p?

Correct.

The x != math.MaxUint64 is also important, as it's possible that gotP << p, which in that case x == math.MaxUint64.

ARR4N · 2026-04-27T12:37:15Z

+func TestOscillatingMinPrice(t *testing.T) {
+	const (
+		target       gas.Gas   = 1_000_000
+		gasPerBlock  gas.Gas   = target // must be sufficiently large


(nit) The reuse of target suggests that it's a deliberate choice rather than something arbitrary but large.

Suggested change

gasPerBlock gas.Gas = target // must be sufficiently large

gasPerBlock gas.Gas = target * TargetToRate // all gas consumed

I think the suggestion is a bit confusing (since we don't actually have a block GasLimit here... I removed the usage of target and just replaced it with a random number (10M) and left the prior comment.

alarso16 · 2026-04-27T17:58:33Z

+	newK := mulAsUint256(newT, newScale)
+	oldK := mulAsUint256(oldT, oldScale)
+
+	var x uint256.Int


I think a brief justification that x won't overflow is necessary. It would be pretty easy to add another multiplication here in a future refactor, and then the addition below could overflow.

As an alternative, is there any reason not to use big.Int?

I added a comment... I kinda waffled on the usefulness... But there is enough math going on that I can see why a specific S/O for the overflow case is reasonable.

As an alternative, is there any reason not to use big.Int?

big.Int is significantly less efficient.

ARR4N · 2026-04-28T08:40:08Z

 		},
 		{
-			name: "intermediate_scaling_overflow_rounds_up",
+			name: "scaling_old_k_overflow_rounds_up",


What is rounding up? Isn't the previous case (with price of 2) rounding up?

The previous case does not round up (the excess calculation):

MaxUint64 * 1_000_000 * 1 / (1_000_000 * math.MaxUint64) (1_000_000 * MaxUint64) / (1_000_000 * math.MaxUint64) 1 (no rounding needed)

This case does round up:

1 * 1_000_000 * 1 / (1_000_000 * math.MaxUint64) (1_000_000 * 1) / (1_000_000 * math.MaxUint64) 1 / MaxUint64 1 (after rounding)

ARR4N · 2026-04-28T09:15:20Z

 		},
 	}
+	opts := cmp.Options{
+		cmpopts.IgnoreTypes(canotoData_GasPriceConfig{}),


This should probably be appended to the incoming options in requireState() as it will be common to all. I'm not sure why it's not complaining elsewhere though.

It isn't complaining elsewhere because Config is ignored entirely elsewhere. I'll add it to the require

ARR4N · 2026-04-28T09:20:30Z

+// scaleExcess returns x * newT * newScale / (oldT * oldScale) rounded up and
+// capped to [math.MaxUint64].
+func scaleExcess(x, newT, newScale, oldT, oldScale gas.Gas) gas.Gas {
+	var (
+		newK uint256.Int
+		v    uint256.Int
+	)
+	newK.SetUint64(uint64(newT))
+	v.SetUint64(uint64(newScale))
+	newK.Mul(&newK, &v)
+
+	var oldK uint256.Int
+	oldK.SetUint64(uint64(oldT))
+	v.SetUint64(uint64(oldScale))
+	oldK.Mul(&oldK, &v)
+
+	v.SetUint64(uint64(x))
+	v.Mul(&v, &newK)
+	v.Add(&v, &oldK) // round up by adding oldK - 1
+	v.SubUint64(&v, 1)
+	v.Div(&v, &oldK)
+
+	if !v.IsUint64() {
+		return math.MaxUint64
+	}
+	return gas.Gas(v.Uint64())
+}


(no action required) Out of interest, why don't you like the short-circuit equality check?

ARR4N · 2026-04-28T09:23:44Z

+			name: "intermediate_scaling_overflow",
+			init: state{
+				target: 1_000_000,
+				excess: math.MaxUint64,
+				config: GasPriceConfig{
+					TargetToExcessScaling: math.MaxUint64,
+					MinPrice:              1,
+				},
+				price: 2,
+			},
+			new: state{
+				target: 1_000_000,
+				excess: 1,
+				config: GasPriceConfig{
+					TargetToExcessScaling: 1,
+					MinPrice:              1,
+				},
+				price: 1,
+			},
+		},
+		{
+			name: "intermediate_scaling_overflow_rounds_up",


Gotcha; no loss of resolution due to clipping.

Add oscillating min price test

6c495ae

github-project-automation Bot added this to avalanchego Apr 14, 2026

alarso16 and others added 8 commits April 14, 2026 14:21

refactor: move gaspriceconfig to saetypes

0d1b7c0

refactor: remove hooks import from gastime

b25165b

chore: comments

b73259a

Update gastime handling to intrinsically support minPrice

a92de59

Merge branch 'master' into sae-price-manipulation

d173c0d

nit

dcb0690

Merge branch 'sae-price-manipulation' of github.com:ava-labs/avalanch…

078a84a

…ego into sae-price-manipulation

lint

6f5743f

StephenButtolph self-assigned this Apr 14, 2026

StephenButtolph commented Apr 15, 2026

View reviewed changes

StephenButtolph and others added 6 commits April 15, 2026 10:40

wip

77d9df8

refactor: delete one of the configs

b48c47b

update tests

5465b55

expand tests

0fc7871

nit

ac6ad4b

Update TODO

b08c178

StephenButtolph commented Apr 16, 2026

View reviewed changes

Comment thread vms/saevm/gastime/acp176_test.go

Comment thread vms/saevm/gastime/acp176_test.go

StephenButtolph added 3 commits April 16, 2026 10:44

Merge branch 'master' into sae-price-manipulation

3a172a8

go.mod

c8f92ba

Merge branch 'sae-price-manipulation' of github.com:ava-labs/avalanch…

13b7ea3

…ego into sae-price-manipulation

StephenButtolph moved this to In Progress 🏗️ in avalanchego Apr 16, 2026

StephenButtolph and others added 7 commits April 16, 2026 10:52

nit

c11e6b1

chore(gastime): fix error message

09eba0f

Add fuzz test

c08767b

nit

1aa2709

remove bad testdata

194bd97

improve fuzzing

b164f17

nit

fd719d8

StephenButtolph added 4 commits April 17, 2026 18:32

typo

414d588

typo

8ab5061

nit

afcad6f

optimize

2a54e32

StephenButtolph changed the title ~~sae: support oscillating MinPrice configs~~ sae: Support oscillating MinPrice configs Apr 25, 2026

ARR4N reviewed Apr 27, 2026

View reviewed changes

StephenButtolph added 4 commits April 27, 2026 09:50

First pass of comments

0920b20

Remove SetTarget and SetRate

bccfdeb

Reuse existing state struct

8953fc6

Merge branch 'master' into sae-price-manipulation

c114a02

alarso16 reviewed Apr 27, 2026

View reviewed changes

Comment thread go.mod Outdated

StephenButtolph added 3 commits April 27, 2026 15:44

nit

069cd5b

kinda weak comment, but ok

441dd4e

whats another test amongst friends

615d180

alarso16 approved these changes Apr 27, 2026

View reviewed changes

ARR4N approved these changes Apr 28, 2026

View reviewed changes

StephenButtolph and others added 4 commits April 28, 2026 08:48

nit

3fa86b7

fix bazel

0369966

ci: free runner disk before publishing Antithesis images (#5310)

85ae854

Merge branch 'master' into sae-price-manipulation

9d780e3

StephenButtolph force-pushed the master branch from 85ae854 to 659b7c3 Compare April 28, 2026 15:41

Merge branch 'master' into sae-price-manipulation

96f8744

StephenButtolph merged commit f5ff45c into master Apr 28, 2026
65 of 66 checks passed

StephenButtolph deleted the sae-price-manipulation branch April 28, 2026 16:19

github-project-automation Bot moved this from In Progress 🏗️ to Done 🎉 in avalanchego Apr 28, 2026

StephenButtolph added a commit that referenced this pull request Apr 28, 2026

sae: Support oscillating MinPrice configs (#5279)

b6ee3b6

	gasPerBlock gas.Gas = target // must be sufficiently large
	gasPerBlock gas.Gas = target * TargetToRate // all gas consumed

Conversation

StephenButtolph commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ARR4N commented Apr 27, 2026

Uh oh!

ARR4N left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ARR4N left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ARR4N left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StephenButtolph Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

StephenButtolph commented Apr 14, 2026 •

edited

Loading

StephenButtolph Apr 27, 2026 •

edited

Loading