Two Part Transactions #10
Conversation
contracts/oracle/FastPriceFeed.sol
Outdated
| @@ -126,6 +132,13 @@ contract FastPriceFeed is ISecondaryPriceFeed, Governable { | |||
| tokenPrecisions = _tokenPrecisions; | |||
| } | |||
|
|
|||
| // do not call setLastUpdatedAt, if the fast price submitter is not updating then | |||
| // the Chainlink price should be used instead since this function does not update all token prices | |||
| function setPrice(address _token, uint256 _price) external override onlyAdmin { | |||
There was a problem hiding this comment.
what's the usecase for this? without setLastUpdatedAt new price won't be used after some time threshold anyway
There was a problem hiding this comment.
was meant for the keeper / signer flow, will remove it
contracts/oracle/FastPriceFeed.sol
Outdated
| @@ -61,7 +64,7 @@ contract FastPriceFeed is ISecondaryPriceFeed, Governable { | |||
| } | |||
|
|
|||
| modifier onlyAdmin() { | |||
| require(msg.sender == admin, "FastPriceFeed: forbidden"); | |||
| require(isAdmin[msg.sender], "FastPriceFeed: forbidden"); | |||
There was a problem hiding this comment.
if it won't bother you maybe change "admin" to something like oracle/updater/etc.? since this role is only used to update price. but admin sounds like it can do everything here
There was a problem hiding this comment.
ok i'll call it updater
contracts/oracle/FastPriceFeed.sol
Outdated
| if (index >= tokens.length) { return; } | ||
|
|
||
| uint256 startBit = 32 * j; | ||
| uint256 price = (_priceBits >> startBit) & PRICE_BITMASK; |
There was a problem hiding this comment.
we may add some sanity checks here as well
contracts/core/PositionManager.sol
Outdated
| address[] memory _path, | ||
| function withdrawFees(address _token, address _receiver) external onlyGov { | ||
| uint256 amount = feeReserves[_token]; | ||
| if(amount == 0) { return; } |
contracts/core/PositionManager.sol
Outdated
| uint256 public minBlockDelayKeeper; | ||
| uint256 public minBlockDelayPublic; | ||
|
|
||
| uint256 public lastCreationBlock; |
There was a problem hiding this comment.
this var is not used (only updated in create position func), do we need it?
There was a problem hiding this comment.
it will be checked by the fast price feed to schedule an update, i've added a comment
contracts/core/PositionManager.sol
Outdated
| bool shouldRefundCollateral = false; | ||
|
|
||
| if (position.isLong) { | ||
| shouldRefundCollateral = IVault(_vault).getMaxPrice(position.indexToken) > position.acceptablePrice; |
There was a problem hiding this comment.
in this case position won't be open and collateral will be returned to a user?
probably it's not shoudRefundCollateral but more like shouldOpenPosition? since it's just two different branches – open position or cancel it
contracts/core/PositionManager.sol
Outdated
| } | ||
|
|
||
| function executeIncreasePositionETH() external nonReentrant onlyPositionKeeper { |
There was a problem hiding this comment.
mmm i guess executeIncreasePosition should handle both cases
contracts/core/PositionManager.sol
Outdated
| uint256 _sizeDelta, | ||
| bool _isLong, | ||
| uint256 _price | ||
| ) external nonReentrant onlyPartners { |
There was a problem hiding this comment.
in case we will implement automatic position opening with e.g. each oracle update we can create IncreaseOrder here as well
that might be more secure
There was a problem hiding this comment.
for partners they might want it to be opened immediately, otherwise they should just call createIncreasePosition
| emit Register(msg.sender, _code); | ||
| } | ||
|
|
||
| function updateAddress(bytes32 _code, address _newAccount) external { |
There was a problem hiding this comment.
it's not clear wether it's about referrer or owner
maybe updateReferralCodeOwner(_code, _newOwner)?
There was a problem hiding this comment.
hm. i guess it should be merged with setReferralCodeOwner
| emit UpdateAddress(msg.sender, _newAccount, _code); | ||
| } | ||
|
|
||
| function getReferral(address _account) external override view returns (bytes32, address) { |
There was a problem hiding this comment.
"referral" is a trader, who used code and "referrer" is the code owner?
better to name it getReferrer?
contracts/core/RouterV2.sol
Outdated
| return keccak256(abi.encodePacked(_account, _index)); | ||
| } | ||
|
|
||
| function getPendingRequestLenghts() public view returns (uint256, uint256) { |
There was a problem hiding this comment.
i guess it should be "external" instead of "public"
contracts/core/RouterV2.sol
Outdated
| IncreasePositionRequest memory request = increasePositionRequests[_key]; | ||
| require(request.account != address(0), "RouterV2: request does not exist"); | ||
|
|
||
| bool shouldExecute = _validateCancellation(request.blockNumber, request.blockTime); |
There was a problem hiding this comment.
shouldExecute -> shouldCancel
| uint256 nextCollateral = collateral.add(collateralDelta); | ||
|
|
||
| uint256 prevLeverage = size.mul(BASIS_POINTS_DIVISOR).div(collateral); | ||
| uint256 nextLeverage = nextSize.mul(BASIS_POINTS_DIVISOR + 1).div(nextCollateral); |
There was a problem hiding this comment.
we should add 100 here (same logic is in validateIncreaseOrder)
contracts/core/RouterV2.sol
Outdated
| if (isKeeperCall) { | ||
| return _positionBlockNumber.add(minBlockDelayKeeper) <= block.number; | ||
| } | ||
|
|
There was a problem hiding this comment.
should we validate that msg.sender == request.account?
There was a problem hiding this comment.
theoretical case:
- keeper doesn't work
- much time has passed and trader prefers to cancel request
- some bot will execute it faster to get fee
No description provided.