Skip to content

chore: reorder rp update on deficit #1078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yan-man merged 22 commits into from
Feb 7, 2026
+19 −16
Merged

chore: reorder rp update on deficit #1078

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
32fc97f
chore: reorder rp update on deficit
DhairyaSethi Dec 23, 2025
b2e00ab
Merge remote-tracking branch 'origin/dev' into chore/deficit-reorder
DhairyaSethi Jan 14, 2026
895d7b2
Merge remote-tracking branch 'origin/dev' into chore/deficit-reorder
DhairyaSethi Jan 16, 2026
0c1bac0
chore: snapshot
DhairyaSethi Jan 16, 2026
415bf3b
Merge remote-tracking branch 'origin/dev' into chore/deficit-reorder
DhairyaSethi Jan 16, 2026
29d6c4f
chore: merge in dev
DhairyaSethi Jan 20, 2026
90ecea1
chore: merge in dev
DhairyaSethi Jan 20, 2026
511ac7d
chore: merge in dev
DhairyaSethi Jan 21, 2026
a46478b
chore: merge dev
DhairyaSethi Jan 23, 2026
72a4c14
chore: merge in dev, rename
DhairyaSethi Jan 30, 2026
b1425a0
chore: merge in dev
DhairyaSethi Jan 30, 2026
3f6f396
chore: merge in dev
DhairyaSethi Jan 30, 2026
1f0a705
chore: merge in dev
DhairyaSethi Feb 2, 2026
5bb8ded
Merge remote-tracking branch 'origin/dev' into chore/deficit-reorder
DhairyaSethi Feb 3, 2026
9c7abd7
chore: add linebreak
DhairyaSethi Feb 3, 2026
9dac5fc
chore: merge in dev
DhairyaSethi Feb 3, 2026
eec574e
chore: finalise merge
DhairyaSethi Feb 3, 2026
99213f7
chore: rename
DhairyaSethi Feb 3, 2026
362f535
chore: merge in dev
DhairyaSethi Feb 5, 2026
2fb5e97
chore: merge in dev
DhairyaSethi Feb 6, 2026
bd60b7b
chore: merge in dev
DhairyaSethi Feb 6, 2026
dc33d4f
docs: natspec fix
miguelmtzinf Feb 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions snapshots/Spoke.Operations.ZeroRiskPremium.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"borrow: first": "190384",
"borrow: second action, same reserve": "170250",
"liquidationCall (receiveShares): full": "303261",
"liquidationCall (receiveShares): partial": "302679",
"liquidationCall (reportDeficit): full": "366383",
"liquidationCall: full": "320897",
"liquidationCall: partial": "320315",
"liquidationCall (receiveShares): full": "303257",
"liquidationCall (receiveShares): partial": "302675",
"liquidationCall (reportDeficit): full": "367686",
"liquidationCall: full": "320893",
"liquidationCall: partial": "320311",
"permitReserve + repay (multicall)": "164576",
"permitReserve + supply (multicall)": "146756",
"permitReserve + supply + enable collateral (multicall)": "161207",
Expand Down
10 changes: 5 additions & 5 deletions snapshots/Spoke.Operations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
{
"borrow: first": "259319",
"borrow: second action, same reserve": "202185",
"liquidationCall (receiveShares): full": "335305",
"liquidationCall (receiveShares): partial": "334723",
"liquidationCall (reportDeficit): full": "361000",
"liquidationCall: full": "352941",
"liquidationCall: partial": "352359",
"liquidationCall (receiveShares): full": "335301",
"liquidationCall (receiveShares): partial": "334719",
"liquidationCall (reportDeficit): full": "359886",
"liquidationCall: full": "352937",
"liquidationCall: partial": "352355",
"permitReserve + repay (multicall)": "162044",
"permitReserve + supply (multicall)": "146756",
"permitReserve + supply + enable collateral (multicall)": "161207",
Expand Down
8 changes: 4 additions & 4 deletions src/spoke/Spoke.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,20 +373,19 @@ abstract contract Spoke is
params: params
});

