Usual USD0++ Upgrade & Redemption Token Security Review Report

The mint(uint256 amountUsd0) and mint(uint256 amountUsd0, address bAssetRecipient, address rAssetRecipient) functions are both protected with the nonReentrant and whenNotPaused modifiers.

However, the mintWithPermit() function does not include these modifiers, resulting in inconsistent security and pause protection across the different minting pathways.

function mintWithPermit(uint256 amountUsd0, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
    external
{

Add the whenNotPaused and nonReentrant modifier to mintWithPermit() to ensure consistent pause behavior across all minting pathways.

function mintWithPermit(uint256 amountUsd0, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
    external
++  nonReentrant
++  whenNotPaused
{

In the Usd0PP.unlockUSD0ppWithUsualWithPermit() function, lines 392–402 invoke permit() to grant the contract an allowance to spend the caller's Usd0PP tokens:

function unlockUSD0ppWithUsualWithPermit(
    uint256 usd0ppAmount,
    uint256 maxUsualAmount,
    PermitApproval calldata usualApproval,
    PermitApproval calldata usd0ppApproval
) external {
    Usd0PPStorageV0 storage $ = _usd0ppStorageV0();
 
    // Execute the USUAL permit
    try IERC20Permit(address($.usual))
        .permit(
            msg.sender,
            address(this),
            maxUsualAmount,
            usualApproval.deadline,
            usualApproval.v,
            usualApproval.r,
            usualApproval.s
        ) {}
    catch {} // solhint-disable-line no-empty-blocks
 
    // Execute the bUSD0 permit
    try IERC20Permit(address(this))
        .permit(
            msg.sender,
            address(this),
            usd0ppAmount,
            usd0ppApproval.deadline,
            usd0ppApproval.v,
            usd0ppApproval.r,
            usd0ppApproval.s
        ) {}
    catch {} // solhint-disable-line no-empty-blocks
 
    // Call the standard unlock function
    unlockUSD0ppWithUsual(usd0ppAmount, maxUsualAmount);
}

This second permit call is unnecessary. The underlying unlockUSD0ppWithUsual() function burns the caller's Usd0PP using the internal _burn() method, which deducts tokens directly from msg.sender and does not require any allowance. As a result, granting approval for Usd0PP via permit() provides no functional benefit and introduces redundant logic.

Consider removing the second permit call in function unlockUSD0ppWithUsualWithPermit().

Several parts of the contract Usd0PP use the literal value 1e18 directly instead of the defined constant SCALAR_ONE. This leads to inconsistencies and makes future refactoring or scalar-related changes more error-prone.

Two instances where 1e18 is used directly include:

1. Comparison of newFloorPrice inside updateFloorPrice()

if (newFloorPrice > 1e18) {
    revert FloorPriceTooHigh();
}

2. Calculation of usd0Amount inside unlockUsd0ppFloorPrice()

uint256 usd0Amount = Math.mulDiv(usd0ppAmount, $.floorPrice, 1e18, Math.Rounding.Floor);

Using the hard-coded literal breaks the intended abstraction of the scalar system and may cause inconsistencies if SCALAR_ONE is ever modified or if the scaling logic needs to evolve.

Consider replacing all occurrences of 1e18 with the predefined constant SCALAR_ONE to ensure consistency.

In the previous contract version, the mint() function explicitly validated that the bond period had already begun before allowing any minting. The logic looked as follows:

function mint(uint256 amountUsd0) public nonReentrant whenNotPaused {
    Usd0PPStorageV0 storage $ = _usd0ppStorageV0();
 
    // revert if the bond period isn't started
    if (block.timestamp < $.bondStart) {
        revert BondNotStarted();
    }
 
    // revert if the bond period is finished
    if (block.timestamp >= $.bondStart + BOND_DURATION_FOUR_YEAR) {
        revert BondFinished();
    }
 
    // get the collateral token for the bond
    $.usd0.safeTransferFrom(msg.sender, address(this), amountUsd0);
 
    // mint the bond for the sender
    _mint(msg.sender, amountUsd0);
}

This implementation correctly reverted when users attempted to mint before the bond period had started.

In the updated version, mint() delegates its core logic to the internal _deconstruct() function. However, _deconstruct() no longer includes the check to ensure that the current timestamp is past bondStart, only validating whether the bond period has already ended:

function _deconstruct(uint256 amountUsd0, address bAssetRecipient, address rAssetRecipient)
    internal
{
    Usd0PPStorageV0 storage $ = _usd0ppStorageV0();
 
    if (amountUsd0 == 0) {
        revert AmountIsZero();
    }
 
    // revert if the bond period is finished
    if (block.timestamp >= $.bondStart + BOND_DURATION_FOUR_YEAR) {
        revert BondFinished();
    }
 
    // get the collateral token for the bond
    $.usd0.safeTransferFrom(msg.sender, address(this), amountUsd0);
 
    // mint the bond token for the specified recipient
    _mint(bAssetRecipient, amountUsd0);
 
    // mint the redemption token for the specified recipient
    $.rtusd0.mint(rAssetRecipient, amountUsd0);
 
    emit Deconstructed(msg.sender, amountUsd0, bAssetRecipient, rAssetRecipient);
}

As a result, users are now able to mint before the bond period begins - behavior that differs from the previous design and likely violates the intended lifecycle of the bond.

Consider reintroducing the missing check:

if (block.timestamp < $.bondStart) {
    revert BondNotStarted();
}

inside _deconstruct() function.

In the Usd0PP contract, several public functions such as unwrap() and mint() explicitly include the whenNotPaused modifier. However, each of these functions ultimately calls _mint() or _burn(), both of which invoke the internal _update() function.

The _update() function in ERC20PausableUpgradeable already applies the whenNotPaused modifier:

// contract ERC20PausableUpgradeable
 
function _update(address from, address to, uint256 value)
    internal
    virtual
    override
    whenNotPaused
{
    super._update(from, to, value);
}

As a result, the pause check is executed twice once at the beginning of the public function and again during the ERC-20 token state update. This redundancy unnecessarily increases gas consumption without adding any additional safety.

Consider removing the whenNotPaused modifier from the public functions that rely on _mint() or _burn(), since the underlying logic already enforces the pause constraint.

In the Usd0PP contract, the functions pause() and totalBondTimes() are currently declared as public. However, neither of these functions is called internally within the contract.

function pause() public {

function totalBondTimes() public pure returns (uint256) {

Because they are only intended for external use, it is more efficient to mark them as external instead of public. Declaring them as external can reduce gas costs when invoked and more accurately reflects their intended usage.