Skip to content

Commit c9797bc

Browse files
miguelmtzinfavniculae
miguelmtzinf
and
avniculae
authored
fix: Removes multiple rounding on totalAddedAssets by grouping all liabilities (#1033)
Co-authored-by: Alexandru Niculae <[email protected]>
1 parent 06ee850 commit c9797bc

10 files changed

+211
-128
lines changed

‎snapshots/Hub.Operations.json‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
{
2-
"add": "90212",
3-
"add: with transfer": "111854",
2+
"add": "90010",
3+
"add: with transfer": "111652",
44
"draw": "107733",
5-
"eliminateDeficit: full": "62124",
6-
"eliminateDeficit: partial": "71772",
7-
"mintFeeShares": "86229",
8-
"payFee": "74134",
5+
"eliminateDeficit: full": "61922",
6+
"eliminateDeficit: partial": "71570",
7+
"mintFeeShares": "86104",
8+
"payFee": "74211",
99
"refreshPremium": "77580",
10-
"remove: full": "79093",
11-
"remove: partial": "83740",
12-
"reportDeficit": "121107",
13-
"restore: full": "86184",
14-
"restore: full - with transfer": "197201",
15-
"restore: partial": "94550",
16-
"restore: partial - with transfer": "144241",
17-
"transferShares": "73024"
10+
"remove: full": "78891",
11+
"remove: partial": "83538",
12+
"reportDeficit": "121184",
13+
"restore: full": "86261",
14+
"restore: full - with transfer": "207278",
15+
"restore: partial": "94627",
16+
"restore: partial - with transfer": "146818",
17+
"transferShares": "73101"
1818
}

‎snapshots/NativeTokenGateway.Operations.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
2-
"borrowNative": "239160",
3-
"repayNative": "256425",
4-
"supplyAsCollateralNative": "162833",
5-
"supplyNative": "138371",
6-
"withdrawNative: full": "127936",
7-
"withdrawNative: partial": "139719"
2+
"borrowNative": "238958",
3+
"repayNative": "256223",
4+
"supplyAsCollateralNative": "162631",
5+
"supplyNative": "138209",
6+
"withdrawNative: full": "127612",
7+
"withdrawNative: partial": "139315"
88
}

‎snapshots/SignatureGateway.Operations.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"borrowWithSig": "223263",
3-
"repayWithSig": "253338",
2+
"borrowWithSig": "223061",
3+
"repayWithSig": "253136",
44
"setSelfAsUserPositionManagerWithSig": "75374",
55
"setUsingAsCollateralWithSig": "85130",
6-
"supplyWithSig": "154862",
6+
"supplyWithSig": "154700",
77
"updateUserDynamicConfigWithSig": "62857",
88
"updateUserRiskPremiumWithSig": "61557",
9-
"withdrawWithSig": "133862"
9+
"withdrawWithSig": "133539"
1010
}

‎snapshots/Spoke.Getters.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"getUserAccountData: supplies: 0, borrows: 0": "11959",
3-
"getUserAccountData: supplies: 1, borrows: 0": "50965",
4-
"getUserAccountData: supplies: 2, borrows: 0": "85086",
5-
"getUserAccountData: supplies: 2, borrows: 1": "106818",
6-
"getUserAccountData: supplies: 2, borrows: 2": "127192"
3+
"getUserAccountData: supplies: 1, borrows: 0": "50763",
4+
"getUserAccountData: supplies: 2, borrows: 0": "84682",
5+
"getUserAccountData: supplies: 2, borrows: 1": "106414",
6+
"getUserAccountData: supplies: 2, borrows: 2": "126788"
77
}

‎snapshots/Spoke.Operations.ZeroRiskPremium.json‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "199502",
3-
"borrow: second action, same reserve": "179474",
4-
"liquidationCall (receiveShares): full": "311296",
5-
"liquidationCall (receiveShares): partial": "311014",
6-
"liquidationCall: full": "321234",
7-
"liquidationCall: partial": "320952",
8-
"permitReserve + repay (multicall)": "214178",
9-
"permitReserve + supply (multicall)": "148866",
10-
"permitReserve + supply + enable collateral (multicall)": "162666",
11-
"repay: full": "174287",
12-
"repay: partial": "192941",
2+
"borrow: first": "199377",
3+
"borrow: second action, same reserve": "179349",
4+
"liquidationCall (receiveShares): full": "310363",
5+
"liquidationCall (receiveShares): partial": "310081",
6+
"liquidationCall: full": "320301",
7+
"liquidationCall: partial": "320019",
8+
"permitReserve + repay (multicall)": "213976",
9+
"permitReserve + supply (multicall)": "148664",
10+
"permitReserve + supply + enable collateral (multicall)": "162464",
11+
"repay: full": "174085",
12+
"repay: partial": "192739",
1313
"setUserPositionManagerWithSig: disable": "44824",
1414
"setUserPositionManagerWithSig: enable": "68853",
15-
"supply + enable collateral (multicall)": "142739",
16-
"supply: 0 borrows, collateral disabled": "125727",
17-
"supply: 0 borrows, collateral enabled": "108649",
18-
"supply: second action, same reserve": "108627",
15+
"supply + enable collateral (multicall)": "142537",
16+
"supply: 0 borrows, collateral disabled": "125525",
17+
"supply: 0 borrows, collateral enabled": "108447",
18+
"supply: second action, same reserve": "108425",
1919
"updateUserDynamicConfig: 1 collateral": "73782",
2020
"updateUserDynamicConfig: 2 collaterals": "88639",
21-
"updateUserRiskPremium: 1 borrow": "99069",
22-
"updateUserRiskPremium: 2 borrows": "110682",
21+
"updateUserRiskPremium: 1 borrow": "98867",
22+
"updateUserRiskPremium: 2 borrows": "110557",
2323
"usingAsCollateral: 0 borrows, enable": "59004",
24-
"usingAsCollateral: 1 borrow, disable": "109331",
24+
"usingAsCollateral: 1 borrow, disable": "109129",
2525
"usingAsCollateral: 1 borrow, enable": "41892",
26-
"usingAsCollateral: 2 borrows, disable": "132258",
26+
"usingAsCollateral: 2 borrows, disable": "132056",
2727
"usingAsCollateral: 2 borrows, enable": "41904",
28-
"withdraw: 0 borrows, full": "131341",
29-
"withdraw: 0 borrows, partial": "136247",
30-
"withdraw: 1 borrow, partial": "165754",
31-
"withdraw: 2 borrows, partial": "180876",
32-
"withdraw: non collateral": "108975"
28+
"withdraw: 0 borrows, full": "130937",
29+
"withdraw: 0 borrows, partial": "135641",
30+
"withdraw: 1 borrow, partial": "165148",
31+
"withdraw: 2 borrows, partial": "180270",
32+
"withdraw: non collateral": "108571"
3333
}

‎snapshots/Spoke.Operations.json‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
{
2-
"borrow: first": "270956",
3-
"borrow: second action, same reserve": "239428",
4-
"liquidationCall (receiveShares): full": "346078",
5-
"liquidationCall (receiveShares): partial": "345796",
6-
"liquidationCall: full": "356016",
7-
"liquidationCall: partial": "355734",
8-
"permitReserve + repay (multicall)": "208928",
9-
"permitReserve + supply (multicall)": "148866",
10-
"permitReserve + supply + enable collateral (multicall)": "162666",
11-
"repay: full": "169037",
12-
"repay: partial": "227707",
2+
"borrow: first": "270831",
3+
"borrow: second action, same reserve": "239303",
4+
"liquidationCall (receiveShares): full": "345145",
5+
"liquidationCall (receiveShares): partial": "344863",
6+
"liquidationCall: full": "355083",
7+
"liquidationCall: partial": "354801",
8+
"permitReserve + repay (multicall)": "208726",
9+
"permitReserve + supply (multicall)": "148664",
10+
"permitReserve + supply + enable collateral (multicall)": "162464",
11+
"repay: full": "168835",
12+
"repay: partial": "227505",
1313
"setUserPositionManagerWithSig: disable": "44824",
1414
"setUserPositionManagerWithSig: enable": "68853",
15-
"supply + enable collateral (multicall)": "142739",
16-
"supply: 0 borrows, collateral disabled": "125727",
17-
"supply: 0 borrows, collateral enabled": "108649",
18-
"supply: second action, same reserve": "108627",
15+
"supply + enable collateral (multicall)": "142537",
16+
"supply: 0 borrows, collateral disabled": "125525",
17+
"supply: 0 borrows, collateral enabled": "108447",
18+
"supply: second action, same reserve": "108425",
1919
"updateUserDynamicConfig: 1 collateral": "73782",
2020
"updateUserDynamicConfig: 2 collaterals": "88639",
21-
"updateUserRiskPremium: 1 borrow": "228357",
22-
"updateUserRiskPremium: 2 borrows": "303137",
21+
"updateUserRiskPremium: 1 borrow": "228232",
22+
"updateUserRiskPremium: 2 borrows": "303166",
2323
"usingAsCollateral: 0 borrows, enable": "59004",
24-
"usingAsCollateral: 1 borrow, disable": "238619",
24+
"usingAsCollateral: 1 borrow, disable": "238494",
2525
"usingAsCollateral: 1 borrow, enable": "41892",
26-
"usingAsCollateral: 2 borrows, disable": "334713",
26+
"usingAsCollateral: 2 borrows, disable": "334665",
2727
"usingAsCollateral: 2 borrows, enable": "41904",
28-
"withdraw: 0 borrows, full": "131341",
29-
"withdraw: 0 borrows, partial": "136247",
30-
"withdraw: 1 borrow, partial": "292539",
31-
"withdraw: 2 borrows, partial": "276125",
32-
"withdraw: non collateral": "108975"
28+
"withdraw: 0 borrows, full": "130937",
29+
"withdraw: 0 borrows, partial": "135641",
30+
"withdraw: 1 borrow, partial": "292010",
31+
"withdraw: 2 borrows, partial": "275519",
32+
"withdraw: non collateral": "108571"
3333
}

‎src/hub/libraries/AssetLogic.sol‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,19 @@ library AssetLogic {
8080
/// @notice Returns the total added assets for the specified asset.
8181
function totalAddedAssets(IHub.Asset storage asset) internal view returns (uint256) {
8282
uint256 drawnIndex = asset.getDrawnIndex();
83+
84+
uint256 premiumRay = Premium.calculatePremiumRay({
85+
premiumShares: asset.premiumShares,
86+
drawnIndex: drawnIndex,
87+
premiumOffsetRay: asset.premiumOffsetRay,
88+
realizedPremiumRay: asset.realizedPremiumRay
89+
});
90+
uint256 aggregatedOwedRay = (asset.drawnShares * drawnIndex) + premiumRay + asset.deficitRay;
91+
8392
return
8493
asset.liquidity +
8594
asset.swept +
86-
asset.deficitRay.fromRayUp() +
87-
asset.totalOwed(drawnIndex) -
95+
aggregatedOwedRay.fromRayUp() -
8896
asset.realizedFees -
8997
asset.getUnrealizedFees(drawnIndex);
9098
}
@@ -183,26 +191,32 @@ library AssetLogic {
183191
}
184192

185193
uint120 drawnShares = asset.drawnShares;
186-
uint256 liquidityGrowthDrawn = drawnShares.rayMulUp(drawnIndex) -
187-
drawnShares.rayMulUp(previousIndex);
188194

189-
uint256 realizedPremiumRay = asset.realizedPremiumRay;
190195
uint120 premiumShares = asset.premiumShares;
191196
uint256 premiumOffsetRay = asset.premiumOffsetRay;
192-
uint256 premiumRayAfter = Premium.calculatePremiumRay({
193-
premiumShares: premiumShares,
194-
drawnIndex: drawnIndex,
195-
premiumOffsetRay: premiumOffsetRay,
196-
realizedPremiumRay: realizedPremiumRay
197-
});
198-
uint256 premiumRayBefore = Premium.calculatePremiumRay({
199-
premiumShares: premiumShares,
200-
drawnIndex: previousIndex,
201-
premiumOffsetRay: premiumOffsetRay,
202-
realizedPremiumRay: realizedPremiumRay
203-
});
204-
uint256 liquidityGrowthPremium = premiumRayAfter.fromRayUp() - premiumRayBefore.fromRayUp();
197+
uint256 realizedPremiumRay = asset.realizedPremiumRay;
198+
uint256 deficitRay = asset.deficitRay;
199+
200+
uint256 aggregatedOwedRayAfter = (drawnShares * drawnIndex) +
201+
Premium.calculatePremiumRay({
202+
premiumShares: premiumShares,
203+
drawnIndex: drawnIndex,
204+
premiumOffsetRay: premiumOffsetRay,
205+
realizedPremiumRay: realizedPremiumRay
206+
}) +
207+
deficitRay;
208+
uint256 aggregatedOwedRayBefore = (drawnShares * previousIndex) +
209+
Premium.calculatePremiumRay({
210+
premiumShares: premiumShares,
211+
drawnIndex: previousIndex,
212+
premiumOffsetRay: premiumOffsetRay,
213+
realizedPremiumRay: realizedPremiumRay
214+
}) +
215+
deficitRay;
205216

206-
return (liquidityGrowthDrawn + liquidityGrowthPremium).percentMulDown(liquidityFee);
217+
return
218+
(aggregatedOwedRayAfter.fromRayUp() - aggregatedOwedRayBefore.fromRayUp()).percentMulDown(
219+
liquidityFee
220+
);
207221
}
208222
}

‎tests/Base.t.sol‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import {console2 as console} from 'forge-std/console2.sol';
1111

1212
// dependencies
1313
import {AggregatorV3Interface} from 'src/dependencies/chainlink/AggregatorV3Interface.sol';
14-
import {TransparentUpgradeableProxy, ITransparentUpgradeableProxy} from 'src/dependencies/openzeppelin/TransparentUpgradeableProxy.sol';
14+
import {
15+
TransparentUpgradeableProxy,
16+
ITransparentUpgradeableProxy
17+
} from 'src/dependencies/openzeppelin/TransparentUpgradeableProxy.sol';
1518
import {IERC20Metadata} from 'src/dependencies/openzeppelin/IERC20Metadata.sol';
1619
import {SafeCast} from 'src/dependencies/openzeppelin/SafeCast.sol';
1720
import {IERC20Errors} from 'src/dependencies/openzeppelin/IERC20Errors.sol';
@@ -45,7 +48,11 @@ import {AccessManagerEnumerable} from 'src/access/AccessManagerEnumerable.sol';
4548
import {HubConfigurator, IHubConfigurator} from 'src/hub/HubConfigurator.sol';
4649
import {Hub, IHub, IHubBase} from 'src/hub/Hub.sol';
4750
import {SharesMath} from 'src/hub/libraries/SharesMath.sol';
48-
import {AssetInterestRateStrategy, IAssetInterestRateStrategy, IBasicInterestRateStrategy} from 'src/hub/AssetInterestRateStrategy.sol';
51+
import {
52+
AssetInterestRateStrategy,
53+
IAssetInterestRateStrategy,
54+
IBasicInterestRateStrategy
55+
} from 'src/hub/AssetInterestRateStrategy.sol';
4956

5057
// spoke
5158
import {Spoke, ISpoke, ISpokeBase} from 'src/spoke/Spoke.sol';
@@ -2793,18 +2800,26 @@ abstract contract Base is Test {
27932800
/// @dev Calculate expected fees based on previous drawn index
27942801
function _calcUnrealizedFees(IHub hub, uint256 assetId) internal view returns (uint256) {
27952802
IHub.Asset memory asset = hub.getAsset(assetId);
2796-
uint256 lastDrawnIndex = asset.drawnIndex;
2803+
uint256 previousIndex = asset.drawnIndex;
27972804
uint256 drawnIndex = asset.drawnIndex.rayMulUp(
27982805
MathUtils.calculateLinearInterest(asset.drawnRate, uint40(asset.lastUpdateTimestamp))
27992806
);
2800-
uint256 liquidityGrowth = asset.drawnShares.rayMulUp(drawnIndex) -
2801-
asset.drawnShares.rayMulUp(lastDrawnIndex) +
2802-
(asset.premiumShares * drawnIndex - asset.premiumOffsetRay + asset.realizedPremiumRay)
2803-
.fromRayUp() -
2804-
(asset.premiumShares * lastDrawnIndex - asset.premiumOffsetRay + asset.realizedPremiumRay)
2805-
.fromRayUp();
28062807

2807-
return liquidityGrowth.percentMulDown(asset.liquidityFee);
2808+
uint256 liquidityGrowthAfter = (uint256(asset.drawnShares) + asset.premiumShares) *
2809+
drawnIndex -
2810+
asset.premiumOffsetRay +
2811+
asset.realizedPremiumRay +
2812+
asset.deficitRay;
2813+
uint256 liquidityGrowthBefore = (uint256(asset.drawnShares) + asset.premiumShares) *
2814+
previousIndex -
2815+
asset.premiumOffsetRay +
2816+
asset.realizedPremiumRay +
2817+
asset.deficitRay;
2818+
2819+
return
2820+
(liquidityGrowthAfter.fromRayUp() - liquidityGrowthBefore.fromRayUp()).percentMulDown(
2821+
asset.liquidityFee
2822+
);
28082823
}
28092824

28102825
function _getExpectedFeeReceiverAddedAssets(

‎tests/misc/supply_share_price_deficit.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Highlights the fact that totalAddedAssets does not decrease when a deficit is reported (hence the share price does not decrease).
2+
from z3 import *
3+
4+
RAY = IntVal(10**27)
5+
6+
def divUp(a, b):
7+
return (a + b - 1) / b
8+
9+
def fromRayUp(a):
10+
return divUp(a, RAY)
11+
12+
def fromRayDown(a):
13+
return a / RAY
14+
15+
def rayMulUp(a, b):
16+
return (a * b + RAY - 1) / RAY
17+
18+
def rayMulDown(a, b):
19+
return (a * b) / RAY
20+
21+
def totalAddedAssets(drawnShares, premiumDebtRay, deficitRay, drawnIndex):
22+
# return rayMulUp(drawnShares, drawnIndex) + fromRayUp(premiumDebtRay) + fromRayUp(deficitRay) # this is wrong
23+
# return rayMulDown(drawnShares, drawnIndex) + fromRayDown(premiumDebtRay) + fromRayDown(deficitRay) # this is wrong
24+
return fromRayUp(drawnShares * drawnIndex + premiumDebtRay + deficitRay)
25+
26+
def check(propertyDescription):
27+
print(f"\n-- {propertyDescription} --")
28+
result = s.check()
29+
if result == sat:
30+
print("Counterexample found:")
31+
print(s.model())
32+
elif result == unsat:
33+
print(f"Property holds.")
34+
elif result == unknown:
35+
print("Timed out or unknown.")
36+
37+
s = Solver()
38+
39+
drawnShares = Int('drawnShares')
40+
s.add(1 <= drawnShares, drawnShares <= 10**30)
41+
drawnIndex = Int('drawnIndex')
42+
s.add(RAY <= drawnIndex, drawnIndex < 100 * RAY)
43+
premiumDebtRay = Int('premiumDebtRay')
44+
s.add(0 <= premiumDebtRay, premiumDebtRay <= 10**30)
45+
deficitRay = Int('deficitRay')
46+
s.add(0 <= deficitRay, deficitRay <= 10**30)
47+
48+
deficitDrawnShares = Int('deficitDrawnShares')
49+
s.add(0 <= deficitDrawnShares, deficitDrawnShares <= drawnShares)
50+
deficitPremiumDebtRay = Int('deficitPremiumDebtRay')
51+
s.add(0 <= deficitPremiumDebtRay, deficitPremiumDebtRay <= premiumDebtRay)
52+
53+
totalAddedAssetsBefore = totalAddedAssets(drawnShares, premiumDebtRay, deficitRay, drawnIndex)
54+
totalAddedAssetsAfter = totalAddedAssets(drawnShares - deficitDrawnShares, premiumDebtRay - deficitPremiumDebtRay, deficitRay + deficitDrawnShares * drawnIndex + deficitPremiumDebtRay, drawnIndex)
55+
56+
s.push()
57+
s.add(totalAddedAssetsBefore > totalAddedAssetsAfter)
58+
check("Total added assets does not decrease after deficit is reported")

0 commit comments

Comments
 (0)