Skip to content

Commit

Permalink
fix: review
Browse files Browse the repository at this point in the history
  • Loading branch information
@vgorkavenko
vgorkavenko committed Nov 28, 2023
1 parent 166b0b9 commit 6c98090
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 49 deletions.
4 changes: 2 additions & 2 deletions src/CSAccounting.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ contract CSAccounting is
}

function setBondCurve(
uint256[] memory bondCurve_
uint256[] memory bondCurve
) external onlyRole(SET_BOND_CURVE_ROLE) {
_setBondCurve(bondCurve_);
_setBondCurve(bondCurve);
}

/// @notice Sets basis points of the bond multiplier for the given node operator.
Expand Down
47 changes: 26 additions & 21 deletions src/CSBondCurve.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ abstract contract CSBondCurve {
error InvalidMultiplier();

// @dev Keys count to bond amount mapping
// x - keys count
// y - bond amount for x keys
// i | array index
// i + 1 | keys count for particular bond amount
// bondCurve[i] | bond amount for `i + 1` keys count
uint256[] public bondCurve;

uint256 internal constant MIN_CURVE_LENGTH = 2;
uint256 internal constant MIN_CURVE_LENGTH = 1;
// todo: might be redefined in the future
uint256 internal constant MAX_CURVE_LENGTH = 20;
uint256 internal constant BASIS_POINTS = 10000;
Expand All @@ -23,14 +24,18 @@ abstract contract CSBondCurve {
/// By default, all Node Operators have x1 multiplier (10000 basis points).
mapping(uint256 => uint256) internal _bondMultiplierBP;

uint256 internal _bondCurveTrend;

constructor(uint256[] memory _bondCurve) {
_checkCurveLength(_bondCurve);
bondCurve = _bondCurve;
_setBondCurve(_bondCurve);
}

function _setBondCurve(uint256[] memory _bondCurve) internal {
_checkCurveLength(_bondCurve);
bondCurve = _bondCurve;
_bondCurveTrend =
_bondCurve[_bondCurve.length - 1] -
(_bondCurve.length > 1 ? _bondCurve[_bondCurve.length - 2] : 0);
}

function _setBondMultiplier(
Expand Down Expand Up @@ -74,14 +79,13 @@ abstract contract CSBondCurve {
uint256 amount
) internal view returns (uint256) {
uint256 mult = getBondMultiplier(nodeOperatorId);
uint256 first = (bondCurve[0] * mult) / BASIS_POINTS;
if (amount < first) return 0;
uint256 last = (bondCurve[bondCurve.length - 1] * mult) / BASIS_POINTS;
if (amount >= last) {
return
bondCurve.length +
((amount - last) /
(last -
(bondCurve[bondCurve.length - 2] * mult) /
BASIS_POINTS));
((amount - last) / ((_bondCurveTrend * mult) / BASIS_POINTS));
}
return _searchKeysByBond(amount, mult);
}
Expand All @@ -94,14 +98,18 @@ abstract contract CSBondCurve {
uint256 high = bondCurve.length - 1;
while (low <= high) {
uint256 mid = (low + high) / 2;
if (
(bondCurve[mid] * multiplier) / BASIS_POINTS > value && mid != 0
) {
uint256 midValue = (bondCurve[mid] * multiplier) / BASIS_POINTS;
if (value == midValue) {
return mid + 1;
}
if (value < midValue) {
// zero mid is avoided above
high = mid - 1;
} else if ((bondCurve[mid] * multiplier) / BASIS_POINTS <= value) {
continue;
}
if (value > midValue) {
low = mid + 1;
} else {
return mid;
continue;
}
}
return low;
Expand All @@ -117,18 +125,15 @@ abstract contract CSBondCurve {
uint256 nodeOperatorId,
uint256 keys
) internal view returns (uint256) {
uint256 mult = getBondMultiplier(nodeOperatorId);
if (keys == 0) return 0;
uint256 mult = getBondMultiplier(nodeOperatorId);
if (keys <= bondCurve.length) {
return (bondCurve[keys - 1] * mult) / BASIS_POINTS;
} else {
uint256 last = (bondCurve[bondCurve.length - 1] * mult) /
BASIS_POINTS;
return
last +
((bondCurve[bondCurve.length - 1] * mult) / BASIS_POINTS) +
(keys - bondCurve.length) *
(last -
((bondCurve[bondCurve.length - 2] * mult) / BASIS_POINTS));
((_bondCurveTrend * mult) / BASIS_POINTS);
}
}
}
25 changes: 22 additions & 3 deletions test/CSAccounting.blockedBond.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { CommunityStakingModuleMock } from "./helpers/mocks/CommunityStakingModu
import { CommunityStakingFeeDistributorMock } from "./helpers/mocks/CommunityStakingFeeDistributorMock.sol";
import { WithdrawalQueueMockBase, WithdrawalQueueMock } from "./helpers/mocks/WithdrawalQueueMock.sol";

import { Utilities } from "./helpers/Utilities.sol";
import { Fixtures } from "./helpers/Fixtures.sol";

contract CSAccounting_revealed is CSAccounting {
Expand Down Expand Up @@ -74,7 +75,12 @@ contract CSAccounting_revealed is CSAccounting {
}
}

contract CSAccounting_BlockedBondTest is Test, Fixtures, CSAccountingBase {
contract CSAccounting_BlockedBondTest is
Test,
Fixtures,
Utilities,
CSAccountingBase
{
using stdStorage for StdStorage;

LidoLocatorMock internal locator;
Expand Down Expand Up @@ -449,9 +455,15 @@ contract CSAccounting_BlockedBondTest is Test, Fixtures, CSAccountingBase {
_createNodeOperator({ ongoingVals: 1, withdrawnVals: 0 });

vm.expectRevert(
"AccessControl: account 0x7fa9385be102ac3eac297483dd6233d62b3e1496 is missing role 0xcc2e7ce7be452f766dd24d55d87a3d42901c31ffa5b600cd1dff475abec91c1f"
bytes(
Utilities.accessErrorString(
address(stranger),
accounting.EL_REWARDS_STEALING_PENALTY_INIT_ROLE()
)
)
);

vm.prank(stranger);
accounting.initELRewardsStealingPenalty({
nodeOperatorId: 0,
blockNumber: 100500,
Expand Down Expand Up @@ -671,8 +683,15 @@ contract CSAccounting_BlockedBondTest is Test, Fixtures, CSAccountingBase {
_createNodeOperator({ ongoingVals: 1, withdrawnVals: 0 });

vm.expectRevert(
"AccessControl: account 0x7fa9385be102ac3eac297483dd6233d62b3e1496 is missing role 0xcc2e7ce7be452f766dd24d55d87a3d42901c31ffa5b600cd1dff475abec91c1f"
bytes(
Utilities.accessErrorString(
address(stranger),
accounting.EL_REWARDS_STEALING_PENALTY_INIT_ROLE()
)
)
);

vm.prank(stranger);
accounting.releaseBlockedBondETH(0, 1 ether);
}

Expand Down
62 changes: 41 additions & 21 deletions test/CSAccounting.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { CommunityStakingModuleMock } from "./helpers/mocks/CommunityStakingModu
import { CommunityStakingFeeDistributorMock } from "./helpers/mocks/CommunityStakingFeeDistributorMock.sol";
import { WithdrawalQueueMockBase, WithdrawalQueueMock } from "./helpers/mocks/WithdrawalQueueMock.sol";

import { Utilities } from "./helpers/Utilities.sol";
import { Fixtures } from "./helpers/Fixtures.sol";

contract CSAccounting_revealed is CSAccounting {
Expand All @@ -39,7 +40,7 @@ contract CSAccounting_revealed is CSAccounting {
)
{}

function setBondCurve() public {
function setBondCurveForTests() public {
uint256[] memory _bondCurve = new uint256[](11);
_bondCurve[0] = 2 ether;
_bondCurve[1] = 3.90 ether; // 1.9
Expand All @@ -52,13 +53,14 @@ contract CSAccounting_revealed is CSAccounting {
_bondCurve[8] = 14.30 ether; // 1.2
_bondCurve[9] = 15.40 ether; // 1.1
_bondCurve[10] = 16.40 ether; // 1.0
bondCurve = _bondCurve;
_setBondCurve(_bondCurve);
}
}

contract CSAccountingTest is
Test,
Fixtures,
Utilities,
PermitTokenBase,
CSAccountingBase,
WithdrawalQueueMockBase
Expand Down Expand Up @@ -137,9 +139,15 @@ contract CSAccountingTest is
_bondCurve[1] = 4 ether;

vm.expectRevert(
"AccessControl: account 0x7fa9385be102ac3eac297483dd6233d62b3e1496 is missing role 0x645c9e6d2a86805cb5a28b1e4751c0dab493df7cf935070ce405489ba1a7bf72"
bytes(
Utilities.accessErrorString(
stranger,
accounting.SET_BOND_CURVE_ROLE()
)
)
);

vm.prank(stranger);
accounting.setBondCurve(_bondCurve);
}

Expand All @@ -152,9 +160,15 @@ contract CSAccountingTest is

function test_setBondMultiplier_RevertWhen_DoesNotHaveRole() public {
vm.expectRevert(
"AccessControl: account 0x7fa9385be102ac3eac297483dd6233d62b3e1496 is missing role 0x62131145aee19b18b85aa8ead52ba87f0efb6e61e249155edc68a2c24e8f79b5"
bytes(
Utilities.accessErrorString(
stranger,
accounting.SET_BOND_MULTIPLIER_ROLE()
)
)
);

vm.prank(stranger);
accounting.setBondMultiplier(0, 9500); // 0.95
}

Expand Down Expand Up @@ -717,7 +731,7 @@ contract CSAccountingTest is
assertEq(totalRewards, ETHAsFee);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

totalRewards = accounting.getTotalRewardsETH(
new bytes32[](1),
Expand Down Expand Up @@ -763,7 +777,7 @@ contract CSAccountingTest is
assertEq(totalRewards, stETHAsFee);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();
totalRewards = accounting.getTotalRewardsStETH(
new bytes32[](1),
0,
Expand Down Expand Up @@ -810,7 +824,7 @@ contract CSAccountingTest is
assertApproxEqAbs(totalRewards, wstETHAsFee, 1);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();
totalRewards = accounting.getTotalRewardsWstETH(
new bytes32[](1),
0,
Expand Down Expand Up @@ -850,7 +864,7 @@ contract CSAccountingTest is
assertApproxEqAbs(accounting.getExcessBondETH(0), 32 ether, 1);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(accounting.getExcessBondETH(0), 42.6 ether, 1);

Expand All @@ -874,7 +888,7 @@ contract CSAccountingTest is
assertApproxEqAbs(accounting.getExcessBondStETH(0), 32 ether, 1);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(accounting.getExcessBondStETH(0), 42.6 ether, 1);

Expand Down Expand Up @@ -902,7 +916,7 @@ contract CSAccountingTest is
);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(
accounting.getExcessBondWstETH(0),
Expand Down Expand Up @@ -930,7 +944,7 @@ contract CSAccountingTest is
assertApproxEqAbs(accounting.getMissingBondETH(0), 16 ether, 1);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(accounting.getMissingBondETH(0), 5.4 ether, 1);

Expand All @@ -954,7 +968,7 @@ contract CSAccountingTest is
assertApproxEqAbs(accounting.getMissingBondStETH(0), 16 ether, 1);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(accounting.getMissingBondStETH(0), 5.4 ether, 1);

Expand Down Expand Up @@ -982,7 +996,7 @@ contract CSAccountingTest is
);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertApproxEqAbs(
accounting.getMissingBondWstETH(0),
Expand Down Expand Up @@ -1015,7 +1029,7 @@ contract CSAccountingTest is
assertEq(accounting.getUnbondedKeysCount(0), 9);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertEq(accounting.getUnbondedKeysCount(0), 7);

Expand All @@ -1034,7 +1048,7 @@ contract CSAccountingTest is
assertEq(accounting.getKeysCountByBondETH(16 ether), 8);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertEq(accounting.getKeysCountByBondETH(16 ether), 10);
}
Expand All @@ -1047,7 +1061,7 @@ contract CSAccountingTest is
assertEq(accounting.getKeysCountByBondETH(16 ether), 8);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertEq(accounting.getKeysCountByBondStETH(16 ether), 10);
}
Expand Down Expand Up @@ -1080,7 +1094,7 @@ contract CSAccountingTest is
);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

assertEq(
accounting.getKeysCountByBondWstETH(
Expand Down Expand Up @@ -1135,7 +1149,7 @@ contract CSAccountingTest is
);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

vm.deal(address(feeDistributor), 0.1 ether);
vm.prank(address(feeDistributor));
Expand Down Expand Up @@ -1400,7 +1414,7 @@ contract CSAccountingTest is
);

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

vm.deal(address(feeDistributor), 0.1 ether);
vm.prank(address(feeDistributor));
Expand Down Expand Up @@ -1579,7 +1593,7 @@ contract CSAccountingTest is
assertEq(stETH.sharesOf(address(user)), 0, "user shares should be 0");

// set sophisticated curve
accounting.setBondCurve();
accounting.setBondCurveForTests();

vm.deal(address(feeDistributor), 0.1 ether);
vm.prank(address(feeDistributor));
Expand Down Expand Up @@ -1798,8 +1812,14 @@ contract CSAccountingTest is

function test_penalize_RevertWhenCallerHasNoRole() public {
vm.expectRevert(
"AccessControl: account 0x0000000000000000000000000000000000000309 is missing role 0x9909cf24c2d3bafa8c229558d86a1b726ba57c3ef6350848dcf434a4181b56c7"
bytes(
Utilities.accessErrorString(
stranger,
accounting.INSTANT_PENALIZE_BOND_ROLE()
)
)
);

vm.prank(stranger);
accounting.penalize(0, 20);
}
Expand Down
Loading

0 comments on commit 6c98090

Please sign in to comment.