yAudit Dopex RDPX Token Review

Dopex is a Decentralized Options Exchange that uses blockchain technology to provide permissionless and non-custodial access to options trading. The RDPX token is a rebate token of the Dopex platform and is used to partially compensate option writer losses to increase liquidity depth.

The contracts of the Dopex RDPX Token Repo were reviewed over 3 days. The code review was performed by 2 auditors between October 16 and October 18, 2023. The repository was under active development during the review, but the review was limited to the latest commit at the start of the review. This was commit b1bc0881aa01520bc8082b2dd35077120928c4ef for the Dopex RDPX token repo.

Scope

The scope of the review consisted of the following contracts at the specific commit:

After the findings were presented to the Dopex team, fixes were made and included in several PRs.

This review is a code review to identify potential vulnerabilities in the code. The reviewers did not investigate security practices or operational security and assumed that privileged accounts could be trusted. The reviewers did not evaluate the security of the code relative to a standard or specification. The review may not have identified all potential attack vectors or areas of vulnerability.

yAudit and the auditors make no warranties regarding the security of the code and do not warrant that the code is free from defects. yAudit and the auditors do not represent nor imply to third parties that the code has been audited nor that the code is free from defects. By deploying or using the code, Dopex and users of the contracts agree to use the code at their own risk.

Code Evaluation Matrix

Category	Mark	Description
Access Control	Low	The token contract has an option to mint and pause. Both roles, as well as the admin role and the proxy admin owner, were set to the same EOA.
Mathematics	Good	No complex math was used.
Complexity	Good	Reviewed token upgrade didn't introduce unnecessary complexity to the token.
Libraries	Average	Only standard OZ libraries were used with the exception of custom `ERC20PresetMinterPauserUpgradeable`.
Decentralization	Low	The protocol does not have decentralized governance yet and the protocol team still controls many key protocol values without a Timelock.
Code stability	Average	The code is an upgrade to the currently deployed token on Arbitrum. However, some errors in the reviewed code, including the reverting `initialize()` function, demonstrate the code was not heavily tested.
Documentation	Average	The development team didn't provide documentation of the incident as to why the tokens are locked in the liquidity pool or which liquidity this token upgrade will recover. However, proper NatSpec and detailed comments existed on the functions that needed explaining.
Monitoring	Average	Events were emitted where applicable but missing in some functions.
Testing and verification	Low	Only a few tests were available at the time of the review which didn't cover all regular user flows nor added functionalities to the token.

Findings Explanation

Findings are broken down into sections by their respective impact:

Critical, High, Medium, Low impact
- These are findings that range from attacks that may cause loss of funds, impact control/ownership of the contracts, or cause any unintended consequences/actions that are outside the scope of the requirements
Gas savings
- Findings that can improve the gas efficiency of the contracts
Informational
- Findings including recommendations and best practices

Critical Findings

None.

High Findings

1. High - Incorrect address used in `_transfer()` logic

The incorrect address is used when fetching the balance of the fee discount token for the to address.

Technical Details

The variable feeDiscountTokenBalanceTo fetches the value of token balance for address from but should fetch the balance for the address to as the variable name describes.

Impact

High. Users will pay the fee even if the to address has enough discount tokens.

Recommendation

Properly fetch the balance of the discount token for the to address.

- uint256 feeDiscountTokenBalanceTo = IERC20(feeDiscount.token).balanceOf(
-     from
- );
+ uint256 feeDiscountTokenBalanceTo = IERC20(feeDiscount.token).balanceOf(
+     to
+ );

Developer Response

Valid issue, will follow recommendation to fix.

Medium Findings

1. Medium - Blacklisted funds can be transferred

The default admin role can blacklist accounts and prevent them from transferring the RDPX token. But a blacklisted user still has the option to bridge the token to L1 using the Arbitrum gateway, which will burn blacklisted tokens on Arbitrum and give the user tokens on L1, where there is no RDPX blacklist functionality (yet).

Technical Details

The contract implementation of the Arbitrum gateway function bridgeBurn() doesn't check if the account is blacklisted before burning RDPX tokens. This allows the user to bridge the tokens back to L1, transfer it to another address, and bridge it back to Arbitrum to repeat the unwanted behaviour. Admins have no ability to break this loop with the existing implementation.

The two most popular tokens that implement blacklisting are USDC and USDT. Each has a different approach to what to do with blacklisted tokens. USDC prevents users from transferring blacklisted tokens. While USDT does the same, it has an additional option to destroy/burn blacklisted tokens.

