House Protocol Looping Feature - #15
Conversation
c1c6e63 to
16a6463
Compare
leeftk
left a comment
leeftk
left a comment
There was a problem hiding this comment.
Great job with this, though I think there may be a bug in the looping feature. Let's discuss more but overall this is on the right path.
|
|
||
| // Calculate how much can be borrowed in this iteration | ||
| // Each iteration can borrow up to the value of the issuance tokens received | ||
| uint borrowAmountThisIteration = borrowingPower; |
There was a problem hiding this comment.
there's a few points where we are setting these variables to each other and imo its kind of redundant, why not just use borrowingPower instead? i believe this exists in other places as well in this function
There was a problem hiding this comment.
Fixed it for collateralForPurchase and borrowAmountThisIteration
| } | ||
|
|
||
| // Track total issuance tokens received and total borrowed | ||
| uint totalIssuanceTokensReceived = 0; |
There was a problem hiding this comment.
setting these to zero wastes gas really cause they're set to default in solidity
| uint totalCollateralUsed = 0; | ||
|
|
||
| // Loop through leverage iterations | ||
| for (uint8 i = 0; i < leverage_; i++) { |
There was a problem hiding this comment.
since this operation is expensive we should optmized this loop, lets store the value of leverage here, also should this be less than or equal to? not sure but lets verify
There was a problem hiding this comment.
Since the value of leverage_ is input param and not being read from storage each time, will adding a new variable save gas?
The loops work correctly, say the leverage is set to 5, the loop starts from 0 and ends at 4.
|
|
||
| // Get the actual amount of issuance tokens received in this iteration | ||
| uint actualIssuanceTokensReceived = | ||
| _issuanceToken.balanceOf(user) - totalIssuanceTokensReceived; |
There was a problem hiding this comment.
Im not sure this is right? This is assuming the the users balance of issuanceTokens would increase after each iteration however when after each loop these tokens should got locked meaning the users issuance token balance actually should be zero after each iteration, what should increase is their locked Issuance token amount.
Iteration 1:
User starts with 0 issuance tokens
buyFor() gives user 100 issuance tokens
actualIssuanceTokensReceived = 100 - 0 = 100
borrow() locks those 100 tokens
User's balance is now 0, but totalIssuanceTokensReceived = 100
Iteration 2:
User has 0 issuance tokens (they're locked)
buyFor() gives user 50 more issuance tokens
actualIssuanceTokensReceived = 50 - 100 = -50 ❌
| │ └── And the user should have an outstanding loan | ||
| */ | ||
| function testPublicBuyAndBorrow_succeedsGivenValidLeverage() public { | ||
| function testFuzzPublicBuyAndBorrow_succeedsGivenValidLeverage( |
There was a problem hiding this comment.
These are great but we should probably add more tests here.
2 participants
buyAndBorrow Function Summary
The
buyAndBorrowfunction is a newly added leveraged trading feature that allows users to buy issuance tokens and borrow against them in a single transaction. Users specify a leverage multiplier (1 to maxLeverage), and the function iteratively uses their collateral to purchase issuance tokens from the bonding curve, then automatically borrows against those locked tokens. This creates an efficient way to amplify both token acquisition and borrowing power simultaneously.Key Features:
Configuration & Security:
maxLeveragestate variable (configurable by authorized managers)BuyAndBorrowCompletedevent for transparency