Skip to content

Conversation

@arr00
Copy link
Collaborator

@arr00 arr00 commented Dec 18, 2025

This PR adds a return type of uint48 to the requestRedeem function. The return value is the timestamp at which the redemption request is ready for redemption--after the cooldown is complete. This make building on top of the OperatorStaking contract much simpler.

We also opted to revert when calls to redeem or requestRedeem specify 0 shares.

@arr00 arr00 requested a review from a team as a code owner December 18, 2025 09:31
@cla-bot cla-bot bot added the cla-signed label Dec 18, 2025
@mergify
Copy link

mergify bot commented Dec 18, 2025

🧪 CI Insights

Here's what we observed from your CI run for f6a6488.

🟢 All jobs passed!

But CI Insights is watching 👀

assert(releaseTime >= lastReleaseTime); // should never happen
$._redeemRequests[controller].push(releaseTime, controllerSharesRedeemed + shares);

emit RedeemRequest(controller, ownerRedeem, 0, msg.sender, shares, releaseTime);
Copy link
Member

Choose a reason for hiding this comment

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

Just realized this now, but we no longer need the 3rd argument here, which is always 0, in the RedeemRequest event, initially it was only here because we tried to adhere to the ERC7540 (async vaults), but since we dont follow this ERC anyways, especially now that we added the releaseTime in the event, I think we could remove it... wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

But if we do this, we will probably need to update the front-end app as well, so it might be too late actually.

Copy link
Contributor

Choose a reason for hiding this comment

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

we will update this in #1681 + consider the change in the frontend once it is merged yes !

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants