closes #1788

similar closed pr #1901

This pr bumps 2 maximum limits

max block gas
max vm cycle

AFAIK, to use amount of increased gas, vm cycle also need to be bump too.

This is temporarily fix pr until we have clear ways to measure gas usage and vm cycle.

Contributors' checklist...

[x] Added new tests, or not needed, or not feasible
[x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
[x] Updated the official documentation or not needed
[x] No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
[x] Added references to related issues and PRs
[ ] Provided any useful hints for running manual tests
[] Added new benchmarks to generated graphs, if any. More info here.

May 09 '24 11:05 r3v4s

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 54.19%. Comparing base (8057505) to head (ea548e2). Report is 3 commits behind head on master.

Files	Patch %	Lines
gno.land/pkg/gnoland/node_inmemory.go	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2065      +/-   ##
==========================================
- Coverage   54.96%   54.19%   -0.77%     
==========================================
  Files         481      520      +39     
  Lines       67391    73194    +5803     
==========================================
+ Hits        37040    39670    +2630     
- Misses      27330    30297    +2967     
- Partials     3021     3227     +206

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

May 09 '24 12:05 codecov[bot]

From the review meeting, we're considering whether it makes sense to bump the limit 10x (not 1000x, as it most definitely is a Bad Idea)

We'll still take a second to consider this, but please in the meantime @r3v4s update the PR to be a 10x increase rather than 1000x; and provide examples of the contracts from GnoSwap which take more than 10M gas.

Thanks!

cc/ @jaekwon

May 09 '24 17:05 thehowl

From the review meeting, we're considering whether it makes sense to bump the limit 10x (not 1000x, as it most definitely is a Bad Idea)

We'll still take a second to consider this, but please in the meantime @r3v4s update the PR to be a 10x increase rather than 1000x; and provide examples of the contracts from GnoSwap which take more than 10M gas.

Thanks!

cc/ @jaekwon

cc/ @thehowl @zivkovicmilos @jaekwon

Decreased bump size from x1000 to x10.

And about examples for max gas and max cycle thing... here are some information.

Tested Env

latest gno commit: 80575054429e07c221f1453104dd0ad29e33291c

MAX GAS ISSUES

Deploying uint256 in Gno takes 8.9M gas
Deploying a modified version of uint256 in GnoSwap takes 9.2M gas

FYI, this modified version has more code to handle the uint512 calculation in GnoSwap.

$ gnokey maketx addpkg -pkgdir ./gnoswap/uint256 -pkgpath gno.land/p/demo/u256_with_512 -broadcast=true -gas-fee 1ugnot -gas-wanted 10000000 -insecure-password-stdin=true test1 -remote localhost:36657
Enter password.

OK!
GAS WANTED: 10000000
GAS USED:   9266840
HEIGHT:     5294
EVENTS:     []

Deploying the same version uint256 from the above 2. with a longer pkg_path takes more gas (MUST BE A VM BUG)

deploy to gno.land/p/demo/gnoswap/u1 => 9.5M
deploy to gno.land/p/demo/gnoswap/u2 => 9.5M
deploy to gno.land/p/demo/gnoswap/what_an_long_pkg_path => out of gas
deploy to gno.land/p/demo/gnoswap/longer_path => 9.9M

Deploying the pool contract takes 9.9M

$ echo "" | gnokey maketx addpkg -pkgdir ./pool -pkgpath gno.land/r/demo/pool -broadcast=true -gas-fee 1ugnot -gas-wanted 10000000 -insecure-password-stdin=true test1 -remote localhost:36657 -insecure-password-stdin=true
Enter password.

OK!
GAS WANTED: 10000000
GAS USED:   9908296
HEIGHT:     89
EVENTS:     []

MAX VM_CYCLE ISSUE

Calling SwapRoute in the router contract can cost more than 10M

The Swap function in the pool contract swaps tokens in a specified pool.

// POOL
// Swap swaps token0 for token1, or token1 for token0
//
// Panics if any of the following conditions are met:
// - The caller is not the router contract
// - Target pool is being used by another transaction
// - The amountSpecified is 0
// - The SqrtPriceLimit is not within the range
func Swap(
	token0Path string,
	token1Path string,
	fee uint32,
	_recipient string,
	zeroForOne bool,
	_amountSpecified string, // int256
	_sqrtPriceLimitX96 string, // uint160
	_payer string, // router
) (string, string) { // int256 x2
	common.DisallowCallFromUser()
	common.AllowCallFromOnly(consts.ROUTER_PATH)

	if _amountSpecified == "0" {
		panic("[POOL] pool.gno__Swap() || _amountSpecified == 0")
	}

	amountSpecified := i256.MustFromDecimal(_amountSpecified)
	sqrtPriceLimitX96 := u256.MustFromDecimal(_sqrtPriceLimitX96)

	recipient := std.Address(_recipient)
	payer := std.Address(_payer)

	pool := GetPool(token0Path, token1Path, fee)
	slot0Start := pool.slot0

	if !(slot0Start.unlocked) {
		panic("[POOL] pool.gno__Swap() || slot0Start.unlocked must be unlocked(true)")
	}

	var feeProtocol uint8
	var feeGrowthGlobalX128 *u256.Uint

	if zeroForOne {
		minSqrtRatio := u256.MustFromDecimal(consts.MIN_SQRT_RATIO)

		cond1 := sqrtPriceLimitX96.Lt(slot0Start.sqrtPriceX96)
		cond2 := sqrtPriceLimitX96.Gt(minSqrtRatio)
		if !(cond1 && cond2) {
			panic(ufmt.Sprintf("[POOL] pool.gno__Swap() || sqrtPriceLimitX96(%s) < slot0Start.sqrtPriceX96(%s) && sqrtPriceLimitX96(%s) > consts.MIN_SQRT_RATIO(%s)", sqrtPriceLimitX96.ToString(), slot0Start.sqrtPriceX96.ToString(), sqrtPriceLimitX96.ToString(), consts.MIN_SQRT_RATIO))
		}

		feeProtocol = slot0Start.feeProtocol % 16
		feeGrowthGlobalX128 = pool.feeGrowthGlobal0X128
	} else {
		maxSqrtRatio := u256.MustFromDecimal(consts.MAX_SQRT_RATIO)

		cond1 := sqrtPriceLimitX96.Gt(slot0Start.sqrtPriceX96)
		cond2 := sqrtPriceLimitX96.Lt(maxSqrtRatio)
		if !(cond1 && cond2) {
			panic(ufmt.Sprintf("[POOL] pool.gno__Swap() || sqrtPriceLimitX96(%s) > slot0Start.sqrtPriceX96(%s) && sqrtPriceLimitX96(%s) < consts.MAX_SQRT_RATIO(%s)", sqrtPriceLimitX96.ToString(), slot0Start.sqrtPriceX96.ToString(), sqrtPriceLimitX96.ToString(), consts.MAX_SQRT_RATIO))
		}

		feeProtocol = slot0Start.feeProtocol / 16
		feeGrowthGlobalX128 = pool.feeGrowthGlobal1X128
	}

	pool.slot0.unlocked = false
	cache := newSwapCache(feeProtocol, pool.liquidity)
	state := pool.newSwapState(amountSpecified, feeGrowthGlobalX128, cache.liquidityStart)

	exactInput := amountSpecified.Gt(i256.Zero())

	// continue swapping as long as we haven't used the entire input/output and haven't reached the price limit
	for !(state.amountSpecifiedRemaining.IsZero()) && !(state.sqrtPriceX96.Eq(sqrtPriceLimitX96)) {
		var step StepComputations
		step.sqrtPriceStartX96 = state.sqrtPriceX96

		step.tickNext, step.initialized = pool.tickBitmapNextInitializedTickWithInOneWord(
			state.tick,
			pool.tickSpacing,
			zeroForOne,
		)

		// ensure that we do not overshoot the min/max tick, as the tick bitmap is not aware of these bounds
		if step.tickNext < consts.MIN_TICK {
			step.tickNext = consts.MIN_TICK
		} else if step.tickNext > consts.MAX_TICK {
			step.tickNext = consts.MAX_TICK
		}

		// get the price for the next tick
		step.sqrtPriceNextX96 = common.TickMathGetSqrtRatioAtTick(step.tickNext)

		isLower := step.sqrtPriceNextX96.Lt(sqrtPriceLimitX96)
		isHigher := step.sqrtPriceNextX96.Gt(sqrtPriceLimitX96)

		var sqrtRatioTargetX96 *u256.Uint
		if (zeroForOne && isLower) || (!zeroForOne && isHigher) {
			sqrtRatioTargetX96 = sqrtPriceLimitX96
		} else {
			sqrtRatioTargetX96 = step.sqrtPriceNextX96
		}

		_sqrtPriceX96Str, _amountInStr, _amountOutStr, _feeAmountStr := plp.SwapMathComputeSwapStepStr(
			state.sqrtPriceX96,
			sqrtRatioTargetX96,
			state.liquidity,
			state.amountSpecifiedRemaining,
			uint64(pool.fee),
		)
		state.sqrtPriceX96 = u256.MustFromDecimal(_sqrtPriceX96Str)
		step.amountIn = u256.MustFromDecimal(_amountInStr)
		step.amountOut = u256.MustFromDecimal(_amountOutStr)
		step.feeAmount = u256.MustFromDecimal(_feeAmountStr)

		amountInWithFee := i256.FromUint256(new(u256.Uint).Add(step.amountIn, step.feeAmount))
		if exactInput {
			state.amountSpecifiedRemaining = i256.Zero().Sub(state.amountSpecifiedRemaining, amountInWithFee)
			state.amountCalculated = i256.Zero().Sub(state.amountCalculated, i256.FromUint256(step.amountOut))
		} else {
			state.amountSpecifiedRemaining = i256.Zero().Add(state.amountSpecifiedRemaining, i256.FromUint256(step.amountOut))
			state.amountCalculated = i256.Zero().Add(state.amountCalculated, amountInWithFee)
		}

		// if the protocol fee is on, calculate how much is owed, decrement feeAmount, and increment protocolFee
		if cache.feeProtocol > 0 {
			delta := new(u256.Uint).Div(step.feeAmount, u256.NewUint(uint64(cache.feeProtocol)))
			step.feeAmount = new(u256.Uint).Sub(step.feeAmount, delta)
			state.protocolFee = new(u256.Uint).Add(state.protocolFee, delta)
		}

		// update global fee tracker
		if state.liquidity.Gt(u256.Zero()) {
			update := u256.MulDiv(step.feeAmount, u256.MustFromDecimal(consts.Q128), state.liquidity)
			state.feeGrowthGlobalX128 = new(u256.Uint).Add(state.feeGrowthGlobalX128, update)
		}

		// shift tick if we reached the next price
		if state.sqrtPriceX96.Eq(step.sqrtPriceNextX96) {
			// if the tick is initialized, run the tick transition
			if step.initialized {
				var fee0, fee1 *u256.Uint

				// check for the placeholder value, which we replace with the actual value the first time the swap crosses an initialized tick
				if zeroForOne {
					fee0 = state.feeGrowthGlobalX128
					fee1 = pool.feeGrowthGlobal1X128
				} else {
					fee0 = pool.feeGrowthGlobal0X128
					fee1 = state.feeGrowthGlobalX128
				}

				liquidityNet := pool.tickCross(
					step.tickNext,
					fee0,
					fee1,
				)

				// if we're moving leftward, we interpret liquidityNet as the opposite sign
				if zeroForOne {
					liquidityNet = i256.Zero().Neg(liquidityNet)
				}

				state.liquidity = liquidityMathAddDelta(state.liquidity, liquidityNet)
			}

			if zeroForOne {
				state.tick = step.tickNext - 1
			} else {
				state.tick = step.tickNext
			}
		} else if !(state.sqrtPriceX96.Eq(step.sqrtPriceStartX96)) {
			// recompute unless we're on a lower tick boundary (i.e. already transitioned ticks), and haven't moved
			state.tick = common.TickMathGetTickAtSqrtRatio(state.sqrtPriceX96)
		}
	}
	// END LOOP

	// update pool sqrtPrice
	pool.slot0.sqrtPriceX96 = state.sqrtPriceX96

	// update tick if it changed
	if state.tick != slot0Start.tick {
		pool.slot0.tick = state.tick
	}

	// update liquidity if it changed
	if !(cache.liquidityStart.Eq(state.liquidity)) {
		pool.liquidity = state.liquidity
	}

	// update fee growth global and, if necessary, protocol fees
	// overflow is acceptable, protocol has to withdraw before it hits MAX_UINT256 fees
	if zeroForOne {
		pool.feeGrowthGlobal0X128 = state.feeGrowthGlobalX128
		if state.protocolFee.Gt(u256.Zero()) {
			pool.protocolFees.token0 = new(u256.Uint).Add(pool.protocolFees.token0, state.protocolFee)
		}
	} else {
		pool.feeGrowthGlobal1X128 = state.feeGrowthGlobalX128
		if state.protocolFee.Gt(u256.Zero()) {
			pool.protocolFees.token1 = new(u256.Uint).Add(pool.protocolFees.token1, state.protocolFee)
		}
	}

	var amount0, amount1 *i256.Int
	if zeroForOne == exactInput {
		amount0 = i256.Zero().Sub(amountSpecified, state.amountSpecifiedRemaining)
		amount1 = state.amountCalculated
	} else {
		amount0 = state.amountCalculated
		amount1 = i256.Zero().Sub(amountSpecified, state.amountSpecifiedRemaining)
	}

	// actual swap
	if zeroForOne {
		// payer > POOL
		pool.transferFromAndVerify(payer, consts.POOL_ADDR, pool.token0Path, amount0, true)

		// POOL > recipient
		pool.transferAndVerify(recipient, pool.token1Path, amount1, false)

	} else {
		// payer > POOL
		pool.transferFromAndVerify(payer, consts.POOL_ADDR, pool.token1Path, amount1, false)

		// POOL > recipient
		pool.transferAndVerify(recipient, pool.token0Path, amount0, true)

	}

	pool.slot0.unlocked = true
	return amount0.ToString(), amount1.ToString()
}

The SwapRoute function in the router contract swap input <-> output tokens by calling the pool contract’s Swap() -- Currently, SwapRoute can route up to 21 different pools to find and provide the best ratio for the input <-> output tokens. -- So once it reaches the above point, the current VM cycle limit can be exceeded.

// SwapRoute swaps the input token to the output token and returns the result amount
// If swapType is EXACT_IN, it returns the amount of output token ≈ amount of user to receive
// If swapType is EXACT_OUT, it returns the amount of input token ≈ amount of user to pay
//
// Panics if any of the following conditions are met:
// - amountSpecified is zero or is not numeric
// - swapType is not EXACT_IN or EXACT_OUT
// - length of route and quotes are not the same
// - length of routes is not 1 ~ 7
// - sum of quotes is not 100
// - number of hops is not 1 ~ 3
// - too many token spend or too few token received
func SwapRoute(
	inputToken string,
	outputToken string,
	_amountSpecified string, // int256
	swapType string,
	strRouteArr string, // []string
	quoteArr string, // []int
	_tokenAmountLimit string, // uint256
) (string, string) { // tokneIn, tokenOut
	if common.GetLimitCaller() {
		isUserCalled := std.PrevRealm().PkgPath() == ""
		if !isUserCalled {
			panic("[ROUTER] router.gno__SwapRoute() || only user can call this function")
		}
	}

	amountSpecified := i256.MustFromDecimal(_amountSpecified)
	if amountSpecified.IsZero() {
		panic("[ROUTER] router.gno__SwapRoute() || amountSpecified == 0")
	}
	if amountSpecified.IsNeg() {
		panic("[ROUTER] router.gno__SwapRoute() || amountSpecified < 0")
	}

	tokenAmountLimit := u256.MustFromDecimal(_tokenAmountLimit)

	switch swapType {
	case "EXACT_IN":
		// do nothing
	case "EXACT_OUT":
		amountSpecified = i256.Zero().Neg(amountSpecified)
	default:
		panic("[ROUTER] router.gno__SwapRoute() || unknown swapType")
	}

	// check route length ( should be 1 ~ 7 )
	routes := strings.Split(strRouteArr, ",")
	isValidRouteLength := (1 <= len(routes)) && (len(routes) <= 7)
	if !isValidRouteLength {
		panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || len(routes) should be 1 ~ 7 (len(routes)[%d])", len(routes)))
	}

	// check if routes length and quotes length are same
	quotes := strings.Split(quoteArr, ",")
	if len(routes) != len(quotes) {
		panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || len(routes[%d]) != len(quotes[%d])", len(routes), len(quotes)))
	}

	// check if quotes are up to 100%
	quotesSum := int64(0)
	for _, quote := range quotes {
		intQuote, _ := strconv.Atoi(quote)
		quotesSum += int64(intQuote)
	}
	if quotesSum != 100 {
		panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || quotesSum != 100 (quotesSum)[%d]", quotesSum))
	}

	// if input is gnot, wrap it
	userOldWugnotBalance := uint64(0)
	if inputToken == consts.GNOT {
		sent := std.GetOrigSend()
		ugnotSentByUser := uint64(sent.AmountOf("ugnot"))

		wrap(ugnotSentByUser)
		userOldWugnotBalance = wugnot.BalanceOf(a2u(std.GetOrigCaller()))
	} else if outputToken == consts.GNOT { // if output is gnot unwrap later (save user's current wugnot balance)
		userOldWugnotBalance = wugnot.BalanceOf(a2u(std.GetOrigCaller()))
	}

	resultAmountIn := u256.Zero()
	resultAmountOut := u256.Zero()

	for i, route := range routes {
		numHops := strings.Count(route, "*POOL*") + 1
		quote, _ := strconv.Atoi(quotes[i])

		if numHops < 1 || numHops > 3 {
			panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || numHops should be 1 ~ 3 (numHops)[%d]", numHops))
		}

		toSwap := i256.Zero().Mul(amountSpecified, i256.NewInt(int64(quote)))
		toSwap = toSwap.Div(toSwap, i256.NewInt(100))

		if numHops == 1 { // SINGLE
			amountIn, amountOut := handleSingleSwap(route, toSwap, false)
			resultAmountIn = new(u256.Uint).Add(resultAmountIn, amountIn)
			resultAmountOut = new(u256.Uint).Add(resultAmountOut, amountOut)
		} else if numHops == 2 || numHops == 3 { // MULTI
			amountIn, amountOut := handleMultiSwap(swapType, route, numHops, toSwap, false)
			resultAmountIn = new(u256.Uint).Add(resultAmountIn, amountIn)
			resultAmountOut = new(u256.Uint).Add(resultAmountOut, amountOut)
		} else {
			panic("[ROUTER] router.gno__SwapRoute() || numHops should be 1 ~ 3")
		}
	}

	// PROTOCOL FEE
	resultAmountOut = handleProtocolFee(outputToken, resultAmountOut, false)

	// UNWRAP IF NECESSARY
	// if input was gnot, refund left over wugnot
	if inputToken == consts.GNOT {
		userNewWugnotBalance := wugnot.BalanceOf(a2u(std.GetOrigCaller()))
		unwrap(userNewWugnotBalance)
	} else if outputToken == consts.GNOT { // if output was gnot, unwrap result
		userNewWugnotBalance := wugnot.BalanceOf(a2u(std.GetOrigCaller()))
		userRecvWugnot := uint64(userNewWugnotBalance - userOldWugnotBalance) // received wugnot
		unwrap(userRecvWugnot)
	}

	if swapType == "EXACT_IN" {
		if !(tokenAmountLimit.Lte(resultAmountOut)) {
			panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || too few received for user (expected minimum received:%s, actual received:%s)", _tokenAmountLimit, resultAmountOut.ToString()))
		}
	} else { // EXACT_OUT
		if !(resultAmountIn.Lte(tokenAmountLimit)) {
			panic(ufmt.Sprintf("[ROUTER] router.gno__SwapRoute() || too much spend for user (expected maximum spend:%s, actual spend:%s)", _tokenAmountLimit, resultAmountIn.ToString()))
		}
	}
	//return resultAmountIn.ToString(), resultAmountOut.ToString()

	intAmountOut := i256.FromUint256(resultAmountOut)
	return resultAmountIn.ToString(), i256.Zero().Neg(intAmountOut).ToString()
}

May 10 '24 03:05 r3v4s

@r3v4s can you open an issue for number 3? It's very disturbing, I'm not sure what package paths have to do with gas usage

May 13 '24 08:05 zivkovicmilos

@r3v4s can you open an issue for number 3? It's very disturbing, I'm not sure what package paths have to do with gas usage

Got it. (UPDATED // Number 3 Issue: https://github.com/gnolang/gno/issues/2083)

How about deploying uint256 spends too much gas? Do you want me to open separate issue for this or just leave it here.

May 13 '24 09:05 r3v4s

fix: Increase 'max gas' and 'vm cycle limit' from 10M to 100M

Codecov Report

Tested Env

MAX GAS ISSUES

MAX VM_CYCLE ISSUE