feat(multi-atm): linear accrual interpolated

[adam-hotait]

No description provided.

[Copilot]

Pull request overview

This pull request introduces linear regression-based price extrapolation (accrual pricing) to the MultiATM contract. The feature allows trading pairs to use historical oracle price data to compute a trend line and extrapolate the current price, providing fairer pricing than the existing min/max strategy when price trends are consistent.

Changes:

• Added accrualRounds parameter to setPair() function, enabling configurable linear regression (0=disabled, 2-30=number of historical price points)
• Implemented _computeLinearRegression() to compute slope and intercept from oracle historical data
• Modified _getPrices() to use extrapolated prices when accrual is enabled, or min/max prices when disabled
• Fixed existing test bugs where oracle staleness tests didn't properly advance time
• Added comprehensive test coverage for linear regression pricing across multiple decimal configurations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.

File	Description
contracts/token/MultiATM.sol	Added accrual pricing infrastructure: new constants (_PRECISION, _MAX_REGRESSION_POINTS), accrualRounds field in Pair struct, linear regression computation functions, and updated _getPrices to support both pricing modes
test/main.test.js	Updated all existing setPair calls to include accrualRounds parameter (backwards compatibility break), fixed oracle staleness test bugs by adding +3601 to time advances, and added 766 lines of comprehensive tests validating accrual pricing with various slopes, decimal configurations, and edge cases

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The constant _PRECISION = 1e18 is used for fixed-point arithmetic in the slope calculation. While 1e18 is a common choice (matching Ethereum's wei precision), consider documenting why this precision level was chosen and what trade-offs it represents.

With 1e18 precision:

• Slope values can represent very fine gradations in price changes
• However, when computing slope * sumT in the intercept calculation (line 347), the intermediate value could be quite large

Consider adding a comment explaining that _PRECISION represents the fixed-point scaling factor for the slope coefficient, and that it provides sufficient precision for typical price movements while avoiding overflow in reasonable oracle data scenarios. This would help future maintainers understand the reasoning behind this choice.

Suggested change

	uint256 private constant _BASIS_POINT_SCALE = 1e4;
	uint256 private constant _BASIS_POINT_SCALE = 1e4;
	// Fixed-point scaling factor used for the regression slope and related price calculations.
	// 1e18 aligns with the common 18-decimal (wei-style) precision, allowing very fine-grained
	// slope values while keeping intermediate products such as `slope * sumT` within uint256
	// bounds for typical oracle data ranges and reasonable regression window sizes.

[Copilot]

The _computeSlope function (lines 306-310) converts the slope numerator and denominator to a fixed-point representation using Math.mulDiv. However, there's no validation that the resulting slope is within reasonable bounds.

An extremely steep slope (either positive or negative) could result in:

1. Rapid extrapolation that produces unrealistic prices within short time periods
2. Potential for the extrapolated price to become negative (already checked on line 369)
3. Integer overflow when computing slope * (block.timestamp - baseTimestamp) on line 367 for very large slopes and time differences

Consider adding bounds checking on the computed slope value, or documenting the expected range of valid slopes. For example, if a price doubles every hour, the slope would be extremely high and might indicate bad oracle data or manipulation. While the current implementation will fail safely (reverting with InvalidOracleData if price becomes non-positive), more explicit validation could provide better error messages and prevent potential edge cases.

[Copilot]

The condition latestRoundId > 0 && latestRoundId + 1 >= n is incorrect for validating sufficient oracle data. According to the Oracle contract (Oracle.sol:58-65, 108), roundIds are 0-indexed and latestRoundData returns the data for _history.length() - 1. This means if we have published 3 prices, latestRoundId will be 2 (not 3).

The current check latestRoundId + 1 >= n would allow n=3 when latestRoundId=2, which is correct. However, the check latestRoundId > 0 incorrectly rejects the case when latestRoundId=0 (i.e., only one price published), even though this should be rejected because we need at least 2 prices for n=2.

The correct validation should be latestRoundId + 1 >= n without the latestRoundId > 0 check, or more clearly: require(latestRoundId >= n - 1, InvalidOracleData()). The current implementation happens to work correctly because n >= 2 is enforced on line 316, so when n=2, we need latestRoundId >= 1, which the current check enforces. But the logic is confusing and could lead to bugs if the minimum n value changes.

Suggested change

	require(latestRoundId > 0 && latestRoundId + 1 >= n, InvalidOracleData());
	require(latestRoundId >= n - 1, InvalidOracleData());

[Copilot]

In the linear regression computation, there's a potential issue with integer overflow in lines 337-338 where t * price and t * t are computed. Since t is the time difference in seconds (can be very large for old data) and price is in oracle decimals (typically 6-18 decimals), the multiplication t * price could overflow for large time differences or high precision prices.

For example, if we have 30 data points spanning several days, the last data point could have t on the order of 2,592,000 seconds (30 days). If the price is expressed in 18 decimals (e.g., 2e18), then t * price could be around 5e24, which is well within int256 range. However, for t * t, with t=2,592,000, we get approximately 6.7e12, which is safe.

While the current implementation is likely safe given reasonable oracle update frequencies and price ranges, consider adding overflow checks or documentation about the expected ranges for oracle data to prevent future issues if oracle update patterns change.

Suggested change

	sumP += price;
	sumP += price;
	// Prevent overflow in t * price and t * t
	if (t != 0) {
	// Assume oracle prices are non-negative; guard against multiplication overflow
	if (price > 0) {
	require(price <= type(int256).max / t, InvalidOracleData());
	} else if (price < 0) {
	// For negative prices, ensure |price| * t does not exceed int256 bounds
	require(price >= type(int256).min / t, InvalidOracleData());
	}
	// Guard against overflow in t * t
	require(t <= type(int256).max / t, InvalidOracleData());
	}

[Copilot]

The extrapolated price calculation on line 368 could result in a negative price if the slope is negative and the extrapolation goes far enough into the future. While line 369 checks that min > 0, this check comes after the calculation. If intercept + (negative slope adjustment) results in a value <= 0, the transaction will revert with InvalidOracleData().

This is actually the correct behavior (preventing negative prices), but it means that with a negative price trend, the pricing will fail after a certain time threshold is reached. Consider documenting this behavior and potentially adding a maximum extrapolation time limit in setPair or _getPrices to prevent pricing failures due to excessive extrapolation. Alternatively, the system could fall back to the latest oracle price when extrapolation would result in non-positive prices.

Suggested change

	uint256 absDeltaPrice = Math.mulDiv(SignedMath.abs(slope), block.timestamp - baseTimestamp, _PRECISION);
	min = intercept + (slope < 0 ? -absDeltaPrice.toInt256() : absDeltaPrice.toInt256());
	require(min > 0, InvalidOracleData());
	max = min;
	uint256 absDeltaPrice = Math.mulDiv(
	SignedMath.abs(slope),
	block.timestamp - baseTimestamp,
	_PRECISION
	);
	int256 extrapolated = intercept + (
	slope < 0 ? -absDeltaPrice.toInt256() : absDeltaPrice.toInt256()
	);
	// If extrapolation would result in a non-positive price, fall back to latest oracle price
	if (extrapolated <= 0) {
	min = latest;
	max = latest;
	} else {
	min = extrapolated;
	max = extrapolated;
	}

[Copilot]

The linear regression implementation uses the standard least squares formula, but there's a subtle issue with the intercept calculation on line 347. The formula intercept = (sumP - (slope * sumT) / int256(_PRECISION)) / nInt computes the intercept in the wrong order of operations.

The standard formula is: intercept = (sumP - slope * sumT) / n, where the slope should already be scaled. However, because slope is multiplied by _PRECISION in _computeSlope (line 308), we need to divide by _PRECISION when using it. The current implementation does this: (sumP - (slope * sumT) / _PRECISION) / n.

This is mathematically correct, but the division (slope * sumT) / _PRECISION could lose precision due to integer division before subtracting from sumP. Consider rewriting as: intercept = (sumP * int256(_PRECISION) - slope * sumT) / (nInt * int256(_PRECISION)) to minimize precision loss, or document why the current approach is acceptable given the expected ranges of values.

Suggested change

	intercept = (sumP - (slope * sumT) / int256(_PRECISION)) / nInt;
	intercept = (sumP * int256(_PRECISION) - slope * sumT) / (nInt * int256(_PRECISION));

[Copilot]

On line 367, there's a potential underflow issue if block.timestamp < baseTimestamp. This could happen if oracle data is published with future timestamps relative to the current block time, which shouldn't be possible in normal operation but could occur due to timestamp manipulation or incorrect oracle data.

The subtraction block.timestamp - baseTimestamp would underflow in Solidity 0.8+, causing a revert. While this is safe (no silent bugs), consider adding an explicit check like require(block.timestamp >= baseTimestamp, InvalidOracleData()) with a clearer error message, or document that oracle timestamps must not be in the future relative to swap execution time. This would make debugging easier if such a situation occurs.

Suggested change

	// price = slope * (currentTime - baseTimestamp) / _PRECISION + intercept
	// price = slope * (currentTime - baseTimestamp) / _PRECISION + intercept
	require(block.timestamp >= baseTimestamp, InvalidOracleData());

[Copilot]

The linear regression computation in _computeLinearRegression performs a loop that makes n external calls to oracle.getRoundData() (line 333). For the maximum value of n=30, this results in 30 external calls per price query, which could be quite expensive in terms of gas.

While this is a view function and doesn't consume gas when called off-chain via eth_call, it does consume gas when called as part of a swap transaction. The gas cost scales linearly with accrualRounds, so users setting high accrualRounds values should be aware of the increased transaction costs.

Consider:

1. Documenting the gas implications of higher accrualRounds values
2. Adding a gas usage test to quantify the cost difference between accrualRounds=0, 2, and 30
3. Potentially caching the regression parameters if they're used multiple times within the same block

The current implementation is functional but may be prohibitively expensive for high accrualRounds values on expensive chains.

[Copilot]

The new accrualRounds field added to the Pair struct (line 34) lacks documentation explaining its purpose and behavior. Given that this is a significant new feature that changes the pricing mechanism from min/max of two prices to linear regression extrapolation, it would be valuable to add a comment block explaining:

1. What accrualRounds represents (number of historical price points used for linear regression)
2. The valid range (0 for disabled, or 2-30 for enabled)
3. The impact on pricing (0 uses min/max, non-zero uses extrapolated price)
4. Gas cost implications for higher values

This documentation would help future developers understand the feature without having to reverse-engineer the implementation from _computeLinearRegression and _getPrices. The existing comment block (lines 36-54) documents the numerator/denominator fields well and serves as a good example to follow.

Suggested change

	}
	}
	// `accrualRounds` controls how many historical oracle price points are used when computing
	// a linear regression to extrapolate the current price for this pair.
	//
	// - A value of 0 disables linear regression pricing and falls back to the legacy behavior
	// that uses the min/max of two prices returned by the oracle.
	// - A non‑zero value enables linear regression pricing. The value specifies the number of
	// past rounds (price points) to include in the regression window.
	//
	// Valid values are:
	// - 0: disabled (use min/max pricing)
	// - [2, _MAX_REGRESSION_POINTS]: enabled (use regression/extrapolated price)
	//
	// Larger `accrualRounds` values may provide a smoother, more time‑aware price signal but
	// increase gas costs, since more historical rounds must be fetched and processed.
	//

[Copilot]

The linear regression implementation doesn't handle the edge case where all timestamps are identical (all oracle prices published at the same time). In this scenario, sumT2 would equal 0 (since all t values would be 0), making denominator = nInt * 0 - 0 * 0 = 0, which would fail the check on line 344.

While this is an extremely unlikely scenario in production (oracles shouldn't publish multiple prices at the exact same timestamp), consider whether this edge case should be explicitly handled with a more descriptive error, or if the current behavior (revert with InvalidOracleData) is acceptable. The current behavior is safe but could be confusing to debug.

[Amxx]

Suggested change

	function _computeSlope(int256 numerator, int256 denominator) private pure returns (int256) {
	bool negative = (numerator < 0) != (denominator < 0);
	uint256 absSlope = Math.mulDiv(SignedMath.abs(numerator), _PRECISION, SignedMath.abs(denominator));
	return negative ? -absSlope.toInt256() : absSlope.toInt256();
	}
	function _computeLinearRegression(
	Oracle oracle,
	uint8 n
	) internal view returns (int256 slope, int256 intercept, uint48 baseTimestamp) {
	require(n >= 2 && n <= _MAX_REGRESSION_POINTS, InvalidAccrualRounds(n));
	uint80 latestRoundId;
	(latestRoundId, , , , ) = oracle.latestRoundData();
	require(latestRoundId > 0 && latestRoundId + 1 >= n, InvalidOracleData());
	uint80 startRoundId = latestRoundId + 1 - n;
	uint256 baseTs;
	(, , baseTs, , ) = oracle.getRoundData(startRoundId);
	baseTimestamp = uint48(baseTs);
	int256 sumT;
	int256 sumP;
	int256 sumTP;
	int256 sumT2;
	for (uint8 i = 0; i < n; i++) {
	(, int256 price, uint256 timestamp, , ) = oracle.getRoundData(startRoundId + i);
	int256 t = (timestamp - baseTs).toInt256();
	sumT += t;
	sumP += price;
	sumTP += t * price;
	sumT2 += t * t;
	}
	int256 nInt = int256(uint256(n));
	int256 denominator = nInt * sumT2 - sumT * sumT;
	require(denominator > 0, InvalidOracleData());
	slope = _computeSlope(nInt * sumTP - sumT * sumP, denominator);
	intercept = (sumP - (slope * sumT) / int256(_PRECISION)) / nInt;
	}
	function _getPrices(
	Oracle oracle,
	uint256 oracleTTL,
	uint8 accrualRounds
	) internal view virtual returns (int256 min, int256 max) {
	(uint80 roundId, int256 latest, , uint256 updatedAt, ) = oracle.latestRoundData();
	require(block.timestamp < updatedAt + oracleTTL, OracleValueTooOld(oracle));
	min = SignedMath.min(latest, previous);
	max = SignedMath.max(latest, previous);
	if (accrualRounds == 0) {
	require(roundId >= 1, InvalidOracleData());
	(, int256 previous, , , ) = oracle.getRoundData(roundId - 1);
	min = SignedMath.min(latest, previous);
	max = SignedMath.max(latest, previous);
	} else {
	(int256 slope, int256 intercept, uint48 baseTimestamp) = _computeLinearRegression(oracle, accrualRounds);
	// price = slope * (currentTime - baseTimestamp) / _PRECISION + intercept
	uint256 absDeltaPrice = Math.mulDiv(SignedMath.abs(slope), block.timestamp - baseTimestamp, _PRECISION);
	min = intercept + (slope < 0 ? -absDeltaPrice.toInt256() : absDeltaPrice.toInt256());
	require(min > 0, InvalidOracleData());
	max = min;
	}
	}
	function _getPrices(
	Oracle oracle,
	uint256 oracleTTL,
	uint256 accrualRounds
	) internal view virtual returns (int256 min, int256 max) {
	(uint80 roundId, int256 latest, , uint256 updatedAt, ) = oracle.latestRoundData();
	require(roundId > accrualRounds, InvalidOracleData());
	require(block.timestamp < updatedAt + oracleTTL, OracleValueTooOld(oracle));
	if (accrualRounds == 0) {
	(, int256 previous, , , ) = oracle.getRoundData(roundId - 1);
	min = SignedMath.min(latest, previous);
	max = SignedMath.max(latest, previous);
	} else {
	// COMMENT: This already checked by setPair, could be removed
	require(accrualRounds >= 2 && accrualRounds <= _MAX_REGRESSION_POINTS, InvalidAccrualRounds(accrualRounds));
	int256 sumT = 0;
	int256 sumP = 0;
	int256 sumTT = 0;
	int256 sumTP = 0;
	for (
	uint256 currentRound = roundId - accrualRounds + 1;
	currentRound <= roundId;
	++currentRound
	) {
	(, int256 price, uint256 timestamp, , ) = oracle.getRoundData(currentRound);
	sumT += timestamp;
	sumP += price;
	sumTT += timestamp * timestamp;
	sumTP += timestamp * price;
	}
	int256 numerator = accrualRounds * sumTP - sumT * sumP;
	int256 denominator = accrualRounds * sumTT - sumT * sumT;
	require(denominator > 0, InvalidOracleData());
	min = max = (sumP - sumT * numerator / denominator) / accrualRounds
	+ block.timestamp * numerator / denominator;
	require(min > 0, InvalidOracleData());
	}
	}