Submission Details
Severity:
Holder receives lesser reward due to division before multiplication
Summary
distributeAssets() calculates _portion which is awarded to the holder as reward. However in certain conditions, it performs division before multiplication as a result of which the holder receives less than expected reward.
Details:
On L219,
_portionis calculated as:
uint256 _portion = asset.amount * _positionStake / stakeTotal;
Then, on L222, the code checks if
costInEuros > _position.EUROsand iftrue,_portionis updated as:
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
This effectively makes the above line equivalent to:
_portion = (asset.amount * _positionStake / stakeTotal) * _position.EUROs / costInEuros; // @audit : division before multiplication
which is division before multiplication and hence causes precision loss. The correct code should have been:
if (costInEuros > _position.EUROs) {
_portion = asset.amount * _positionStake * _position.EUROs / costInEuros / stakeTotal;
I provide the PoC below which shows this loss of reward with example numbers.
PoC
We will execute this PoC in 3 steps -
Step 1 : Add a new file
test/reducedRewards.jswith the following code and run vianpx hardhat test --grep 'reduced rewards'. You will get the output as1, which is the reward received byuser1:
const { expect } = require("chai");
const { ethers } = require("hardhat");
const { ETH, getNFTMetadataContract, DEFAULT_ETH_USD_PRICE, DEFAULT_COLLATERAL_RATE, fastForward, DAY, POOL_FEE_PERCENTAGE, fullyUpgradedSmartVaultManager, PROTOCOL_FEE_RATE } = require("./common");
describe('Reward Calculation', async () => {
let LiquidationPoolManager, LiquidationPool, TokenManager,
TST, EUROs, ERC20MockFactory, admin, user1, user2, user3, protocol, liquidator, ClEurUsd, ClEthUsd, ClCoinUsd, VaultManager, Vault, collateralCoin, SmartVaultIndex;
const rewardAmountForAsset = (rewards, symbol) => {
return rewards.filter(reward => reward.symbol === ethers.utils.formatBytes32String(symbol))[0].amount;
}
beforeEach(async () => {
[admin, user1, user2, user3, protocol, liquidator] = await ethers.getSigners();
ERC20MockFactory = await ethers.getContractFactory('ERC20Mock');
TST = await ERC20MockFactory.deploy('The Standard Token', 'TST', 18);
EUROs = await (await ethers.getContractFactory('EUROsMock')).deploy();
ClEurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await ClEurUsd.setPrice(100000000); // for easier calculation
ClEthUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('ETH / USD');
await ClEthUsd.setPrice(DEFAULT_ETH_USD_PRICE);
TokenManager = await (await ethers.getContractFactory('TokenManagerMock')).deploy(ETH, ClEthUsd.address);
const SmartVaultDeployer = await (await ethers.getContractFactory('SmartVaultDeployerV3')).deploy(ETH, ClEurUsd.address);
SmartVaultIndex = await (await ethers.getContractFactory('SmartVaultIndex')).deploy();
const NFTMetadataGenerator = await (await getNFTMetadataContract()).deploy();
const SwapRouterMock = await (await ethers.getContractFactory('SwapRouterMock')).deploy();
const MockWeth = await (await ethers.getContractFactory('WETHMock')).deploy();
VaultManager = await fullyUpgradedSmartVaultManager(
DEFAULT_COLLATERAL_RATE, PROTOCOL_FEE_RATE, EUROs.address, protocol.address,
liquidator.address, TokenManager.address, SmartVaultDeployer.address,
SmartVaultIndex.address, NFTMetadataGenerator.address, MockWeth.address,
SwapRouterMock.address
);
await SmartVaultIndex.setVaultManager(VaultManager.address);
await EUROs.grantRole(await EUROs.DEFAULT_ADMIN_ROLE(), VaultManager.address);
await VaultManager.connect(user3).mint();
const _vault = (await VaultManager.connect(user3).vaults())[0];
const vaultAddress = _vault.status.vaultAddress;
Vault = await ethers.getContractAt('SmartVaultV3', vaultAddress);
const LiquidationPoolManagerContract = await ethers.getContractFactory('LiquidationPoolManager');
LiquidationPoolManager = await LiquidationPoolManagerContract.deploy(
TST.address, EUROs.address, VaultManager.address, ClEurUsd.address, protocol.address, POOL_FEE_PERCENTAGE
);
await VaultManager.setLiquidatorAddress(LiquidationPoolManager.address);
await VaultManager.setProtocolAddress(LiquidationPoolManager.address);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address);
// to be used for depositing as collateral inside tests
collateralCoin = await ERC20MockFactory.deploy('Collateral Coin', 'cCOIN', 18);
ClCoinUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('cCOIN / USD');
await ClCoinUsd.setPrice(DEFAULT_ETH_USD_PRICE);
await TokenManager.addAcceptedToken(collateralCoin.address, ClCoinUsd.address);
await collateralCoin.mint(user3.address, 4);
});
describe('reduced rewards', async () => {
it('rounds-down the rewards', async () => {
// Step 1. stake TST & EUROs
const tstStake1 = 3;
const eurosStake1 = 2490;
await TST.mint(user1.address, tstStake1);
await EUROs.mint(user1.address, eurosStake1);
await TST.connect(user1).approve(LiquidationPool.address, tstStake1);
await EUROs.connect(user1).approve(LiquidationPool.address, eurosStake1);
await LiquidationPool.connect(user1).increasePosition(tstStake1, eurosStake1);
const tstStake2 = 2;
const eurosStake2 = 2;
await TST.mint(user2.address, tstStake2);
await EUROs.mint(user2.address, eurosStake2);
await TST.connect(user2).approve(LiquidationPool.address, tstStake2);
await EUROs.connect(user2).approve(LiquidationPool.address, eurosStake2);
await LiquidationPool.connect(user2).increasePosition(tstStake2, eurosStake2);
await fastForward(DAY);
let { _position } = await LiquidationPool.position(user1.address);
expect(_position.TST).to.equal(tstStake1);
expect(_position.EUROs).to.equal(eurosStake1);
({ _position } = await LiquidationPool.position(user2.address));
expect(_position.TST).to.equal(tstStake2);
expect(_position.EUROs).to.equal(eurosStake2);
// Step 2. deposit collateral & mint EUROs
const userCollateral = 4;
await collateralCoin.connect(user3).mint(Vault.address, userCollateral);
const mintedEUROs = 5000;
await Vault.connect(user3).mint(user3.address, mintedEUROs);
expect(await EUROs.balanceOf(user3.address)).to.equal(mintedEUROs);
// Step 3. Liquidate
await ClCoinUsd.setPrice(150000000000); // $1500
await expect(LiquidationPoolManager.connect(user1).runLiquidation(1)).not.to.be.reverted;
const { minted, maxMintable, totalCollateralValue, collateral, liquidated } = await Vault.status();
expect(minted).to.equal(0);
expect(maxMintable).to.equal(0);
expect(totalCollateralValue).to.equal(0);
collateral.forEach(asset => expect(asset.amount).to.equal(0));
expect(liquidated).to.equal(true);
// check reward
let { _rewards } = await LiquidationPool.position(user1.address);
console.log("reward received by user1 =", rewardAmountForAsset(_rewards, 'cCOIN'));
});
});
});
Output:
Reward Calculation
reduced rewards
reward received by user1 = BigNumber { value: "1" }
Step 2 : Let's correct the code inside
contracts/LiquidationPool.solby applying the following patch:
diff --git a/contracts/LiquidationPool.sol b/contracts/LiquidationPool.sol
index e7ebe08..59fe773 100644
--- a/contracts/LiquidationPool.sol
+++ b/contracts/LiquidationPool.sol
contract LiquidationPool is ILiquidationPool {
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
- _portion = _portion * _position.EUROs / costInEuros;
+ _portion = asset.amount * _positionStake * _position.EUROs / costInEuros / stakeTotal;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
Step 3 : Run the test again via
npx hardhat test --grep 'reduced rewards'to receive the output:
Reward Calculation
reduced rewards
reward received by user1 = BigNumber { value: "2" }
As you can see, the current implementation awards user1 with 1 instead of his rightful amount of 2.
Impact
Loss of reward for the holder.
Tools Used
Hardhat
Recommendations
Apply the following patch:
diff --git a/contracts/LiquidationPool.sol b/contracts/LiquidationPool.sol
index e7ebe08..59fe773 100644
--- a/contracts/LiquidationPool.sol
+++ b/contracts/LiquidationPool.sol
contract LiquidationPool is ILiquidationPool {
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
- _portion = _portion * _position.EUROs / costInEuros;
+ _portion = asset.amount * _positionStake * _position.EUROs / costInEuros / stakeTotal;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
60933
USDC / LOC
17,500 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.