contracts: Add interfaces #80

bingen · 2024-03-11T11:17:08Z

First draft of interfaces changes for the frontend.
I skipped:

Anything related to redemptions, as they won’t be implemented in the frontend
Combined operations. Let’s do this the other way around: once we know which operations the frontend is adding, I’ll add them to the contracts.

danielattilasimon · 2024-03-12T02:45:47Z

Regarding combined operations: could we keep those outside the core contracts, i.e. in zapper contracts using "on behalf of" functionality? Just a thought.

Edit: I guess we should bake in at least a combined open Trove + delegate, as it could save some gas (to be seen).

danielattilasimon · 2024-03-12T09:37:23Z

contracts/src/Interfaces/IBorrowerOperations.sol


-    function claimCollateral() external;
+    function delegateInterest(uint256 _troveId, address _delegate) external;


How do you undelegate? Is the idea to reuse adjustTroveInterestRate() for that?

I was thinking of delegating to zero address (or to owner), but that’s an option too.

I thought of adjustTroveInterestRate() because I expect many people to also change their interest rate at the time of undelegation, so this would save an operation.

Upon further thought, there should probably be 2 different calls for interest rate delegation:

delegateInterestRateToIndividualManager(troveId, manager, min, max)

delegateInterestRateToBatchManager(troveId, batchManager)

The first one would grant the arbitrary address "manager" the right to call adjustTroveInterestRate() on the Trove with an interest rate within range of [min, max]. The owner of the Trove can undelegate by passing a zero address (or their own address) to the same function, like you suggested.

The second form is for batch delegation (duh) and requires batchManager address to have previously registered themselves as a batch manager, providing a [min, max] range (as well as a management fee) which can't be changed later. Undelegation in this case involves removing the Trove from the batch and reinserting it into a new position in the list, most likely even if the owner continues to use the last interest rate set by the batch manager (because batches must be kept contiguous). As such, we could either reuse adjustTroveInterestRate() for undelegation (and have it recognize when a Trove is part of a batch, and perform removal), or have a dedicated function that shares some of its internals with adjustTroveInterestRate(). I tend to prefer the latter as I dislike functions that branch too much, so I would add one more function:

undelegateInterestRateFromBatchManager(troveId, newAnnualInterestRate, upperHint, lowerHint)

Actually, we probably have to add hints to batch delegation too, as in:

delegateInterestRateToBatchManager(troveId, batchManager, upperHint, lowerHint)

Most of the time, hints won't be crucial as each batch will store "pointers" to both ends of its slice of the list for the purpose of quickly skipping over the entire batch, which can be used to find the correct insertion point on-chain. However, if a batch is empty, we don't know the correct position of constituents yet, so we need a hint.

Alternatively, we could put placeholder nodes in the list for each batch manager (or just empty batches) where new Troves are to be inserted, but this could be abused (spamming the list full of useless empty batches that make traversing the list costly, e.g. when redeeming). For that reason, I prefer hints.

Yes, your reasoning above make a lot of sense to me. Feel free to add a commit with those new function signatures so they don’t get lost in this comments.
Thanks!

danielattilasimon · 2024-03-12T09:46:05Z

contracts/src/Interfaces/IBorrowerOperations.sol

@@ -24,25 +24,29 @@ interface IBorrowerOperations is ILiquityBase {
        address _boldTokenAddress
    ) external;

-    function openTrove(uint _maxFee, uint256 _ETHAmount, uint _boldAmount, address _upperHint, address _lowerHint, uint256 _annualInterestRate) external;
+    function openTrove(address _owner, uint256 _ownerIndex, uint _maxFee, uint256 _ETHAmount, uint _boldAmount, address _upperHint, address _lowerHint, uint256 _annualInterestRate, uint256 _spDeposit) external;


I know combined operations will be coming later, but there's one that's worth considering already: opening a Trove with a delegated interest rate.

Definitely! Good idea.

danielattilasimon · 2024-03-12T09:49:29Z

contracts/src/Interfaces/IBorrowerOperations.sol


-    function adjustTrove(uint _maxFee, uint _collChange, bool _isCollIncrease, uint _debtChange, bool isDebtIncrease) external;
+    function adjustTrove(uint256 _troveId, uint _maxFee, uint _collChange, bool _isCollIncrease, uint _debtChange, bool isDebtIncrease) external;


Is _maxFee still needed? I know there's still an upfront interest payment (1 week's worth), but it's no longer unpredictable, like the borrow fee in v1 (unless you're delegating I guess, which is not the case for normal openTrove() / adjustTrove() / withdrawBold().

Good point. I’ll remove it.

danielattilasimon · 2024-03-12T09:53:47Z

contracts/src/Interfaces/IStabilityPool.sol

+    /* buyETHBatch():
+     * Swap ETH to Bold using opt-in swap facility liquidation gains, from multiple depositors
+     */
+    function buyETHBatch(address[] calldata _depositors, uint256 _ethAmount) external;


Haven't thought this through, but shouldn't _ethAmount also be an array (of how much to buy from each depositor)? Or is the idea to move on from one depositor to the next when its intent is fully used up?

Yes, my idea was the latter. I’m assuming you would order the array from most favorable to least.

danielattilasimon · 2024-03-12T10:17:24Z

Noticed there are no changes related to multi-collateral. I guess the idea is to have multiple instances of TroveManager/BorrowerOperations/StabilityPool etc., and redemption would be moved to its own contract as it interacts with every collateral type, right? Will each collateral have its own NFT contract for Troves, or do we want it to be shared?

bingen · 2024-03-12T21:04:56Z

Noticed there are no changes related to multi-collateral. I guess the idea is to have multiple instances of TroveManager/BorrowerOperations/StabilityPool etc., and redemption would be moved to its own contract as it interacts with every collateral type, right?

Yes, exactly.

Will each collateral have its own NFT contract for Troves, or do we want it to be shared?

Yes, that was my idea, to keep it simpler (in terms of backend code). If these NFTs were to be traded on OpenSea and the like, it would be confusing, but I don’t think that’s the case. We just care about transferFrom, and frontends can handle the different contracts. Does that sound good to you?

bpierre · 2024-03-12T19:44:06Z

contracts/src/Interfaces/IBorrowerOperations.sol


-    function addColl(uint256 _ETHAmount) external;
+    function addColl(uint256 _troveId, uint256 _ETHAmount) external;


Does _ETHAMount (and generally, the term ETH) represent WETH and LSTs in the contracts?

Yes, exactly. Technically it’s actually any ERC20 used as collateral, but in our case it will be WETH and some LSTs.

contracts: Add interfaces

9111c63

bingen requested review from bpierre, RickGriff and danielattilasimon March 11, 2024 11:17

bingen self-assigned this Mar 11, 2024

liquity deleted a comment from vercel bot Mar 11, 2024

danielattilasimon reviewed Mar 12, 2024

View reviewed changes

bpierre reviewed Mar 12, 2024

View reviewed changes

danielattilasimon deleted the branch ERC20_ETH April 12, 2024 06:15

danielattilasimon closed this Apr 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

contracts: Add interfaces #80

contracts: Add interfaces #80

bingen commented Mar 11, 2024

danielattilasimon commented Mar 12, 2024 •

edited

Loading

danielattilasimon Mar 12, 2024

bingen Mar 12, 2024

danielattilasimon Mar 13, 2024

danielattilasimon Mar 20, 2024 •

edited

Loading

danielattilasimon Mar 20, 2024 •

edited

Loading

bingen Mar 21, 2024

danielattilasimon Mar 12, 2024

bingen Mar 12, 2024

danielattilasimon Mar 12, 2024 •

edited

Loading

bingen Mar 12, 2024

danielattilasimon Mar 12, 2024

bingen Mar 12, 2024

danielattilasimon commented Mar 12, 2024 •

edited

Loading

bingen commented Mar 12, 2024

bpierre Mar 12, 2024

bingen Mar 21, 2024


		function claimCollateral() external;
		function delegateInterest(uint256 _troveId, address _delegate) external;


		function adjustTrove(uint _maxFee, uint _collChange, bool _isCollIncrease, uint _debtChange, bool isDebtIncrease) external;
		function adjustTrove(uint256 _troveId, uint _maxFee, uint _collChange, bool _isCollIncrease, uint _debtChange, bool isDebtIncrease) external;


		function addColl(uint256 _ETHAmount) external;
		function addColl(uint256 _troveId, uint256 _ETHAmount) external;

contracts: Add interfaces #80

contracts: Add interfaces #80

Conversation

bingen commented Mar 11, 2024

danielattilasimon commented Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielattilasimon Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

danielattilasimon Mar 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielattilasimon Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielattilasimon commented Mar 12, 2024 • edited Loading

bingen commented Mar 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielattilasimon commented Mar 12, 2024 •

edited

Loading

danielattilasimon Mar 20, 2024 •

edited

Loading

danielattilasimon Mar 20, 2024 •

edited

Loading

danielattilasimon Mar 12, 2024 •

edited

Loading

danielattilasimon commented Mar 12, 2024 •

edited

Loading