Skip to content

[VEN-2386]: Remove round-up in the conversion rates of Token Converters #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d5fb612
refactor: remove round-up in the conversion rates of token converters
Debugger022 Feb 8, 2024
21b8473
test: fork tests for single token converter
Debugger022 Feb 8, 2024
b3f420d
test: add amount validation test
Debugger022 Feb 10, 2024
5486b99
fix: round-up condition suggested by certik
Debugger022 Feb 12, 2024
09885c0
fix: lint
Debugger022 Feb 12, 2024
b457102
tests: add getAmountIn test
Debugger022 Feb 12, 2024
684155a
refactor: add round-up comment in netspec
Debugger022 Feb 13, 2024
9590f0a
feat: update converter implementation deployment files
Debugger022 Feb 13, 2024
987a287
feat: updating deployment files
Debugger022 Feb 13, 2024
a3bd0b8
fix: spelling
Debugger022 Feb 20, 2024
ba39ce0
docs: add extra audit reports for private conversions
chechu Feb 20, 2024
20d9df9
fix: _getAmountOut normalisation
Debugger022 Feb 21, 2024
1642ed2
test: release funds for all assets
Debugger022 Feb 21, 2024
c34f78f
refactor: fork tests
Debugger022 Feb 22, 2024
1819985
refactor: tokenInToOutConversion calculation in getAmountIn and getAm…
Debugger022 Feb 22, 2024
498441f
fix: round up for _getAmountIn
Debugger022 Feb 26, 2024
8c73c48
tests: add fork tests for getUpdatedAmountIn and getUpdatedAmountOut
Debugger022 Feb 26, 2024
6c74d7b
fix: amountOutMantissa in _getAmountOut()
Debugger022 Feb 26, 2024
8599fb1
test: use some utility functions and the expected env variables
chechu Feb 26, 2024
d2a99ef
refactor: remove redundant export statements
Debugger022 Feb 28, 2024
a556032
refactor: implement and utilize _divRoundingUp for safer rounding in …
Debugger022 Feb 28, 2024
65da556
feat: update converters deployment files for bsctestnet
Debugger022 Feb 28, 2024
dced648
feat: updating deployment files
Debugger022 Feb 28, 2024
8dc80e9
feat: update converters deployment files for bscmainnet
Debugger022 Feb 28, 2024
eed49a2
feat: updating deployment files
Debugger022 Feb 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added audits/081_privateConversions_certik_20240215.pdf
Binary file not shown.
Binary file not shown.
1 change: 1 addition & 0 deletions contracts/Test/imports.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ pragma solidity 0.8.13;

import { UpgradeableBeacon } from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import { BeaconProxy } from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
42 changes: 31 additions & 11 deletions contracts/TokenConverter/AbstractTokenConverter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo

/// @notice To get the amount of tokenAddressOut tokens sender could receive on providing amountInMantissa tokens of tokenAddressIn.
/// This function does not account for potential token transfer fees(in case of deflationary tokens)
/// @notice The amountInMantissa might be adjusted if amountOutMantissa is greater than the balance of the contract for tokenAddressOut
/// @dev This function retrieves values without altering token prices
/// @param amountInMantissa Amount of tokenAddressIn
/// @param tokenAddressIn Address of the token to convert
Expand Down Expand Up @@ -574,9 +575,7 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo

/// If contract has less liquidity for tokenAddressOut than amountOutMantissa
if (maxTokenOutReserve < amountOutMantissa) {
amountConvertedMantissa =
((maxTokenOutReserve * EXP_SCALE) + tokenInToOutConversion - 1) /
tokenInToOutConversion; //round-up
amountConvertedMantissa = _divRoundingUp(maxTokenOutReserve * EXP_SCALE, tokenInToOutConversion);
amountOutMantissa = maxTokenOutReserve;
}
}
Expand Down Expand Up @@ -808,6 +807,7 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo

/// @dev Converts tokens for tokenAddressIn for the amount of tokenAddressOut used for deflationary tokens
/// it is called by convertForExactTokensSupportingFeeOnTransferTokens function
/// @notice Advising users to input a smaller amountOutMantissa to avoid potential transaction revert
/// @param amountInMaxMantissa Max amount of tokenAddressIn
/// @param amountOutMantissa Amount of tokenAddressOut required as output
/// @param tokenAddressIn Address of the token to convert
Expand Down Expand Up @@ -1089,17 +1089,16 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo
/// conversion rate after considering incentive(conversionWithIncentive)
uint256 conversionWithIncentive = MANTISSA_ONE + incentive;

tokenInToOutConversion = (tokenInUnderlyingPrice * conversionWithIncentive) / tokenOutUnderlyingPrice;
/// amount of tokenAddressOut after including incentive as amountOutMantissa will be greater than actual as it gets
/// multiplied by conversionWithIncentive which will be >= 1
amountOutMantissa =
(amountInMantissa * tokenInUnderlyingPrice * conversionWithIncentive) /
(tokenOutUnderlyingPrice * EXP_SCALE);

tokenInToOutConversion = (tokenInUnderlyingPrice * conversionWithIncentive) / tokenOutUnderlyingPrice;
amountOutMantissa = (amountInMantissa * tokenInToOutConversion) / (EXP_SCALE);
}

/// @dev To get the amount of tokenAddressIn tokens sender would send on receiving amountOutMantissa tokens of tokenAddressOut
/// @dev This function retrieves values without altering token prices.
/// @dev For user conversions, the function returns an amountInMantissa that is rounded up, ensuring that the equivalent amountInMantissa
/// is obtained from users for corresponding amountOutMantissa, preventing any losses to the protocol. However, no rounding up is required for private conversions
/// @param amountOutMantissa Amount of tokenAddressOut user wants to receive
/// @param tokenAddressIn Address of the token to convert
/// @param tokenAddressOut Address of the token to get after conversion
Expand All @@ -1126,16 +1125,29 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo
uint256 tokenOutUnderlyingPrice = priceOracle.getPrice(tokenAddressOut);

uint256 incentive = configuration.incentive;
if ((address(converterNetwork) != address(0)) && (converterNetwork.isTokenConverter(msg.sender))) {

bool isPrivateConversion = address(converterNetwork) != address(0) &&
converterNetwork.isTokenConverter(msg.sender);
if (isPrivateConversion) {
incentive = 0;
}

/// conversion rate after considering incentive(conversionWithIncentive)
uint256 conversionWithIncentive = MANTISSA_ONE + incentive;
tokenInToOutConversion = (tokenInUnderlyingPrice * conversionWithIncentive) / tokenOutUnderlyingPrice;

/// amount of tokenAddressIn after considering incentive(i.e. amountInMantissa will be less than actual amountInMantissa if incentive > 0)
amountInMantissa = ((amountOutMantissa * EXP_SCALE) + tokenInToOutConversion - 1) / tokenInToOutConversion; //round-up
if (isPrivateConversion) {
amountInMantissa =
(amountOutMantissa * tokenOutUnderlyingPrice * EXP_SCALE) /
(tokenInUnderlyingPrice * conversionWithIncentive);
} else {
amountInMantissa = _divRoundingUp(
amountOutMantissa * tokenOutUnderlyingPrice * EXP_SCALE,
tokenInUnderlyingPrice * conversionWithIncentive
);
}

tokenInToOutConversion = (tokenInUnderlyingPrice * conversionWithIncentive) / tokenOutUnderlyingPrice;
}

/// @dev Check if msg.sender is allowed to convert as per onlyForPrivateConversions flag
Expand Down Expand Up @@ -1164,4 +1176,12 @@ abstract contract AbstractTokenConverter is AccessControlledV8, IAbstractTokenCo
/// @dev Get base asset address of the destination contract
/// @return Address of the base asset
function _getDestinationBaseAsset() internal view virtual returns (address) {}

/// @dev Performs division where the result is rounded up
/// @param numerator The numerator of the division operation
/// @param denominator The denominator of the division operation. Must be non-zero
/// @return The result of the division, rounded up
function _divRoundingUp(uint256 numerator, uint256 denominator) internal pure returns (uint256) {
return (numerator + denominator - 1) / denominator;
}
}
4 changes: 2 additions & 2 deletions deployments/bscmainnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -5127,7 +5127,7 @@
]
},
"RiskFundConverter_Implementation": {
"address": "0x2fB02b3D49D45967DA471C7685Ce524d833520Dc",
"address": "0xd420Bf9C31F6b4a98875B6e561b13aCB19210647",
"abi": [
{
"inputs": [
Expand Down Expand Up @@ -7632,7 +7632,7 @@
]
},
"SingleTokenConverterImp": {
"address": "0xe80f5A2adE24bEd740408B944197b3c24f2f2e91",
"address": "0x40ed28180Df01FdeB957224E4A5415704B9D5990",
"abi": [
{
"inputs": [],
Expand Down
72 changes: 36 additions & 36 deletions deployments/bscmainnet/RiskFundConverter_Implementation.json

Large diffs are not rendered by default.

67 changes: 34 additions & 33 deletions deployments/bscmainnet/SingleTokenConverterImp.json

Large diffs are not rendered by default.

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions deployments/bscmainnet_addresses.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@
"ProtocolShareReserve_Implementation": "0x86a2a5EB77984E923E7B5Af45819A8c8f870f061",
"ProtocolShareReserve_Proxy": "0xCa01D5A9A248a830E9D93231e791B1afFed7c446",
"RiskFundConverter": "0xA5622D276CcbB8d9BBE3D1ffd1BB11a0032E53F0",
"RiskFundConverter_Implementation": "0x2fB02b3D49D45967DA471C7685Ce524d833520Dc",
"RiskFundConverter_Implementation": "0xd420Bf9C31F6b4a98875B6e561b13aCB19210647",
"RiskFundConverter_Proxy": "0xA5622D276CcbB8d9BBE3D1ffd1BB11a0032E53F0",
"RiskFundV2": "0x2F377545Fd095fA59A56Cb1fD7456A2a0B781Cb6",
"SingleTokenConverterBeacon": "0x4c9D57b05B245c40235D720A5f3A592f3DfF11ca",
"SingleTokenConverterImp": "0xe80f5A2adE24bEd740408B944197b3c24f2f2e91",
"SingleTokenConverterImp": "0x40ed28180Df01FdeB957224E4A5415704B9D5990",
"USDCPrimeConverter": "0xa758c9C215B6c4198F0a0e3FA46395Fa15Db691b",
"USDTPrimeConverter": "0xD9f101AA67F3D72662609a2703387242452078C3",
"XVSVaultConverter": "0xd5b9AE835F4C59272032B3B954417179573331E0",
Expand Down
4 changes: 2 additions & 2 deletions deployments/bsctestnet.json
Original file line number Diff line number Diff line change
Expand Up @@ -5127,7 +5127,7 @@
]
},
"RiskFundConverter_Implementation": {
"address": "0xbF732B12f87963dF5d7c729E84E5CF3698F8d349",
"address": "0x857ebb8CAcb97DE5ab719320c9FB3aa16076bFe3",
"abi": [
{
"inputs": [
Expand Down Expand Up @@ -7584,7 +7584,7 @@
]
},
"SingleTokenConverterImp": {
"address": "0xf014C86b6071cfFB8fE39306F802770C70733a23",
"address": "0x42Ec3Eb6F23460dFDfa3aE5688f3415CDfE0C6AD",
"abi": [
{
"inputs": [],
Expand Down
68 changes: 34 additions & 34 deletions deployments/bsctestnet/RiskFundConverter_Implementation.json

Large diffs are not rendered by default.

67 changes: 34 additions & 33 deletions deployments/bsctestnet/SingleTokenConverterImp.json

Large diffs are not rendered by default.

Loading