uint256 newRiskPremium = 0;
if (isUserInDeficit) {
// report deficit for all debt reserves, including the reserve being repaid
LiquidationLogic.reportDeficit(
LiquidationLogic.notifyReportDeficit(
_reserves,
_userPositions,
_positionStatus,
_reserveCount,
user
);
} else {
newRiskPremium = _calculateUserAccountData(user).riskPremium;
uint256 newRiskPremium = _calculateUserAccountData(user).riskPremium;
_notifyRiskPremiumUpdate(user, newRiskPremium);
}
_notifyRiskPremiumUpdate(user, newRiskPremium);
Copy link
Member

@miguelmtzinf miguelmtzinf Jan 19, 2026

Choose a reason for hiding this comment

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

here the question, when reading this fn you can assume either
i. _reportDeficit only clears user debt and notify so we must _notifyRiskPremiumUpdate at the end no matter what (dev)
ii. _reportDeficit also clears risk premium so we don't need to notify later. (PR)

imo:
i. option follow single responsability principle, and makes liq fn more readable
ii. option is more self-contained and favors reusability. It's only used once and worsen liq fn readabiltiy

Copy link
Member Author

@DhairyaSethi DhairyaSethi Jan 19, 2026

Choose a reason for hiding this comment

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

single responsibility is readable but more error prone imo, since the true action is contained in two methods

i think renaming to notifyReportDefict or reportAndNotifyDeficit is a great suggestion to improve the lost readability

}

/// @inheritdoc ISpoke
Expand Down Expand Up @@ -846,6 +845,7 @@ abstract contract Spoke is
userPosition.applyPremiumDelta(premiumDelta);
emit RefreshPremiumDebt(reserveId, user, premiumDelta);
}

emit UpdateUserRiskPremium(user, newRiskPremium);
}

Expand Down
5 changes: 4 additions & 1 deletion src/spoke/libraries/LiquidationLogic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,15 @@ library LiquidationLogic {
/// @param positionStatus The mapping of position status per user.
/// @param reserveCount The number of reserves.
/// @param user The address of the user.
function reportDeficit(
function notifyReportDeficit(
mapping(uint256 reserveId => ISpoke.Reserve) storage reserves,
mapping(address user => mapping(uint256 reserveId => ISpoke.UserPosition)) storage userPositions,
mapping(address user => ISpoke.PositionStatus) storage positionStatus,
uint256 reserveCount,
address user
) external {
ISpoke.PositionStatus storage userPositionStatus = positionStatus[user];
userPositionStatus.riskPremium = 0;

uint256 reserveId = reserveCount;
while (
Expand Down Expand Up @@ -298,6 +299,8 @@ library LiquidationLogic {

emit ISpoke.ReportDeficit(reserveId, user, debtComponents.drawnShares, premiumDelta);
}

emit ISpoke.UpdateUserRiskPremium(user, 0);
Copy link
Member

@miguelmtzinf miguelmtzinf Feb 5, 2026

Choose a reason for hiding this comment

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

this is being emitted even if the risk premium was already 0. We d need to do smt like to avoid emitting false events...

    if (newRiskPremium == 0 && positionStatus.riskPremium == 0) {
      return;
    }

I d keep the code as is OR create an internal helper for updating user risk premium that i) returns early if not needed, ii) update storage and iii) emit event.

Copy link
Member Author

@DhairyaSethi DhairyaSethi Feb 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

its just a redundant event, idt it's 'false'. to me its fine as is

internal helper will make the logic of _notifyRiskPremium more expensive

Copy link
Member Author

@DhairyaSethi DhairyaSethi Feb 5, 2026

Choose a reason for hiding this comment

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

see https://github.com/aave/aave-v4/compare/chore/deficit-reorder...poc/setuserrp?expand=1 wdyt?

Copy link
Member

@miguelmtzinf miguelmtzinf Feb 6, 2026

Choose a reason for hiding this comment

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

why are you fine with the redudant event in this case? it d make the logic of _reportDeficit more expensive no? the _notifyRiskPremium wouldn't be much affected.
the suggestion does not work for the same reason that calling _notifyRiskPremiumUpdate but not actually calling hub.refresh is misleading (one of the motivations for this PR).

Copy link
Member Author

@DhairyaSethi DhairyaSethi Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

it adds additional comparator which is more expensive in the general case, the redundant event is only redundant in one case when prev rp is 0; im fine here because next rp bc of report deficit is always 0
(unlike notify risk premium)

the logic is simpler w/o suggestion but i'm happy to apply

Copy link
Member

@miguelmtzinf miguelmtzinf Feb 7, 2026

Choose a reason for hiding this comment

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

We agreed to accept the redundancy, as it makes deficit scenarios more explicit when the system fully clears a user position, without impacting gas efficiency in the average case.

}

/// @notice Calculates the liquidation bonus at a given health factor.
Expand Down
Loading