Skip to content

SortedTroves. insert is vulnerable to developer error #1031

@xavierlepretre

Description

@xavierlepretre

The function SortedTroves.insert takes in uint256 _NICR as a parameter. Further in the call stack, this value is compared to _troveManager.getNominalICR(_nextId) and _troveManager.getNominalICR(_prevId).

I understand it is the intention of the developer to have the insert function be called with a correct _NICR value.

However, I reckon the SortedTroves contract could protect itself form the risk that the developer does not follow the original intentions with this change:

-   _insert(troveManagerCached, _id, _NICR, _prevId, _nextId);
+   _insert(troveManagerCached, _id, troveManagerCached.getNominalICR(_id), _prevId, _nextId);

On the downside, it creates a couple extra SLOAD costs, mitigated by the fact that the storage locations are likely to have been written in the same transaction and so should cost WARM_STORAGE_READ_COST, plus, the rest of the function cold reads a lot of other storage locations.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions