Skip to content

Blur Integration - #371

Open
zhoujia6139 wants to merge 42 commits into
mainfrom
blur_integration
Open

Blur Integration#371
zhoujia6139 wants to merge 42 commits into
mainfrom
blur_integration

Conversation

@zhoujia6139

Copy link
Copy Markdown
Contributor

Security Checklist

  • 1. Re-Entrancy
  • 2. Arithmetic Over/Under Flows
  • 3. Unexpected Ether
  • 4. Delegatecall
  • 5. Default Visibilities
  • 6. Entropy Illusion
  • 7. External Contract Referencing
  • 8. Short Address/Parameter Attack (off chain)
  • 9. Unchecked CALL Return Values
  • 10. Race Conditions / Front Running
  • 11. Denial Of Service (DOS)
  • 12. Block Timestamp Manipulation
  • 13. Constructors with Care
  • 14. Uninitialized Storage Pointers
  • 15. Floating Points and Precision
  • 16. Tx.Origin Authentication
  • 17. Address.isContract Re-Entrancy via Constructor

⚠️ NOTES ⚠️

Make sure to think about each of these exploits in this PR.

@zhoujia6139
zhoujia6139 requested a review from a team as a code owner April 15, 2023 03:56
@zhoujia6139
zhoujia6139 marked this pull request as draft April 15, 2023 03:57
@zhoujia6139
zhoujia6139 marked this pull request as ready for review April 17, 2023 13:33
Comment thread contracts/protocol/libraries/logic/MarketplaceLogic.sol Outdated
}

/// @inheritdoc IPoolParameters
function enableBlurExchange() external onlyPoolAdmin {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest using different names for these. as we are not really enabling the actual exchange

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

enableBlurIntegration?

request.borrowAmount,
timeLockParams
);
IWETH(weth).withdraw(request.borrowAmount);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can't we transfer weth directly? might save us one transfer

Comment thread contracts/protocol/libraries/logic/PoolExtendedLogic.sol Outdated
Comment thread contracts/protocol/libraries/logic/PoolExtendedLogic.sol Outdated
Comment thread contracts/protocol/libraries/logic/PoolExtendedLogic.sol Outdated
Comment thread contracts/protocol/libraries/logic/PoolExtendedLogic.sol Outdated
// currency token. address(0) means ETH
address paymentToken;
// cash amount from user wallet
uint256 listingPrice;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is cash amount ? or listing price?

Errors.INVALID_REQUEST_STATUS
);

address keeper = ps._blurExchangeKeeper;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

could we not to check “msg.sender == keeper” to save some gas.

address nTokenAddress = nftReserve.xTokenAddress;
// no time lock needed here
DataTypes.TimeLockParams memory timeLockParams;
(, uint64 collateralizedBalance) = INToken(nTokenAddress).burn(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if the nToken is in auction , will revert.

.getParams();
// ensure user can't borrow/withdraw with the new mint nToken
require(
request.listingPrice >= floorPrice.percentMul(liquidationThreshold),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. user init buy from blur which have high boost token id (set require.listingprice = nToken floor price * LS)
  2. user get more borrow limit : nToken floor price * boost *LTV - nToken floor price * LS
  3. user can borrow more money.

Comment thread contracts/protocol/libraries/logic/ValidationLogic.sol

//mint debt token
if (totalBorrow > 0) {
BorrowLogic.executeBorrow(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should we do transferUnderlyingTo before executeBorrow ? since the borrow use rate is not correct in updateInterestRates now.
borrow use rate = total debt / (balance (pToken contract) + total debt)
balance (pToken contract) should subtract the totalBorrow

request.tokenId
);

return request.listingPrice + requestFee - request.borrowAmount;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we save some gas if calculate needCashETH first and sent into validateInitiateBlurExchangeRequest ?

Comment thread contracts/protocol/libraries/logic/ValidationLogic.sol
* chore: basic implementation for BLUR sell

* chore: fix typo

* chore: check ape pair staking and gas optimization

* chore: check owner and fix interest rate issue

* chore: keeper fulfill request with ETH

* chore: fix updating wrong status

* chore: forbid initiate request for uniswapV3 and stakeFish
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.

6 participants