Unibot token, which is used as an example for the blacklisting solution, only prevents users from transferring the tokens. Because Unibot is not bridgeable, it doesn't have a problem with moving/bridging blacklisted tokens.

Impact

Medium. Admin restrictions can be bypassed.

Recommendation

Verify that tokens are not blacklisted before calling _burn() function inside bridgeBurn(). Do the same check inside bridgeMint(). Include a require check:

require(!blacklisted[account], "Account blacklisted");

Bridged USDC.e on Arbitrum has implemented a similar solution to disable bridgeMint() and bridgeBurn() functions for blacklisted accounts.

Developer Response

Valid issue, will follow recommendation to fix.

2. Medium - RDPX token admin is EOA

The DEFAULT_ADMIN_ROLE of the RDPX v1 token, currently deployed on-chain, is an EOA. Because the admin role is privileged, it is recommended to avoid a single point of failure and use a multisig or similar smart contract as the admin.

Technical Details

The DEFAULT_ADMIN_ROLE address of the on-chain RDPX token can be checked with:

cast call 0x32Eb7902D4134bf98A28b963D26de779AF92A212 "getRoleMember(bytes32,uint256)(address)" "0x0000000000000000000000000000000000000000000000000000000000000000" "0" --rpc-url https://arbitrum.llamarpc.com

The same EOA address has the MINTER_ROLE and PAUSER_ROLE roles too. The EOA is also the owner of the ProxyAdmin contract that governs upgrades of the token contract. Many DeFi protocols aim to increase trust in users by removing single points of failure and ensuring governance cannot act unilaterally without warning users. To this end, many protocols use multisig contracts for the privileged admin role of their contracts. The on-chain RDPX contract does not use the privileged default admin role for any specific purposes, but the new RDPX v2 token does. As part of the process of upgrading the token contract, it is recommended to change the privileged roles first in order to avoid a single point of failure if any unexpected events should impact the EOA address. Unexpected events could include loss of the private key, making the admin role inaccessible to anyone, or an unauthorized party gaining access to the private key through unexpected means.

Impact

Medium. The deployment scripts are not complete for RDPX v2, so a change in privileged roles may already be part of the plan. The current on-chain settings would not be ideal if carried over to RDPX v2 where the EOA has access to special functions like minting tokens and changing fees without notice.

Recommendation

Change the DEFAULT_ADMIN_ROLE, MINTER_ROLE, and PAUSER_ROLE from an EOA to a multisig or similar smart contract. First use grantRole() from OpenZeppelin's access control library to grant the new multisig each role, then call revokeRole() to remove the EOA from these roles.

The owner of the ProxyAdmin contract should also be changed to a multisig to improve protection around token contract upgrades.

Developer Response

Valid issue, the EOAs roles will be passed on to our Team Multisig.

Low Findings

1. Low - Ensure fees do not exceed 100%

sellFees and buyFees are declared with the comment 1e8 precision, but there are no checks to prevent the fee from exceeding 100% (a value of 1e8).

Technical Details

buyFees() and sellFees() have 1e8 precision, but updateBuyFees() and updateSellFees() do not verify that the value stored exceeds 1e10. If a fee exceeds 100% (a value of 1e10), it will cause any transfer to fail due to an underflow in this subtraction.

Impact

Low. The value is admin controlled, but from the user perspective, the comments make it unclear how the fee value will be used in practice.

Recommendation

Add require(_sellFees < 1e10); to buyFees() and require(_sellFees < 1e10); to sellFees() to prevent the admin from settings fees that exceed the maximum intended value. If the actual maximum value is 1e10, then replace 1e10 with 1e8. The meaning of '1e8 precision' should be further elaborated on where these variables are declared or in the setter function NatSpec.

Developer Response

Valid issue, will follow recommendation to fix and add better comments.

2. Low - Transfer will revert on invalid fee discount

Ensure that buy and sell fees are above fee discount value and that the fee discount token is IERC20.

Technical Details

If buyFees or sellFees is below fee discount, every transfer where the user has fee discount will revert with arithmetic underflow. More importmant, if the fee discount token is not IERC20 compatible, every token transfer will fail on fetching token balance. This also means that thefeeDiscount must be set before the any token transfer.

Impact

Low. Incorrect setup will prevent users from transferring tokens.

Recommendation

Before updating feeDiscount, ensure the discount value is below or equal to buy and sell fees. Also, verify that the discount token has the method balanceOf.
Add a require check before feeDiscount:

require(_feeDiscount.discount <= buyFees, "!buyFees");
require(_feeDiscount.discount <= sellFees, "!sellFees");
IERC20(_feeDiscount.token).balanceOf(address(this)); // verify token is IERC20.balanceOf compatible
feeDiscount = _feeDiscount;

With the current fees discount calculation, discount must be lower than both fees. If you want to have only buy fees, without sell fees, and add discount, you should change how fees are calculated in transfer method or split discount value for both fees.

Developer Response

Valid issue, will follow recommendation to fix and add better comments.

The intended token to be used for fee discount is veDPX which is a vote escrow token following the exact specification of the veCRV model which makes the ERC20 non-transferrable and decaying over time.

3. Low - Transfer will revert on sending fees to address(0)

Ensure that feeCollector is not address(0) before setting fees.

Technical Details

If feeCollector is address(0), every transfer that has fees will revert because it's not allowed to send ArbRdpxTokenV2 to address(0).

Impact

Low. Incorrect setup will prevent users from transferring tokens.

Recommendation

A require check can prevent feeCollector from storing the zero address.

require(feeCollector != address(0), "!feeCollector");

This require check should be added in three places:

before setting buyFees in updateBuyFees()
before setting sellFees in updateSellFees()
before setting feeCollector in updateFeeCollector() to verify that the feeCollector is not set to address(0) after buyFees and sellFees are set.

Developer Response

Valid issue, will follow recommendation to fix.

Gas Saving Findings

1. Gas - Replace require with custom errors

Several require statements in RDPX v2 token can be replaced with custom errors. They are more gas efficient.

Technical Details

ArbRdpxTokenV2.sol contains 10 require statements that can all be changed to custom errors for minor gas savings.

Impact

Gas savings.

Recommendation

Use solidity custom errors instead of require statements.

2. Gas - Change `blacklistRenounced` type to uint256

A minor gas optimization is possible by changing the type of blacklistRenounced from bool to uint256.

Technical Details

blacklistRenounced is a bool but changing it to a uint256 type that stores 0 or 1 can save gas on deployment.

Impact

Gas savings.

Recommendation

Consider changing the type of blacklistRenounced for a minor gas savings on deployment demonstrated below.

Deployment Cost with bool type `blacklistRenounced`	Deployment Size
2234606	11196

Deployment Cost with uint256 type `blacklistRenounced`	Deployment Size
2227581	11153

3. Gas - Change static variables in Cup to constants

Several variables in Cup.sol can be constants, which saves gas on deployment and when calling sip().

Technical Details

There are 6 variables in Cup.sol that are never modified and can be made constant to save gas. The tables below show the before/after gas differences.

Cup.sol (before change to use constants)
Deployment Cost	Deployment Size
4633293	22676
Function Name	min	avg	median	max
sip	4025089	4025089	4025089	4025089

Cup.sol (after change to use constants)
Deployment Cost	Deployment Size
4545401	22708
Function Name	min	avg	median	max
sip	4023193	4023193	4023193	4023193

Impact

Gas savings.

Recommendation

Make the 6 static variables in Cup.sol constants.

4. Gas - Optimize transfer function

Transfer function in contract ArbRdpxTokenV2 can be gas optimized.

Technical Details

The internal _transfer() function can be optimized with small changes. This will be the most used function so any gas improvements will be good for end users and protocols.

Impact

Gas savings.

Recommendation

Rewrite the _transfer() function:

function _transfer(
    address from,
    address to,
    uint256 amount
) internal override {
    require(!blacklisted[from], "Sender blacklisted");
    require(!blacklisted[to], "Receiver blacklisted");
    if (amount == 0) {
        super._transfer(from, to, 0);
        return;
    }

    if (!excludedFromFees[from] && !excludedFromFees[to]) {
        uint256 fees;
        IERC20 discountToken = IERC20(feeDiscount.token);
        uint256 feeDiscountTokenBalanceFrom = discountToken.balanceOf(from);
        uint256 feeDiscountTokenBalanceTo = discountToken.balanceOf(to);
        uint256 _sellFees = sellFees;
        uint256 _buyFees = buyFees;

        // reduce _sellFees and _buyFees if applicable
        uint256 requiredBalanceCached = feeDiscount.requiredBalance;
        if (
            feeDiscountTokenBalanceFrom >= requiredBalanceCached ||
            feeDiscountTokenBalanceTo >= requiredBalanceCached
        ) {
            uint256 feeDiscountCached = feeDiscount.discount;
            _sellFees -= feeDiscountCached;
            _buyFees -= feeDiscountCached;
        }
        // only take fees on buys/sells, do not take on wallet transfers
        // on sell
        if (automatedMarketMakerPairs[to] && _sellFees > 0) {
            fees = (amount * _sellFees) / 1e10;
            super._transfer(from, feeCollector, fees);
            amount -= fees;
        }
        // on buy
        else if (automatedMarketMakerPairs[from] && _buyFees > 0) {
            fees = (amount * _buyFees) / 1e10;
            super._transfer(from, feeCollector, fees);
            amount -= fees;
        }
    }

    super._transfer(from, to, amount);
}

