Skip to content

Comments

Superstate USCC strategy#4

Open
0xnaman1 wants to merge 3 commits intoStrata-Money:strat/superstatefrom
0xnaman1:strat/renzo
Open

Superstate USCC strategy#4
0xnaman1 wants to merge 3 commits intoStrata-Money:strat/superstatefrom
0xnaman1:strat/renzo

Conversation

@0xnaman1
Copy link
Contributor

No description provided.

amount = usdcBalance > 0 ? usdcBalance : amountToRedeem;

// Transfer USDC to receiver
if (amount > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @0xnaman1 , could you please elaborate on how the finalization and the queue work:

  1. We check the amount for the current withdrawRequestIndex.
  2. We claim withdrawRequestIndex.
amount = usdcBalance > 0 ? usdcBalance : amountToRedeem;

Can it be usdcBalance == 0 after claim? If yes, why do we pick amountToRedeem for the transfer if the balance is already zero?

If withdrawRequestIndex > 0, e.g., 1 - we claim the request at index 1. What happens with the request at index 0? Or are they claimed all at once?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Ideally the claimed balance should never be zero. It could only be zero if the LEZyVault.sol vault's exchange rate drops to zero. And that's an edge case. So this is a check for safety.

  2. The WithdrawQueue.sol maintains a global queue for requests. If the cooldown is for ex 7 days,

Withdrawal 1 at day 0 → unlocks at day 7
Withdrawal 2 at day 3 → unlocks at day 10

First finalization at day 7 → only request 1 is claimable
Second finalization at day 10 → request 2 becomes claimable

Hence, request 0 would have already been claimed if 1 is claimable now. Or at least 0 would be claimable as well if we are trying to claim 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, still not quit clear:

  1. Does withdrawQueue.withdrawRequests return amountToRedeem for a specific index (withdrawRequestIndex) or for all requests in the range [0..withdrawRequestIndex] when withdrawRequestIndex > 0?

  2. Same for claim: does it claim only the specific index or all requests up to the specified index?

  3. Something still seems off with the logic here:

uint256 usdcBalance = USDC.balanceOf(address(this));
// ... if usdcBalance == 0
amount = amountToRedeem;
// ... if amount > 0
USDC.safeTransfer(receiver, amount);

Here, if usdcBalance == 0 and the reported amountToRedeem > 0, we attempt to transfer USDC. However, as we have just checked, the balance is equal to 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. This is the definition of withdrawRequests
struct WithdrawRequest {
    uint256 withdrawRequestID;
    uint256 amountToRedeem;
    uint256 sharesLocked;
    uint256 createdAt;
    uint256 fillAt;
 }

/// @dev mapping of withdraw requests array, indexed by user address
mapping(address => WithdrawRequest[]) public withdrawRequests;

So it only returns the request at a single index passed as an argument. Similar functionality with claim. It only claims the request at that index.

  1. About the usdcBalance code snippet, yeah, it seems incorrect. It shouldn't transfer USDC if it didn't claim anything. Lemme refactor it.

Also, I found a bug with the current withdraw flow. In case the sUSCCCooldownRequestImpl proxy is reused, it overwrites the previous index in the queue. Thus, making the previous withdraw index unclaimable. Let me check how to fix this in implementation contract.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @0xnaman1, right - the implementation should claim all Underlying Requests before marking Our Cooldown Request as inactive.

Since we also have the mechanic where, on redemption, we create a distinct unstake cooldown request on our side (an extra proxy contract), it means there will be a single Underlying Request per Our Cooldown Request. This works up to 70 requests; after that, the user's last cooldown request will be extended with an additional underlying request.

In that case, there will be multiple underlying requests, but either way, to finalize Our Cooldown Request, we must finalize (claim) all requests from the Underlying Protocol.

Also, the Superstate team once said that finalization (claiming) works only after the cooldown period has passed and when their contract has enough assets to fulfill the claim.

Can we check this during finalization - i.e., verify that the queued request is actually claimable? It seems we need to include this in our balanceOf calculation to show the user what is already finalizable.

function totalAssets() external view returns (uint256 baseAssets) {
uint256 rawAssets = _getRawTotalAssets();
uint256 unvested = getUnvestedAmount();
return rawAssets > unvested ? rawAssets - unvested : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @0xnaman1 , the issue here is that rawTotalAssets returns the current total assets from the underlying protocol, while getUnvestedAmount refers to the cached rawTotalAssets. So whenever Strategy::totalAssets is called before _updateVesting, it returns total assets including the gain, which is incorrect.

For example, Strategy::totalAssets can be called while updating accounting and calculating the tranche's totalAssets to compute the current exchange rate for deposit/redeem. And the tranche calls this method before calling cdo.deposit (strategy.deposit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants