Skip to content

Quote Collateral tests#368

Open
MishaShWoof wants to merge 8 commits intofeat/service-patchfrom
test/quoteCollateral
Open

Quote Collateral tests#368
MishaShWoof wants to merge 8 commits intofeat/service-patchfrom
test/quoteCollateral

Conversation

@MishaShWoof
Copy link
Copy Markdown
Collaborator

Quote Collateral Tests

quoteCollateral test suite — added a dedicated set of tests covering the quoteCollateral function, verifying its correct behavior.

Copy link
Copy Markdown
Collaborator

@VladyslavPerederWoof VladyslavPerederWoof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Division-by-zero panic (critical, untested)

assetPriceDiscounted is mulFactor(assetPrice, FACTOR_SCALE - discountFactor). If discountFactor == FACTOR_SCALE, the divisor becomes zero and the EVM panics.

discountFactor = storeFrontPriceFactor * (1 − liquidationFactor). This equals FACTOR_SCALE when storeFrontPriceFactor = 1e18 and liquidationFactor = 0. The constructor only rejects
storeFrontPriceFactor > FACTOR_SCALE, so the value 1e18 is allowed.

Missing test: Set storeFrontPriceFactor = 1e18, deploy an asset with liquidationFactor = 0, call quoteCollateral — verify whether it panics or not. This documents an arithmetic
boundary the protocol needs to decide about.

  1. storeFrontPriceFactor = 1e18 (100%) with normal liquidationFactor

Currently tested up to 80%. At 100% the discount is maximal:

discountFactor = 1e18 * (1 - 0.6) = 0.4e18
assetPriceDiscounted = assetPrice * 0.6 (non-zero, safe)

No test covers this valid extreme.

  1. Price changes between quoteCollateral and buyCollateral

In two separate transactions: call quoteCollateral to obtain expectedAmount, then move the price (via setRoundData), then call buyCollateral with the now-stale minAmount =
expectedAmount. The TooMuchSlippage revert should trigger because the new quote is lower. This validates the exact purpose of the minAmount slippage guard.

Most likely, this scenario isn't possible because quoteCollateral and buyCollateral must be called within the same transaction, as is done in Liquidator.sol.
But just to be sure, you can check.

Do it separately:

  • quote
  • update price
  • quote

and see if the value has changed

or try a more complex variation

set automine to false

make a batch of transactions in one block and see the difference.

  1. Different base token price (base != $1)

All tests fix USDC price at $1. The formula uses basePrice as a multiplicative factor, so when basePrice != 1e8 the result changes proportionally. There is no test that verifies
behavior when the base token has a non-unit price (e.g., simulating a WETH-based or WBTC-based market).

Comment thread test/quote-collateral-test.ts Outdated
Comment thread test/quote-collateral-test.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants