Skip to content
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.

Sign up for GitHub

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

use same calculation as CDPManager for CDP split fee in invariant test #799

Closed
wants to merge 1 commit into from
Closed

use same calculation as CDPManager for CDP split fee in invariant test #799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 16 additions & 5 deletions packages/contracts/contracts/TestContracts/invariants/BeforeAfter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ abstract contract BeforeAfter is BaseStorageVariables {
uint256 yieldStEthIndexAfter;
uint256 yieldCdpDebtBefore;
uint256 yieldCdpDebtAfter;
uint256 yieldActorSplitFeeIdxBefore;
uint256 yieldActorSplitFeeIdxAfter;
}

Vars vars;
Expand Down Expand Up @@ -172,12 +174,16 @@ abstract contract BeforeAfter is BaseStorageVariables {
vars.totalCollateralSnapshotBefore = cdpManager.totalCollateralSnapshot();

vars.yieldControlValueBefore = collateral.balanceOf(yieldControlAddress);
( vars.yieldCdpDebtBefore, vars.yieldActorSharesBefore) = cdpManager.getSyncedDebtAndCollShares(_cdpId);
(vars.yieldCdpDebtBefore, vars.yieldActorSharesBefore) = cdpManager
.getSyncedDebtAndCollShares(_cdpId);
vars.yieldActorValueBefore = collateral.getPooledEthByShares(vars.yieldActorSharesBefore);
vars.yieldProtocolValueBefore = collateral.getPooledEthByShares(activePool.getSystemCollShares());
vars.yieldProtocolValueBefore = collateral.getPooledEthByShares(
activePool.getSystemCollShares()
);
vars.yieldProtocolCollSharesBefore = activePool.getSystemCollShares();

vars.yieldStEthIndexBefore = cdpManager.stEthIndex();
vars.yieldActorSplitFeeIdxBefore = cdpManager.cdpStEthFeePerUnitIndex(_cdpId);
}

function _after(bytes32 _cdpId) internal {
Expand Down Expand Up @@ -254,11 +260,16 @@ abstract contract BeforeAfter is BaseStorageVariables {

// PYS
vars.yieldControlValueAfter = collateral.balanceOf(yieldControlAddress);
(vars.yieldCdpDebtAfter, vars.yieldActorSharesAfter) = cdpManager.getSyncedDebtAndCollShares(_cdpId);
(vars.yieldCdpDebtAfter, vars.yieldActorSharesAfter) = cdpManager.getSyncedDebtAndCollShares(
_cdpId
);
vars.yieldActorValueAfter = collateral.getPooledEthByShares(vars.yieldActorSharesAfter);
vars.yieldProtocolValuePerNewIndex = collateral.getPooledEthByShares(vars.activePoolCollBefore);
vars.yieldProtocolValuePerNewIndex = collateral.getPooledEthByShares(
vars.activePoolCollBefore
);
vars.yieldProtocolCollSharesAfter = activePool.getSystemCollShares();
vars.yieldStEthIndexAfter = cdpManager.stEthIndex();
vars.yieldActorSplitFeeIdxAfter = cdpManager.cdpStEthFeePerUnitIndex(_cdpId);
}

function _diff() internal view returns (string memory log) {
Expand Down
75 changes: 42 additions & 33 deletions packages/contracts/contracts/TestContracts/invariants/Properties.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,10 +588,11 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr

// If the CDP has 0 shares then it has been liquidated
if (_coll == 0) return true;

// In case the Yield Actor closes or redeems, we check their address as well
// Note: we must also remember the gas stipend gets subtracted from the shares at the start
uint256 yieldTargetCollateral = collateral.balanceOf(address(actors[yieldTarget])) + collateral.getPooledEthByShares(_coll);
uint256 yieldTargetCollateral = collateral.balanceOf(address(actors[yieldTarget])) +
collateral.getPooledEthByShares(_coll);

/**
If the collateral does not match then we prob have a cdp that has been partially redeemed by another party
Expand All @@ -604,18 +605,15 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
uint256 collValue;
if (debt < eBtcAmount) {
// Price is in btc/stETH
yieldTargetCollateral += (eBtcAmount - debt) * priceFeedMock.getPrice() / 1e18;
yieldTargetCollateral += ((eBtcAmount - debt) * priceFeedMock.getPrice()) / 1e18;
}

// Is approximate eq good here? 1e8 would be dust
return _assertApproximateEq(collateral.balanceOf(controlActor), yieldTargetCollateral, 1e8);
return _assertApproximateEq(collateral.balanceOf(controlActor), yieldTargetCollateral, 1e8);
}

// At all times the fees taken from systemCollShares should be added to feeRecipientCollShares
function invariant_PYS_02(
CdpManager cdpManager,
Vars memory vars
) internal view returns (bool) {
function invariant_PYS_02(CdpManager cdpManager, Vars memory vars) internal view returns (bool) {
// If yieldControlAddress is not set this invariant cannot be tested
if (yieldControlAddress == address(0)) return true;
// If our target CDP has no collateral then we cannot test this invariant
Expand All @@ -624,19 +622,20 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
// In total the yield growth percentage of the control must be equal to protocol growth percentage
// Has there been yield?
if (vars.prevStEthFeeIndex < vars.afterStEthFeeIndex) {
return _assertApproximateEq(vars.yieldProtocolCollSharesBefore - vars.yieldProtocolCollSharesAfter, vars.feeRecipientCollSharesAfter - vars.feeRecipientCollSharesBefore, 1e4);
return
_assertApproximateEq(
vars.yieldProtocolCollSharesBefore - vars.yieldProtocolCollSharesAfter,
vars.feeRecipientCollSharesAfter - vars.feeRecipientCollSharesBefore,
1e4
);
}

// Implies there was no yield
return true;

}

// As a user, the value of my CDP after rebase should be equal to it's value prior to rebase + yield - fee, according to the PYS at the moment of rebase
function invariant_PYS_03(
CdpManager cdpManager,
Vars memory vars
) internal view returns (bool) {
function invariant_PYS_03(CdpManager cdpManager, Vars memory vars) internal view returns (bool) {
// If yieldControlAddress is not set this invariant cannot be tested
if (yieldControlAddress == address(0)) return true;
// If our target CDP has no collateral then we cannot test this invariant
Expand All @@ -654,31 +653,42 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
// Has there been yield?
if (vars.yieldStEthIndexAfter > vars.yieldStEthIndexBefore) {
// Determine the growth, adjusted to 1e18 precision
uint256 yieldGrowthPercent = (vars.yieldStEthIndexAfter - vars.yieldStEthIndexBefore) * 1e18 / vars.yieldStEthIndexBefore;
// uint256 yieldGrowthPercent = (vars.yieldStEthIndexAfter - vars.yieldStEthIndexBefore) * 1e18 / vars.yieldStEthIndexBefore;

// Just calling here to check if there is a change (which is expected)
// Chasing down a bug, please excuse the calls
collateral.getPooledEthByShares(vars.yieldStEthIndexBefore);
collateral.getPooledEthByShares(vars.yieldStEthIndexAfter);
collateral.getPooledEthByShares(vars.yieldActorSharesBefore);
collateral.getPooledEthByShares(vars.yieldActorSharesAfter);
collateral._getTotalPooledEther();
collateral._getTotalShares();
// collateral.getPooledEthByShares(vars.yieldStEthIndexBefore);
// collateral.getPooledEthByShares(vars.yieldStEthIndexAfter);
// collateral.getPooledEthByShares(vars.yieldActorSharesBefore);
// collateral.getPooledEthByShares(vars.yieldActorSharesAfter);
// collateral._getTotalPooledEther();
// collateral._getTotalShares();

// Expected fee is (growth % * startingVal) * feeSplit %
uint256 expectedFee = (vars.yieldActorValueBefore * yieldGrowthPercent / 1e18) * cdpManager.stakingRewardSplit() / cdpManager.MAX_REWARD_SPLIT();
uint256 feeInShares = collateral.getSharesByPooledEth(expectedFee);

return _assertApproximateEq(
// It seems that the actual fees taken are always slightly higher than expected. Why?
collateral.getPooledEthByShares(vars.yieldActorSharesBefore - feeInShares),
collateral.getPooledEthByShares(vars.yieldActorSharesAfter),
1e6 // @audit more acceptable precision loss
// TO DO: tweak and decrease tolerance until acceptable
// uint256 expectedFee = (vars.yieldActorValueBefore * yieldGrowthPercent / 1e18) * cdpManager.stakingRewardSplit() / cdpManager.MAX_REWARD_SPLIT();
uint256 feeInShares = vars.cdpStakeBefore *
(vars.afterStEthFeeIndex - vars.yieldActorSplitFeeIdxBefore); //collateral.getSharesByPooledEth(expectedFee);
require(
vars.yieldActorSplitFeeIdxAfter == vars.afterStEthFeeIndex,
"!fee index should be synced"
);
uint256 _collAfterFeeApplied = (vars.yieldActorSharesBefore * 1e18 - feeInShares) / 1e18;
// CDP collateral value should increase after positive rebase even with fee split
require(
(vars.yieldActorSharesBefore * vars.yieldStEthIndexBefore) <
(vars.yieldActorSharesAfter * vars.yieldStEthIndexAfter),
"!CDP collateral value should increase after positive rebase"
);
}


return
_assertApproximateEq(
// It seems that the actual fees taken are always slightly higher than expected. Why?
(_collAfterFeeApplied),
(vars.yieldActorSharesAfter),
1e6 // @audit more acceptable precision loss
// TO DO: tweak and decrease tolerance until acceptable
);
}

/***
deltaIndex = 1027643607040687991 - 1000000000000000000
Expand Down Expand Up @@ -727,7 +737,6 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr

// Implies there was no yield
return true;

}
//
}
Loading