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.

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

Fix Review #4

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Review #4

wants to merge 1 commit into from

Conversation

sherlock-admin4
Copy link

Fix Review of

Repo: sentimentxyz/protocol-v2
Commit Hash: 46519d5dfd66f827413ea06867aa5279f877f1d1

@@ -84,32 +84,31 @@ contract Deploy is BaseScript {
// risk
riskEngine = new RiskEngine(address(registry), params.minLtv, params.maxLtv);
riskEngine.transferOwnership(params.owner);
riskModule = new RiskModule(address(registry), params.liquidationDiscount);
riskModule = new RiskModule(address(registry), params.liquidationDiscount, params.liquidationFee);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#339.

Fixes issues: #91, #309

address(registry),
params.feeRecipient,
params.minDebt,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +97 to +98
params.defaultInterestFee,
params.defaultOriginationFee
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +105 to +106
bytes memory posmgrInitData =
abi.encodeWithSelector(PositionManager.initialize.selector, params.owner, address(registry));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#339.

Fixes issues: #91, #309

positionManager = PositionManager(
address(new TransparentUpgradeableProxy(positionManagerImpl, params.proxyAdmin, posmgrInitData))
);
// position
address positionImpl = address(new Position(address(pool), address(positionManager)));
address positionImpl = address(new Position(address(pool), address(positionManager), address(riskEngine)));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#337.

Fixes issues: #382

Comment on lines +6 to +7

import { IERC20 } from "forge-std/interfaces/IERC20.sol";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

import { Pool } from "src/Pool.sol";

contract InitializePool is BaseScript {
address pool;

address owner;
address asset;
bytes32 rateModelKey;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

Comment on lines +15 to +16
uint256 borrowCap;
uint256 depositCap;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

uint128 poolCap;
uint256 borrowCap;
uint256 depositCap;
uint256 initialDepositAmt;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

vm.broadcast(vm.envUint("PRIVATE_KEY"));
uint256 poolId = Pool(pool).initializePool(owner, asset, poolCap, rateModelKey);
IERC20(asset).approve(pool, initialDepositAmt);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

vm.broadcast(vm.envUint("PRIVATE_KEY"));
uint256 poolId = Pool(pool).initializePool(owner, asset, poolCap, rateModelKey);
IERC20(asset).approve(pool, initialDepositAmt);
uint256 poolId = Pool(pool).initializePool(owner, asset, rateModelKey, depositCap, borrowCap, initialDepositAmt);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +33 to +34
borrowCap = vm.parseJsonUint(config, "$.InitializePool.borrowCap");
depositCap = vm.parseJsonUint(config, "$.InitializePool.depositCap");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

poolCap = uint128(vm.parseJsonUint(config, "$.InitializePool.poolCap"));
borrowCap = vm.parseJsonUint(config, "$.InitializePool.borrowCap");
depositCap = vm.parseJsonUint(config, "$.InitializePool.depositCap");
initialDepositAmt = (vm.parseJsonUint(config, "$.InitializePool.initialDepositAmt"));
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

@@ -19,13 +15,21 @@ import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
// contracts
import { ERC6909 } from "./lib/ERC6909.sol";
import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#327.

Fixes issues: #9, #557


/// @title Pool
/// @notice Singleton pool for all pools that superpools lend to and positions borrow from
contract Pool is OwnableUpgradeable, ERC6909 {
contract Pool is OwnableUpgradeable, PausableUpgradeable, ERC6909 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#327.

Fixes issues: #9, #557

using Math for uint256;
using SafeERC20 for IERC20;

address private constant DEAD_ADDRESS = 0x000000000000000000000000000000000000dEaD;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

Comment on lines +29 to +30
/// @notice Maximum amount of borrow shares per base pool
uint256 public constant MAX_BORROW_SHARES = type(uint112).max;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +31 to +32
/// @notice Minimum amount of initial shares to be burned
uint256 public constant MIN_BURNED_SHARES = 1_000_000;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

@@ -40,9 +44,9 @@ contract Pool is OwnableUpgradeable, ERC6909 {
0x5b6696788621a5d6b5e3b02a69896b9dd824ebf1631584f038a393c29b6d7555;

/// @notice Initial interest fee for pools
uint128 public defaultInterestFee;
uint256 public defaultInterestFee;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

/// @notice Initial origination fee for pools
uint128 public defaultOriginationFee;
uint256 public defaultOriginationFee;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +76 to +80
uint256 borrowCap;
uint256 depositCap;
uint256 lastUpdated;
uint256 interestFee;
uint256 originationFee;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +110 to +111
/// @notice Pool fee recipient set
event FeeRecipientSet(address feeRecipient);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#322.

Fixes issues: #571

Comment on lines +115 to +117
event PoolCapSet(uint256 indexed poolId, uint256 poolCap);
/// @notice Borrow debt ceiling for a pool was set
event BorrowCapSet(uint256 indexed poolId, uint256 borrowCap);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

/// @notice Owner for a pool was set
event PoolOwnerSet(uint256 indexed poolId, address owner);
/// @notice Rate model for a pool was updated
event RateModelUpdated(uint256 indexed poolId, address rateModel);
/// @notice Interest fee for a pool was updated
event InterestFeeSet(uint256 indexed poolId, uint128 interestFee);
event InterestFeeSet(uint256 indexed poolId, uint256 interestFee);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

/// @notice Origination fee for a pool was updated
event OriginationFeeSet(uint256 indexed poolId, uint128 originationFee);
event OriginationFeeSet(uint256 indexed poolId, uint256 originationFee);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

Comment on lines +155 to +158
/// @notice Total borrow shares exceed MAX_BORROW_SHARES
error Pool_MaxBorrowShares(uint256 poolId);
/// @notice Pool borrow cap exceeded
error Pool_BorrowCapExceeded(uint256 poolId);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

@@ -171,6 +190,12 @@ contract Pool is OwnableUpgradeable, ERC6909 {
error Pool_DebtTooLow(uint256 poolId, address asset, uint256 amt);
/// @notice No oracle found for pool asset
error Pool_OracleNotFound(address asset);
/// @notice Fee recipient must be non-zero
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#322.

Fixes issues: #571

Comment on lines +195 to +196
/// @notice Pool has zero assets and non-zero shares
error Pool_ZeroAssetsNonZeroShares(uint256 poolId);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#332.

Fixes issues: #319, #584

Comment on lines +197 to +198
/// @notice Less than MIN_BURNED_SHARES burned during pool initialization
error Pool_MinBurnedShares(uint256 shares);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#335.

Fixes issues: #400, #597

address registry_,
address feeRecipient_,
uint256 minDebt_,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change related to sentimentxyz/protocol-v2#336.

Fixes issues: #585

@sherlock-admin sherlock-admin force-pushed the fix-review branch 28 times, most recently from efc5d4f to 7ecf9ef Compare November 15, 2024 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants