🥝 Remove gas compensation #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
contracts/BorrowerOperations.sol
Outdated
| @@ -90,8 +90,7 @@ contract BorrowerOperations is TrinityBase, ReentrancyGuardUpgradeable, UUPSUpgr | |||
| _requireAtLeastMinNetDebt(vars.asset, vars.netDebt); | |||
|
|
|||
| // ICR is based on the composite debt, i.e. the requested debt token amount + borrowing fee + gas comp. | |||
| uint256 gasCompensation = IAdminContract(adminContract).getDebtTokenGasCompensation(vars.asset); | |||
| vars.compositeDebt = vars.netDebt + gasCompensation; | |||
| vars.compositeDebt = vars.netDebt; | |||
There was a problem hiding this comment.
Should we remove compositeDebt then?
Szymx95
force-pushed
the
Remove_gas_compensation
branch
2 times, most recently
from
dc134d7 to
5341d46
Compare
Szymx95
force-pushed
the
Remove_gas_compensation
branch
from
5341d46 to
742f56d
Compare
nezouse
reviewed
Apr 25, 2024
contracts/BorrowerOperations.sol
Outdated
| vars.compositeDebt = vars.netDebt + gasCompensation; | ||
| require(vars.compositeDebt != 0, "compositeDebt cannot be 0"); | ||
| // ICR is based on the composite debt, i.e. the requested debt token amount + borrowing fee. | ||
| require(vars.netDebt != 0, "compositeDebt cannot be 0"); |
There was a problem hiding this comment.
Suggested change
| require(vars.netDebt != 0, "compositeDebt cannot be 0"); | |
| require(vars.netDebt != 0, "Debt cannot be 0"); |
There was a problem hiding this comment.
In general should remove 'composite debt' from this file
| @@ -85,7 +83,6 @@ interface IVesselManagerOperations is ITrinityBase { | |||
| struct LiquidationValues { | |||
| uint256 entireVesselDebt; | |||
| uint256 entireVesselColl; | |||
| uint256 collGasCompensation; | |||
| uint256 debtTokenGasCompensation; | |||
There was a problem hiding this comment.
The other gas compensation is stil here?
contracts/VesselManager.sol
Outdated
| @@ -237,7 +237,7 @@ contract VesselManager is IVesselManager, UUPSUpgradeable, ReentrancyGuardUpgrad | |||
| ) external override nonReentrant onlyVesselManagerOperations { | |||
| _removeStake(_asset, _borrower); | |||
| _closeVessel(_asset, _borrower, Status.closedByRedemption); | |||
| _redeemCloseVessel(_asset, _borrower, IAdminContract(adminContract).getDebtTokenGasCompensation(_asset), _newColl); | |||
| _redeemCloseVessel(_asset, _borrower, 0, _newColl); | |||
There was a problem hiding this comment.
Is this param is unused maybe it should be removed from function signature?
| @@ -87,22 +87,16 @@ contract VesselManagerOperations is IVesselManagerOperations, UUPSUpgradeable, R | |||
| IActivePool(activePool).sendAsset(_asset, collSurplusPool, totals.totalCollSurplus); | |||
| } | |||
|
|
|||
| IVesselManager(vesselManager).updateSystemSnapshots_excludeCollRemainder(_asset, totals.totalCollGasCompensation); | |||
| IVesselManager(vesselManager).updateSystemSnapshots_excludeCollRemainder(_asset, 0); | |||
Comment on lines
98
to
99
| 0, | ||
| 0 |
Comment on lines
149
to
150
| 0, | ||
| 0 |
| @@ -144,22 +138,16 @@ contract VesselManagerOperations is IVesselManagerOperations, UUPSUpgradeable, R | |||
| } | |||
|
|
|||
| // Update system snapshots | |||
| IVesselManager(vesselManager).updateSystemSnapshots_excludeCollRemainder(_asset, totals.totalCollGasCompensation); | |||
| IVesselManager(vesselManager).updateSystemSnapshots_excludeCollRemainder(_asset, 0); | |||
MiksuJak
reviewed
Apr 25, 2024
MiksuJak
reviewed
Apr 25, 2024
test/utils/testHelpers.js
Outdated
| @@ -319,18 +316,13 @@ class TestHelper { | |||
| // Virtual debt = 50 TRI. | |||
There was a problem hiding this comment.
Shouldn't we just get rid of these functions? Or keep them to preserve the least amount of changes?
MiksuJak
reviewed
Apr 25, 2024
|
|
||
| // --- Vessel ordering by ICR tests --- | ||
|
|
||
| it("Vessel ordering: same collateral, decreasing debt. Price successively increases. Vessels should maintain ordering by ICR", async () => { |
There was a problem hiding this comment.
Shouldn't this be kept too, just adjust for no gas compensation?
MiksuJak
approved these changes
Apr 25, 2024
JustynaBroniszewska
approved these changes
Apr 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.