This table shows a comparison of current and proposed _transfer() function gas costs. The test covers four different scenarios in the _transfer() function.

Function Name	min	avg	median	max	# calls
transfer original	3911	35311	36063	65207	4
transfer proposed	3841	33259	32372	64453	4

5. Gas - Unnecessary setting of variables to default values

Variables don't need to be set to their default values.

Technical Details

Variable blacklistRenounced is set to false while the default value for any bool is already false.

Impact

Gas savings.

Recommendation

Remove explicitly setting blacklistRenounced to false. Add a comment that false is the default value for clarity.

bool public blacklistRenounced; // false is default value

This will reduce contract deployment cost and size:

ArbRdpxTokenV2 contract
Deployment Cost	Deployment Size	Version
2214784	11097	Original
2212560	11083	Recommended

Informational.

Recommendation

The easiest solution is for the contract admin to only choose a feeDiscount.token that does not have flashloans, but a comment should be added to updateFeeDiscount() to specify such a choice if that is the chosen solution. The current plan is to choose veDPX, which is documented as a non-transferable token. If a flashloan bypass of the standard fee payment is not acceptable, consider an alternative approach to applying the discount or store the most recent block that a RDPX v2 token transfer happened so that transfers that may involve flashloans can be detected.

Developer Response

Final remarks

Overall the $rDPX token upgrade should deliver its expected result of rescuing WETH from a pool. However, the lack of documentation explaining why the token is being upgraded and exactly what liquidity is being rescued will draw some questions, especially because the pool is being actively used at the time of this review. The team explained that the burned tokens from the liquidity pool will be minted back to the users by taking a snapshot of the existing state and moving liquidity to another protocol. The token upgrade introduces additional power to the team by introducing a blacklisting option. Also, transfer fees can be set by the team for specific cases like transfers on buy and sell. These changes increase the centralization of token management, which could be partially mitigated by introducing a multisig and more decentralized governance. The team was advised to inform the users before turning on the fees and blacklisting the users.

yAudit Dopex RDPX Token Review

Table of Contents

Review Summary

Scope

Code Evaluation Matrix

Findings Explanation

Critical Findings

High Findings

1. High - Incorrect address used in _transfer() logic

Technical Details

Impact

Recommendation

Developer Response

Medium Findings

1. Medium - Blacklisted funds can be transferred

Technical Details

Impact

Recommendation

Developer Response

2. Medium - RDPX token admin is EOA

Technical Details

Impact

Recommendation

Developer Response

Low Findings

1. Low - Ensure fees do not exceed 100%

Technical Details

Impact

Recommendation

Developer Response

2. Low - Transfer will revert on invalid fee discount

Technical Details

Impact

Recommendation

Developer Response

3. Low - Transfer will revert on sending fees to address(0)

Technical Details

Impact

Recommendation

Developer Response

Gas Saving Findings

1. Gas - Replace require with custom errors

Technical Details

Impact

Recommendation

2. Gas - Change blacklistRenounced type to uint256

Technical Details

Impact

Recommendation

3. Gas - Change static variables in Cup to constants

Technical Details

Impact

Recommendation

4. Gas - Optimize transfer function

Technical Details

Impact

Recommendation

5. Gas - Unnecessary setting of variables to default values

Technical Details

Impact

Recommendation

Informational Findings

1. Informational - Transfer could revert on really high amounts

Technical Details

Impact

Recommendation

Developer Response

2. Informational - Remove unneeded import

Technical Details

Impact

Recommendation

3. Informational - Call initialize() on implementation contracts

Technical Details

Impact

Recommendation

4. Informational - initialize() in RDPX token reverts

Technical Details

Impact

Recommendation

5. Informational - Flashloans could allow fee bypass

1. High - Incorrect address used in `_transfer()` logic

2. Gas - Change `blacklistRenounced` type to uint256

3. Informational - Call `initialize()` on implementation contracts

4. Informational - `initialize()` in RDPX token reverts

← Back to Reports

DopexV2 rDPX upgrade

↑ Back to Top