Owner Address Discussion

[dapp-whisperer]

When we _adjustCdpInternal, we check ownership two ways

• via sortedCdps.existCdpOwners
• via the borrower address, which is extracted from the cdpId

function _adjustCdpInternal(
        bytes32 _cdpId,
        uint _stEthBalanceToDecrease,
        uint _debtChange,
        bool _isDebtIncrease,
        bytes32 _upperHint,
        bytes32 _lowerHint,
        uint _stEthBalanceToIncrease
    ) internal {
        _requireCdpOwner(_cdpId); // <- one owner check
        _requireCdpisActive(_cdpId);

        address _borrower = sortedCdps.getOwnerAddress(_cdpId);
        require(msg.sender == _borrower, "BorrowerOperations: only allow CDP owner to adjust!");  // <- second owner check

function _requireCdpOwner(bytes32 _cdpId) internal view {
        address _owner = sortedCdps.existCdpOwners(_cdpId);
        require(msg.sender == _owner, "BorrowerOperations: Caller must be cdp owner");
    }

function existCdpOwners(bytes32 cdpId) public view override returns (address) {
        return cdpOwners[cdpId];
    }

both of the checks require msg.sender == the owner or borrower, respectively. For them both to pass, they must be the same address.

Questions

Can the borrower address ever not be the top 20 bytes of the cdpId?

If this fails, could be a DDOS vector
Do we need the other check?

Do we need all these owner data structures in general?

You'll also see the _requireCdpisActive check, can we revisit and potentially remove status? can we pack it with other variables?

CdpId

function toCdpId(
        address owner,
        uint256 blockHeight,
        uint256 nonce
    ) public pure returns (bytes32) {
        bytes32 serialized;

        serialized |= bytes32(nonce);
        serialized |= bytes32(blockHeight) << BLOCK_SHIFT; // to accommendate more than 4.2 billion blocks
        serialized |= bytes32(uint256(uint160(owner))) << ADDRESS_SHIFT;

        return serialized;
    }

The CdpId comes from three values: an ever-incrementing nonce that overflows back to zero at uin256.max.
However, there are only 6 slots for this value.
The blockHeight (is this really necessary, given the nonce?)
Then the owner address

Between these three we should have a unique address for which the creation mechanics stand the test of time.

New property: borrower address, top 20 bytes of CdpId, must always be msg.sender of opening tx

We don't and never will support meta-transactions with this design.

The Ownership Storage Stack

How is CDP ownership tracked?

• address in CdpId
• everything in here:

function _addCdpToOwnerEnumeration(address to, bytes32 cdpId) private {
        uint256 length = _ownedCount[to];
        _ownedCdps[to][length] = cdpId;
        _ownedCdpIndex[cdpId] = length;
        _ownedCount[to] = length + 1;
    }

so we have:

• ownedCount of given owner
• a list of owned CdpIds by this owner, _ownedCdps
• a mapping describing the index of each CDP within this _ownedCdps list
• You also have the owners Index array in CdpManager

So for each user, we know:

• how many CDPs they have open ("active")
• an ordering of these CDPs by creation date within the. This is how we get them (get this user's first CDP, etc)

Where do we use each of these values?

CDPs open

Reading a CDP by index

Getting CDPs opened by a given owner

Can we put ordering within the CDPId?
yes, blockheight + nonce can do it in practice.

• earlier blockheight = earlier
• same blockheight + smaller nonce = earlier

because we never need to read these values on-chain, we don't need to store them if we can collect them accurately by events. However, events aren't guarenteed so there should be a way to read on chain.

WIthout events, We can always find all the owner's CDPs by iterating over all the CDPs that exist and finding where the address in the cdpId matches.

The problem with iteration is exceeding the capacity of the node to read all within a single function. So, we need pagination.

sortedCdps.cdpOfOwnerByIndex(address owner, uint index)
sortedCdps.cdpOfOwnerByIndex(address owner, uint index, uint startIndex, uint numToRead)

sortedCdps.getCdpsOf(address owner)
sortedCdps.getCdpsOf(address owner, uint startIndex, uint numToRead)

sortedCdps.cdpCountOf(address owner)
sortedCdps.cdpCountOf(address owner, uint startIndex, uint numToRead)

Active CDPs can be found according to their place in the list. Others actually cannot be read by the existing methods either.

API Usage

• cdpCountOf - ever used on-chain?
  These would all ultimately be walking through all CdpIds that are in the list or owners array

• HintHelpers.sol <- is it used?

Cdp Lifecycle

Opening

The operation starts in BorrowerOperations

SortedCdps

First we generate the cdpId and insert it into the list, initializing all owner variables as well

bytes32 _cdpId = sortedCdps.insert(_borrower, vars.NICR, _upperHint, _lowerHint);

we create a node for the cdpId, and find the correct place for the node in the list

New property idea: If the hints supplied are wrong, what happens?

// Information for a node in the list
    struct Node {
        bytes32 nextId; // Id of next node (smaller NICR) in the list
        bytes32 prevId; // Id of previous node (larger NICR) in the list
    }

    // Information for the list
    struct Data {
        bytes32 head; // Head of the list. Also the node in the list with the largest NICR
        bytes32 tail; // Tail of the list. Also the node in the list with the smallest NICR
        uint256 size; // Current size of the list
        mapping(bytes32 => Node) nodes; // Track the corresponding ids for each node in the list
    }

(we end up in the Node mapping, increase size, and may rarely change head or tail)

once we have the ID and are in the list, we set the owner-related mapping

cdpOwners[_id] = owner;
_addCdpToOwnerEnumeration(owner, _id);

_ownedCdps[to][length] = cdpId;
_ownedCdpIndex[cdpId] = length;
_ownedCount[to] = length + 1;

🦉 cdpOwners gives us the owner of a given ID, which is redundant with the address in the
If one of these is wrong (aka different), we will permanently DDOS that CdpId
This is pretty much as bad as stealing it by granting to wrong owner so if that's wrong it's fundamentally wrong
Let's move to one of these checks

Where is CdpOwners used?

when we _requireCdpOwner in BorrowerOperations.
This happens in any adjust* or close operation

We use it to find the owner of a given cdpId in redemptions ex

We do use it in Leverage too, so we need to see how we can avoid this.
The key thing there is an O(1) to lookup the same

Where is _ownedCount used?

via cdpCountOf

Externally:

• it's used in testing to confirm the proper CDP count of the owner is present. It's pretty much just tested to confirm it works but is never used
• it's used in Leverage to find the ID of the just-created CDP (doesn't seem necessary)

Within SortedCdps:

• it's used to determine if an index is out-of-bounds for cdpOfOwnerByIndex
• it's used to getCdpsOf a user, so we know how many there are to walk down the list

where is getCdpsOf then used?

in tests, mostly to confirm it's functionality. So, it's effectively unused outside of confirming it works

CdpManager

we call initializeCdp in CdpM

we set the values of the core CDP struct

struct Cdp {
        uint debt;
        uint collShares;
        uint stake;
        uint liquidatorRewardShares;
        Status status;
        uint128 arrayIndex;
    }

We also set the snapshots to the current value global values (after updating the global values if needed)

stFeePerUnitcdp[_cdpId]
rewardSnapshots[_cdpId]

Where is arrayIndex used?

It's used in HintHelpers, think that's it.

API team: how does hinthelpers fit into your workflow

It's never used on-chain so having more off-chain components is ok.

Conclusions

There are two functions to get a CDP Owner. If they both don't work a CDP is permanently DDOSed. Let's reduce that to the one which uses the CdpId.

• Do we really not need existCdpOwners or more importantly, the underlying data cdpOwners anywhere?
• Is the read access efficiency the same? yes, actually better for address from ID because ID is a memory input

In Leverage, we just want to know the ID of the Cdp which was just created. We don't need to read any values for this.
We can deterministically recreate the ID of the CDP which was just created by the previous nonce, blockHeight (we're in the same block) and the owner address which we know as an input.

Do we really need NodeAdded and NodeRemoved events? I doubt it.

We should add more CDP getter functions to MultiCdpGetter

• add a method to CdpManager to return the whole CDP struct
• add a method to return both the reward index values

In MultiCdpGetter return these both in a new struct. We can also return it's node and anything else that remains from SortedCdps.

do we need this event in CdpManager? emit CdpIndexUpdated(idToMove, index);

_addCdpIdToArray does not have a symetrical removal function. Is this justified?

"getCurrentICR" -> "getICR": it doesn't get the current ICR, it gets the ICR taking in a price value. It happens to be the current value where we use it.

Consider removing the whole redemption bootstrap period thing now that redemptions are pausable. And then pause redemptions at the start

Consider packing the redemption variables.

There's no reason we need to do the SortedCdps stuff in BO separate from the CdpManager operation. Just pass in the necessary variables to CdpM and let it do it's thing. It makes more conceptual sense this way (CdpM manages Cdps, including the sorted list)

Content

We should show the thought process behind this design, and what things we had and then removed.

We should show how batchRemove improves efficiency of liquidations and redemptions as it scales.