XDC Prime Stake

V3 Vault

6.1 Returned masternode stakes are double-counted in trackedTotalAssets

When XDC is sent to the validator via proposeMasternode or auto-propose, the native balance decreases but trackedTotalAssets is intentionally left unchanged because the assets are still managed by the protocol, just illiquid at the validator.

IXDCValidator(validator).propose{value: amount}(candidate);
emit MasternodeProposed(candidate, amount);

If that staked XDC later returns to the contract (e.g., the masternode is resigned externally, the validator deregisters the node, or a network-level return occurs), the incoming XDC arrives through receive(). The validator contract does not hold any privileged role, so it follows the non-privileged sender path:

receive() external payable {
  if (migrationInProgress) {
    require(
      hasRole(MIGRATION_MANAGER_ROLE, _msgSender()),
      UnauthorizedMigrationManager()
    );
    emit MigrationLiquidityFunded(_msgSender(), msg.value);
    return;
  } else if (
    !hasRole(OPERATIONS_MANAGER_ROLE, _msgSender()) &&
    !hasRole(DEFAULT_ADMIN_ROLE, _msgSender())
  ) {
    require(msg.value >= MIN_REWARD_DISTRIBUTION, InsufficientLiquidAssets());
  }

  trackedTotalAssets += msg.value;
  _syncTrackedAssetsExcludingInflow(0);
}

The returned stake passes the MIN_REWARD_DISTRIBUTION check and is added to trackedTotalAssets. However, those assets were never removed from trackedTotalAssets when they were originally proposed. The result is that the returned stake is counted twice: once from the original proposal (never deducted) and once from the receive() increment.

The receive() function cannot distinguish between reward inflows (new income that should be tracked) and stake returns (already tracked, should not be added again). This inflates the share price, causing new depositors to receive fewer shares than they should, and allowing existing holders to redeem at an artificially high rate.

Recommendation

Consider adding a privileged function that allows the operations manager to receive returned masternode stakes without incrementing trackedTotalAssets, or tracking proposed-but-not-returned amounts so that receive() can offset returns against outstanding proposals.

Update from Nethermind Security

The fix introduced a new bug that leads to yield suppression and wrong trackedTotalAssets accounting.

The previous issue was addressed, but the new fix introduced a bug that leads to wrong accounting in the case of slashing events.

The main vulnerability resurfaced.

The issue is partially fixed, but the yield suppression vulnerability resurfaced.

Fixed under the assumption that validators will not return the principal amount staked directly to the contract to trigger the receive() logic. Tokens must always be returned via the resignMasternode(...) flow.

6.2 Malicious receivers can trigger a gas bomb attack in _processWithdrawalQueueInternal(...) blocking the withdrawal queue

The PrimeStakedXDC_V3 contract allows the operations manager to process queued user withdrawals via the processWithdrawalQueue(...) function. This function iterates through pending requests and attempts to send the owed native XDC to the designated receiver. To gracefully handle receivers that cannot accept native tokens (like certain smart contracts), the protocol catches the boolean result of the external call and defers the payout if it fails.

function _processWithdrawalQueueInternal(uint256 maxRequests) internal {
    // ...
    while (processedCount < maxRequests) {
        // ...
        request.processed = true;
        _burn(address(this), request.shares);

        trackedTotalAssets -= assetsOut;
        totalWithdrawnByUser[request.owner] += assetsOut;
        // ...

The vulnerability lies in the external call lacking a strict gas limit stipened.

        // ...
        // @audit-issue External call forwards 63/64 of available gas
        (bool success, ) = payable(request.receiver).call{value: assetsOut}("");
        if (!success) {
            // @audit This remaining logic requires gas to execute successfully
            pendingQueuedAssets[request.owner] += assetsOut;
            emit WithdrawalQueuePayoutDeferred(...);
        }

        emit Withdraw(...);
        // ...
    }
}

An external call forwards 63/64 of the currently available gas to the target address. An attacker can set their withdrawal receiver to a malicious "gas bomb" smart contract. This contract can implement a receive() function that purposely runs a loop until it consumes all of its allocated 63/64 gas, eventually reverting the transaction with an Out-of-Gas (OOG) error.

When the call reverts, the EVM returns control to the PrimeStakedXDC_V3 contract with only the remaining 1/64 of the gas. This amount of gas is likely to be insufficient to finish executing the subsequent state updates. Consequently, the parent transaction also reverts due to an OOG error, meaning the request.processed = true state change is never saved.

This attack is amplified by the "infinite loop" behavior described in the below issue because it blocks the entire withdrawal queue. Because the failed request is never successfully marked as processed, the _nextPendingRequestId(...) function will continuously fetch the exact same malicious request ID on every subsequent attempt. This permanently blocks the entire withdrawal queue for all users. The only way for the operations manager to bypass the malicious request is to pump an exorbitant amount of gas into the transaction, ensuring that the remaining 1/64 gas is enough to complete the loop's execution. However, an attacker can continuously queue withdrawals to grief the protocol, forcing the admin to spend money on gas just to keep the queue operational.

Recommendation

Consider enforcing a strict gas limit on the external call to ensure the parent contract always retains enough gas to complete its execution. For example, changing the call to payable(request.receiver).callvalue: assetsOut, gas: 20000("") will safely limit the receiver's gas consumption while still allowing standard wallets and simple fallback functions to receive the funds.

6.3 Permissionless syncTrackedAssets() during migration allows early migrators to steal funded liquidity via share price manipulation

During migration, native XDC sent by the migration manager through fundMigrationLiquidity() or receive() is intentionally excluded from trackedTotalAssets so that it does not affect ERC4626 share pricing for migrate() calls:

receive() external payable {
  if (migrationInProgress) {
    require(
      hasRole(MIGRATION_MANAGER_ROLE, _msgSender()),
      UnauthorizedMigrationManager()
    );
    emit MigrationLiquidityFunded(_msgSender(), msg.value);
    return; // early return, trackedTotalAssets not updated
  }
  // ...
}

function fundMigrationLiquidity()
  external
  payable
  onlyMigrationManager
  whenMigrationInProgress
{
  require(msg.value != 0, ZeroStakeAmount());
  emit MigrationLiquidityFunded(_msgSender(), msg.value);
  // trackedTotalAssets not updated
}

However, syncTrackedAssets() is permissionless and has no migration-mode guard. It detects the surplus between address(this).balance and trackedTotalAssets and folds it in as if it were newly arrived reward income:

function syncTrackedAssets() external {
  _syncTrackedAssetsExcludingInflow(0);
}

This allows anyone to call syncTrackedAssets() after migration liquidity has been funded, incorporating that liquidity into trackedTotalAssets mid-migration. Since migrate() computes share amounts via previewDeposit(assets), which depends on totalAssets(), the share pricing for all subsequent migrators changes dramatically.

An attacker who migrates a small amount before the sync receives shares priced against a low trackedTotalAssets. After calling syncTrackedAssets(), all later migrators receive far fewer shares for the same V2 token amount because trackedTotalAssets now includes the full migration liquidity. The attacker ends up holding a disproportionately large fraction of the total share supply and can redeem a corresponding share of the funded liquidity after migration ends.

Recommendation

Consider adding a whenMigrationFinished guard to syncTrackedAssets(), or have the sync function account for migration-mode state by skipping the sync when migrationInProgress is true.

6.4 Deferred withdrawal liabilities are not ring-fenced, enabling share price inflation and claim insolvency

When a queued withdrawal is processed but the native transfer to receiver fails, the protocol burns the shares and reduces trackedTotalAssets while the XDC remains in the contract balance. The owed amount is recorded in pendingQueuedAssets[owner] for later claiming.

trackedTotalAssets -= assetsOut;
totalWithdrawnByUser[request.owner] += assetsOut;

(bool success, ) = payable(request.receiver).call{value: assetsOut}("");
if (!success) {
    pendingQueuedAssets[request.owner] += assetsOut;
}

The deferred payout XDC is not isolated from the rest of the protocol's accounting and liquidity, creating two distinct impacts.

Because the XDC stayed in the contract, address(this).balance now exceeds trackedTotalAssets. Any subsequent call that triggers _syncTrackedAssetsExcludingInflow(0) (which runs inside stake, depositNative, redeem, withdraw, redeemWithQueue, withdrawWithQueue, and syncTrackedAssets) detects this surplus and re-adds it to trackedTotalAssets as if it were newly arrived reward income:

uint256 currentBalanceExcludingInflow = liquid - inflow;
if (currentBalanceExcludingInflow > trackedTotalAssets) {
    uint256 untracked = currentBalanceExcludingInflow - trackedTotalAssets;
    uint256 rewardsToSync = (untracked / MIN_REWARD_DISTRIBUTION) * MIN_REWARD_DISTRIBUTION;
    if (rewardsToSync > 0) {
        trackedTotalAssets += rewardsToSync;
    }
}

When the deferred claimant later calls claimQueuedAssets, the XDC leaves the contract but trackedTotalAssets is not reduced:

function claimQueuedAssets(
    address payable receiver
) external nonReentrant returns (uint256 amount) {
    amount = pendingQueuedAssets[_msgSender()];
    require(amount != 0, NoQueuedAssetsToClaim());
    pendingQueuedAssets[_msgSender()] = 0;

    (bool success, ) = receiver.call{value: amount}("");
    require(success, TransferFailed());
}

The result is that the same XDC is counted twice: once as backing for outstanding shares (via the re-sync) and once as a deferred liability (via pendingQueuedAssets). This inflates the share price for remaining holders beyond what the contract can actually pay out, creating a solvency gap equal to the deferred amount. This accounting inflation requires the protocol to be in a mostly liquid state where address(this).balance is close to trackedTotalAssets (e.g., early in operation before masternodes are proposed), and requires the deferred amount to be at least MIN_REWARD_DISTRIBUTION (1,000 XDC) for the sync to trigger.

Independently of the sync inflation, the deferred XDC is also not excluded from the protocol's available liquidity. Both _availableForImmediateWithdrawal() and _availableForQueuePayout() use raw address(this).balance without subtracting pending deferred claims:

function _availableForImmediateWithdrawal() internal view returns (uint256) {
    uint256 liquid = address(this).balance;
    uint256 keep = desiredBuffer();
    if (liquid <= keep) {
        return 0;
    }
    return liquid - keep;
}

This means other users can consume the deferred XDC through instant withdraw()/ redeem() calls, the queue processor can use it to pay other queued withdrawals, and proposeMasternode can lock it into a validator. The victim's shares have already been burned during queue processing, so they cannot re-queue. Their only recovery path is claimQueuedAssets, which reverts if the balance is insufficient, leaving their funds in limbo until fresh deposits or masternode returns replenish the contract. This impact applies even for deferred amounts below MIN_REWARD_DISTRIBUTION and regardless of how liquid the vault is.

Recommendation

Consider introducing a global totalDeferredPayouts state variable that is incremented when a payout is deferred and decremented in claimQueuedAssets. Subtract this value from address(this).balance when calculating available liquidity in _availableForImmediateWithdrawal() and _availableForQueuePayout(). Additionally, subtract it from the balance in _syncTrackedAssetsExcludingInflow to prevent deferred amounts from being mistakenly categorized as validator yield. Finally, decrement trackedTotalAssets by amount inside claimQueuedAssets.

6.5 Flawed _nextOperatorIndex reset logic in removeOperator(...) skips validator turns

The PrimeStakedXDC_V3 contract distributes stake among registered operators in a round-robin fashion. To select the next eligible operator, the _selectNextAvailableOperator() function computes the target index using a modulo operation: _nextOperatorIndex % _operators.length.

The problem occurs within the removeOperator(...) function. When an operator is removed, the function checks if the _nextOperatorIndex is greater than the new _operators.length. If it is, the index is reset to 0.

function removeOperator(address operator_) external onlyAdmin {
  // ...
  isOperator[operator_] = false;
  // @audit-issue Resetting to 0 breaks the ongoing modulo sequence
  if (_nextOperatorIndex > _operators.length) {
    _nextOperatorIndex = 0;
  }

  emit OperatorRemoved(operator_);
}

By resetting it to 0, the continuous round-robin sequence is destroyed.

For example, imagine a set of 5 validators where the _nextOperatorIndex pointer is currently at 9. The next validator to be selected is at index 4 (since 9 % 5 = 4). If the admin removes a validator, the array length shrinks to 4. Because 9 > 4, the reset condition triggers and _nextOperatorIndex becomes 0. On the next staking operation, the system will evaluate 0 % 4 = 0 and select the validator at index 0.

This leads to an unfair distribution of stakes. The validator previously scheduled for the next turn (index 4) is skipped, while the validator at index 0 is favored.

Recommendation

Consider removing the _nextOperatorIndex reset logic entirely. Since the _selectNextAvailableOperator(...) function relies on modulo arithmetic, the continuously growing index will naturally and correctly wrap around the new array length without requiring a manual reset to 0.

6.6 Legacy withdraw() is permanently bricked due to nested nonReentrant guards

The no-argument withdraw() function, intended as a legacy-compatible full-balance unstake path for V2 users, carries the nonReentrant modifier and internally calls redeem(), which also carries nonReentrant. OpenZeppelin's ReentrancyGuard uses a shared storage flag that is set to ENTERED on the first call; when the nested redeem() checks the same flag, it finds it already entered and reverts with ReentrancyGuardReentrantCall.

function withdraw() external nonReentrant returns (uint256 assetsOut) {
  uint256 shares = balanceOf(_msgSender());
  assetsOut = redeem(shares, _msgSender(), _msgSender());
}

function redeem(
  uint256 shares,
  address receiver,
  address owner
)
  public
  virtual
  override
  nonReentrant
  whenMigrationFinished
  returns (uint256 assetsOut)
{
  _syncTrackedAssetsExcludingInflow(0);
  assetsOut = super.redeem(shares, receiver, owner);
}

Because withdraw() is an external function calling a public function on the same contract, Solidity treats this as an internal call that shares the same execution context. The reentrancy lock acquired by withdraw() is still held when redeem() attempts to acquire it, causing an unconditional revert on every call regardless of parameters or caller.

This means the legacy withdrawal interface that V2 users would naturally reach for after migration is completely non-functional. Users must instead discover and use redeem(shares, receiver, owner) directly or one of the other withdrawal paths ( withdraw(uint256,address,address), redeemWithQueue, withdrawWithQueue).

Recommendation

Consider removing the nonReentrant modifier from the no-argument withdraw() since the downstream redeem() call already enforces it.

6.7 Migration manager can freeze all user-facing operations instantly

The setMigrationInProgress function allows the migration manager to toggle migration mode with no governance delay:

function setMigrationInProgress(bool enabled) external onlyMigrationManager {
    migrationInProgress = enabled;
    emit MigrationModeUpdated(enabled);
}

The whenMigrationFinished modifier gates all deposit and withdrawal functions:

• stake(), depositNative() – deposits blocked;
• withdraw(uint256), withdraw(uint256, address, address), redeem(uint256, address, address) – instant withdrawals blocked;
• redeemWithQueue(), withdrawWithQueue() – queued withdrawals blocked;

Setting migrationInProgress = true freezes the entire user-facing protocol.

If the migration manager account is compromised or acts maliciously, the admin cannot override the freeze instantly. The admin must go through the full governance delay cycle to replace the migration manager:

1. Call setMigrationManager(newAddress) to start the governance delay timer;
2. Wait for governanceDelay to elapse (minimum 1 minute, default 1 day);
3. Call executeMigrationManager() to install the new migration manager;
4. Have the new migration manager call setMigrationInProgress(false);

During this entire window, the protocol is completely frozen for all user operations.

Recommendation

Consider making the migration toggle a one-time lifecycle: once migration is turned off after being active, it cannot be re-enabled. This removes the ongoing trust assumption on the migration manager after migration completes.

6.8 The removeOperator(...) function reshuffles the operators array causing validators to skip their turns

The PrimeStakedXDC_V3 contract maintains a list of registered operators in the _operators array to propose them sequentially for masternode staking. The selection mechanism relies on the _selectNextAvailableOperator() function, which uses the _nextOperatorIndex state variable to iterate over the array and pick the next eligible operator in a round-robin fashion.

However, the removeOperator(...) function does not preserve the original array order when an operator is deleted. To avoid leaving gaps in the array, it swaps the element being removed with the last element in the array and then pops the last element.

function removeOperator(address operator_) external onlyAdmin {
  // ...
  uint256 length = _operators.length;
  uint256 removeIndex = _operatorIndexPlusOne[operator_] - 1;
  uint256 lastIndex = length - 1;
  if (removeIndex != lastIndex) {
    // @audit-issue This swap moves the last operator to `removeIndex`
    address lastOperator = _operators[lastIndex];
    _operators[removeIndex] = lastOperator;
    _operatorIndexPlusOne[lastOperator] = removeIndex + 1;
  }
  _operators.pop();
  // ...
}

If an operator positioned at the end of the array is swapped to an index that is lower than the current _nextOperatorIndex, that swapped operator will be skipped in the current selection cycle. For example, assume there are 5 operators in the array and _nextOperatorIndex is 4 (meaning it's the 5th operator's turn to be selected next). If the admin removes the operator at index 1, the 5th operator (at index 4) is moved to index 1. When the next selection occurs, the cursor will wrap around, meaning the 5th operator lost their turn.

This reshuffling disrupts the fairness of the round-robin distribution.

Recommendation

Consider modifying the logic to preserve the order of the operators.

6.9 triggerAutoPropose reverts on validator failure unlike _autoProposeOne

The contract has two internal paths for proposing masternodes. The implicit path used during staking wraps the validator call in try/catch, gracefully handling failures:

function _autoProposeOne() internal {
  // ...
  try
    IXDCValidator(validator).propose{ value: autoProposeThreshold }(candidate)
  {
    emit MasternodeProposed(candidate, autoProposeThreshold);
  } catch {
    emit AutoProposeFailed(candidate, autoProposeThreshold);
  }
}

The explicit path used by triggerAutoPropose does not:

function _proposeNextMasternode(uint256 amount) internal returns (bool) {
  address candidate = _selectNextAvailableOperator();
  if (candidate == address(0) || amount > _availableForImmediateWithdrawal()) {
    return false;
  }

  IXDCValidator(validator).propose{ value: amount }(candidate);
  emit MasternodeProposed(candidate, amount);
  return true;
}

If the validator contract reverts on propose() for any reason (candidate already registered, validator paused, unexpected state), the entire triggerAutoPropose transaction reverts. The implicit stake-path auto-propose continues to work in the same conditions because it catches the error.

Recommendation

Consider wrapping the propose call in _proposeNextMasternode with try/catch to match the error-handling behavior of _autoProposeOne.

6.10 Bridge withdrawal timelock can be neutralized by instantly changing delay and cap

The migration bridge implements a time-delayed withdrawal pattern for excess native XDC via withdrawExcessNative and executeExcessNativeWithdrawal. However, the two protective parameters, withdrawalDelay and dailyWithdrawalCap, can be changed instantly by the admin with no timelock of their own:

function setWithdrawalDelay(uint256 newDelay) external onlyAdmin {
  require(newDelay != 0 && newDelay <= MAX_WITHDRAWAL_DELAY, InvalidDelay());
  withdrawalDelay = newDelay;
}

function setDailyWithdrawalCap(uint256 amount) external onlyAdmin {
  require(amount != 0, ZeroAmount());
  dailyWithdrawalCap = amount;
}

A compromised admin key can execute the following sequence in a single block: set withdrawalDelay to 1 second (the minimum allowed by the != 0 check), set dailyWithdrawalCap to type(uint256).max, schedule a withdrawal via withdrawExcessNative, wait 1 second, then call executeExcessNativeWithdrawal to drain the entire native balance. This reduces the effective observation window from the intended DEFAULT_WITHDRAWAL_DELAY (1 day) to approximately 1 second, defeating the purpose of the time-delayed pattern.

The bridge should hold zero native XDC during normal operation, so the practical impact is limited to accidentally sent XDC. However, the safeguard pattern is misleading as it appears to provide a 1-day observation window when it can be reduced to near-zero.

This is inconsistent with V3, where all analogous sensitive parameter changes go through a governance delay.

Recommendation

Consider applying a timelock to changes of withdrawalDelay and dailyWithdrawalCap, or require that the existing withdrawalDelay elapses before the new delay takes effect.

6.11 Constructor seed without share mint sets an unfavorable initial exchange rate

The V3 constructor is marked payable and assigns msg.value directly to trackedTotalAssets:

constructor(
    address initialOwner,
    address previousPstXDC,
    address initialProposer,
    address initialValidator,
    uint256 initialMinStake
) payable ERC20("PrimeStakedXDC", "psXDC") ERC4626(IERC20(address(0))) {
    // ...
    trackedTotalAssets = msg.value;
    // ...
}

No shares are minted to match the seeded assets. Because the ERC4626 share-pricing formula (with a _decimalsOffset of 9) computes shares as:

shares = assets * (totalSupply + 10^9) / (totalAssets + 1)

the deployment value of msg.value directly controls the exchange rate seen by the first depositor or migrator and the same rate will be used by subsequent depositors/migrators.

If the contract is deployed with a nonzero msg.value (e.g. 10,000 XDC), totalAssets starts at 10,000e18 while totalSupply remains 0. The first migrator depositing 5,000 XDC would receive:

shares = 5000e18 * (0 + 10^9) / (10000e18 + 1) = 500,000,000 raw shares

With 27-decimal psXDC (18 base + 9 offset), 500,000,000 raw shares is effectively zero display tokens. The migrator's deposit is absorbed into the inflated totalAssets denominator, and the seeded XDC becomes unredeemable dead capital that permanently skews the exchange rate against all subsequent depositors.

Conversely, if the contract is deployed with msg.value = 0, the first migration cleanly establishes a 1:1 peg:

shares = 5000e18 * 10^9 / (0 + 1) = 5000e27 raw shares = 5,000 psXDC

All subsequent deposits then maintain the 1:1 rate until validator rewards begin accruing and naturally shift the exchange rate in favor of existing holders.

Note that the _decimalsOffset of 9 causes V3's decimals() to return 27 (18 base + 9 offset), whereas the live V2 psXDC token uses 18 decimals. While the migration math produces a 1:1 exchange rate in display terms (5000 V2 psXDC becomes 5000 V3 psXDC), the raw balanceOf values differ by a factor of 10^9. Any off-chain system, wallet integration, or block explorer that hardcodes the V2 decimal count of 18 for the new V3 token will display incorrect values.

Recommendation

Consider either deploying with msg.value = 0 so the first migration naturally establishes a 1:1 exchange rate, or minting a proportional number of initial shares to the deployer to match the seeded assets and anchor a safe base rate. If preserving the V2 token's 18-decimal representation is desired, the _decimalsOffset should be set to 0 and the constructor should both seed assets and mint a matching number of shares to establish a safe initial exchange rate that is resistant to vault inflation attacks.

6.12 No mechanism to resign or unstake masternodes through the contract

The IXDCValidator interface declares a resign(address _candidate) function, but the V3 contract has no calls to resign and exposes no wrapper for it. Capital flows out to masternodes via proposevalue: amount() but cannot be recalled through the contract:

// In IXDCValidator.sol
function resign(address _candidate) external;

// In PrimeStakedXDC_V3.sol - only propose exists, no resign
IXDCValidator(validator).propose{value: amount}(candidate);

Recommendation

Consider adding a resignMasternode(address candidate) function restricted to an appropriate role, calling IXDCValidator(validator).resign(candidate).

6.13 The OperationsManager can pause the protocol via bufferBps manipulation

The PrimeStakedXDC_V3 contract relies on a liquid buffer to fulfill user withdrawals and process the withdrawal queue. The setBufferBps(...) function allows the OperationsManager to adjust this vault's liquidity buffer target.

However, the function allows the OperationsManager to set the bufferBps up to a hardcoded maximum of 10_000, which represents 100% of the vault's assets.

function setBufferBps(uint256 newBufferBps) external onlyOperationsManager {
    // @audit-issue The Operations Manager can set the buffer to 100% (10_000)
    require(newBufferBps <= 10_000, InvalidThreshold());
    bufferBps = newBufferBps;
    emit BufferConfigUpdated(newBufferBps);
}

If the OperationsManager sets the buffer to 10_000, the desiredBuffer() function will mathematically equal the entirety of the trackedTotalAssets. Consequently, the liquidity checks inside both the _availableForImmediateWithdrawal(...) and _availableForQueuePayout(...) functions will evaluate to true, as the liquid assets can never legitimately exceed the total tracked assets.

function _availableForImmediateWithdrawal() internal view returns (uint256) {
    uint256 liquid = address(this).balance;
    uint256 keep = desiredBuffer();
    // @audit When buffer is 100%, keep >= liquid, so this always returns 0
    if (liquid <= keep) {
        return 0;
    }
    return liquid - keep;
}

Because these functions will permanently return 0, all withdrawals will revert. Although this role is intended for operational parameter adjustments, this configuration allows the OperationsManager to hold the protocol's TVL blocked. This represents a privilege escalation, bypassing the need for the DEFAULT_ADMIN_ROLE to execute a system-wide pause.

Recommendation

Consider capping the bufferBps configuration.

6.14 Timelocked governance changes can be silently overwritten without cancellation events

Across both V3 and the migration bridge, timelocked setter functions allow the admin to overwrite a pending change by calling the same setter again without first cancelling the previous one. The overwrite silently replaces the pending value and resets the delay timer, with no cancellation event emitted for the replaced schedule.

In V3, all nine timelocked setters exhibit this pattern: setOperationsManager, setProposer, setRiskManager, setMigrationManager, setMigrationBridge, setMaxLossBpsPerReport, setMaxDailyLossBps, setGovernanceDelay, and scheduleOwnerTransfer. For example:

function setOperationsManager(address account) external onlyAdmin {
    require(account != address(0), AddressZero());
    pendingOperationsManager = account;
    pendingOperationsManagerExecuteAfter = block.timestamp + governanceDelay;

    emit OperationsManagerChangeScheduled(account, pendingOperationsManagerExecuteAfter);
}

In the migration bridge, withdrawExcessNative overwrites a pending scheduled withdrawal in the same way:

function withdrawExcessNative(address payable recipient, uint256 amount) external onlyAdmin {
    require(recipient != address(0), AddressZero());
    require(amount != 0, ZeroAmount());
    require(address(this).balance >= amount, InsufficientNativeBalance());

    pendingWithdrawalRecipient = recipient;
    pendingWithdrawalAmount = amount;
    pendingWithdrawalExecuteAfter = block.timestamp + withdrawalDelay;

    emit ExcessNativeWithdrawalScheduled(recipient, amount, pendingWithdrawalExecuteAfter);
}

Each of these functions has a dedicated cancel* counterpart that properly emits a cancellation event (e.g., OperationsManagerChangeCancelled, ExcessNativeWithdrawalCancelled). When the overwrite path is used instead, off-chain monitoring tools see a new scheduling event but receive no corresponding cancellation event for the previous schedule, creating an audit trail gap.

Recommendation

Consider either requiring the existing pending change to be cancelled before scheduling a new one, or emitting a cancellation event inside the setter when overwriting existing pending state.

6.15 Users can frontrun the reportValidatorLoss(...) function to evade slashing penalties

The reportValidatorLoss(...) function from the PrimeStakedXDC_V3 contract is utilized by the risk manager to record slashing penalties or losses that occur on the validator side. When called, it reduces the trackedTotalAssets state variable, which subsequently lowers the exchange rate of the psXDC shares for all current depositors.

The contract also provides instant withdrawal capabilities through the withdraw(...) and redeem(...) functions, allowing users to immediately cash out their shares for native assets as long as there is sufficient available liquidity.

function reportValidatorLoss(uint256 assets) external onlyRiskManager {
  // ...
  // @audit-issue Decreasing the assets drops the share price for everyone.
  trackedTotalAssets -= assets;
  emit ValidatorLossReported(assets);
}

function redeem(
  uint256 shares,
  address receiver,
  address owner
)
  public
  virtual
  override
  nonReentrant
  whenMigrationFinished
  returns (uint256 assetsOut)
{
  _syncTrackedAssetsExcludingInflow(0);
  // @audit Users can instantly exit at the current, pre-slash exchange rate.
  assetsOut = super.redeem(shares, receiver, owner);
}

A user or an MEV bot can see the pending reportValidatorLoss(...) transaction and frontrun it with a redeem(...) or withdraw(...) transaction. This allows the user to exit at the pre-slashing exchange rate and avoid the penalty.

Recommendation

Consider routing all exit paths through the existing withdrawal queue. Enforcing a time delay between request and execution neutralizes frontrunning.

Resolution

Acknowledged

6.16 V3 has no dedicated emergency pause mechanism

V2 inherited ERC20PausableUpgradeable and provided pause()/ unpause() functions gated by the owner, allowing all token operations to be halted in an emergency. V3 does not include any pausability. The only flow-control mechanism is migrationInProgress, which blocks all user-facing deposit and withdrawal operations.

Since V3 is also non-upgradeable, its code cannot be patched in place if a vulnerability is discovered post-deployment.

Recommendation

Consider adding a pausability layer gated by DEFAULT_ADMIN_ROLE and applied to core user-facing functions.

6.17 setValidator and setMinStake emit no events

Every other operations manager setter in V3 emits an event upon state change: setAutoProposeConfig emits AutoProposeConfigUpdated, setBufferBps emits BufferConfigUpdated, setOperatorScanLimit emits OperatorScanLimitUpdated, setQueueScanLimit emits QueueScanLimitUpdated, and setMigrationInProgress emits MigrationModeUpdated.

However, setValidator and setMinStake are silent:

function setValidator(address newValidator) external onlyOperationsManager {
    require(newValidator != address(0), AddressZero());
    validator = newValidator;
}

function setMinStake(uint256 newMinStake) external onlyOperationsManager {
    minStake = newMinStake;
}

The validator address is particularly sensitive as it controls both where XDC is sent during masternode proposals and the KYC verification oracle ( isKYCVerified queries IXDCValidator(validator).getHashCount). A change to this address without an emitted event creates a monitoring blind spot for off-chain systems, admin dashboards, and governance watchers.

Recommendation

Consider emitting events for both setValidator and setMinStake to maintain consistent observability across all configuration changes.

6.18 Unreachable guard in _syncTrackedAssetsExcludingInflow

The _syncTrackedAssetsExcludingInflow function contains an early return guard checking liquid < inflow that can never evaluate to true in any reachable call path.

function _syncTrackedAssetsExcludingInflow(uint256 inflow) internal {
  uint256 liquid = address(this).balance;
  if (liquid < inflow) {
    return;
  }
  // ...
}

When inflow is msg.value (the stake and depositNative paths), the EVM credits msg.value to the contract's balance before the function body executes, so address(this).balance always includes msg.value and is therefore always greater than or equal to it. No external calls occur between balance crediting and this check. All other callers pass inflow = 0, making the comparison liquid < 0 which is trivially false for a uint256.

Recommendation

Consider removing the unreachable branch to reduce bytecode size and improve readability.

6.19 executeOwnerTransfer to self permanently locks admin role

In both V3 and the migration bridge, the executeOwnerTransfer function grants the admin role to the new owner and then revokes it from the caller:

function executeOwnerTransfer() external onlyAdmin {
  address newOwner = pendingOwner;
  require(newOwner != address(0), NoPendingChange());
  require(block.timestamp >= pendingOwnerExecuteAfter, ChangeNotReady());

  pendingOwner = address(0);
  pendingOwnerExecuteAfter = 0;

  _grantRole(DEFAULT_ADMIN_ROLE, newOwner);
  _revokeRole(DEFAULT_ADMIN_ROLE, _msgSender());
}

If an admin schedules an ownership transfer to their own address and later executes it, _grantRole is a no-op (OpenZeppelin's _grantRole returns false when the account already holds the role) while _revokeRole succeeds and removes it. Because grantRole, revokeRole, and renounceRole are all overridden to unconditionally revert, the admin role becomes permanently irrecoverable.

The same pattern exists in the bridge. The consequence is a permanent loss of all administrative capabilities on the affected contract, including role management, governance parameter changes, and owner transfers.

Recommendation

Consider adding a check that the new owner is not the caller, such as require(newOwner != _msgSender()).

6.20 Constructor does not enforce a non-zero seed at deployment

The PrimeStakedXDC_V3 constructor is payable and seeds trackedTotalAssets with msg.value, but only mints matching shares when the seed is non-zero. There is no enforcement that the deployer actually provides a seed:

trackedTotalAssets = msg.value;
if (msg.value != 0) {
    _mint(initialOwner, msg.value);
}

_decimalsOffset() returns 0 (matching V2's 18-decimal psXDC), which means the only inflation-attack defense remaining inside _convertToShares is OpenZeppelin's +1 virtual offset on both totalSupply() and totalAssets(). OpenZeppelin's own NatSpec on ERC4626.sol (v5.2.0) documents this trade-off: _"While not fully preventing the attack, analysis shows that the default offset (0) makes it non-profitable even if an attacker is able to capture value from multiple user deposits, as a result of the value being captured by the virtual shares (out of the attacker's donation) matching the attacker's expected gains."_ OpenZeppelin therefore considers offset 0 an acceptable choice when developers have other reasons (e.g. preserving a particular decimals() value) to use it, but explicitly notes that the attack is not fully prevented.

When the contract is deployed with msg.value == 0, the protocol enters a state where tracked = 0 and supply = 0. Until the migration manager calls setMigrationInProgress(true), both stake() and the unprivileged receive() path remain open ( _syncTrackedAssetsExcludingInflow only reverts under whenMigrationFinished). In that pre-migration window the first depositor establishes the share rate and a malicious first depositor can perform the textbook ERC-4626 inflation pattern: stake 1 wei to mint 1 share, force a donation into address(this).balance (either through a SELFDESTRUCT of a helper contract or via receive() with msg.value >= MIN_REWARD_DISTRIBUTION), and let the next state-changing call fold the donation into trackedTotalAssets. Subsequent small deposits where V < D/2 + 1 then floor to zero shares because previewDeposit(V) = V × 2 / (D + 2) rounds down. The victim's native is added to trackedTotalAssets but no shares are minted in return.

The practical exploitability of this configuration is limited. Even though the attacker is not directly profitable in the typical case, every victim that does land below the rounding threshold loses their entire deposit, so the configuration is unsafe to ship as a deployment default. The constructor not enforcing a minimum seed is the underlying defect; everything downstream is operational risk.

Recommendation

Consider enforcing a non-zero seed at construction time so that totalSupply() and trackedTotalAssets are bounded below by a meaningful floor from block 0 regardless of deployer behaviour.

Resolution

Acknowledged

6.21 redeemWithQueue and withdrawWithQueue use all-or-nothing logic, queueing the entire request even when partial immediate liquidity exists

redeemWithQueue and withdrawWithQueue either redeem the full amount immediately or queue the full amount; there is no partial-fill path. When a request exceeds maxRedeem(owner) (or _availableForImmediateWithdrawal()), the entire request is queued, even if a meaningful fraction could have been satisfied immediately:

function redeemWithQueue(uint256 shares, address receiver)
    external nonReentrant
    returns (bool queued, uint256 requestId, uint256 assetsOut)
{
    require(shares != 0, ZeroShares());
    _syncTrackedAssetsExcludingInflow(0);
    address owner = _msgSender();

    if (shares <= maxRedeem(owner)) {
        assetsOut = super.redeem(shares, receiver, owner);
        return (false, 0, assetsOut);
    }

    assetsOut = previewRedeem(shares);
    requestId = _queueWithdrawal(owner, receiver, shares, assetsOut);
    return (true, requestId, 0);
}

If a user holding 100M shares calls redeemWithQueue(100M, receiver) while the protocol has 50M XDC of immediate liquidity, maxRedeem(owner) = 50M, the check 100M <= 50M fails, and all 100M shares are escrowed in address(this) with totalPendingQueuedAssets += 100M. The user receives zero XDC immediately even though 50M was sitting in the contract.

A side effect compounds the cost: _availableForImmediateWithdrawal() subtracts totalPendingQueuedAssets from address(this).balance (clamping to zero when the subtraction would underflow), so after the queue it evaluates to 0 for everyone. Other users can no longer redeem immediately and are pushed into the queue as well, even though 50M of native is still on hand.

A user wishing to combine immediate and queued redemption today must do it manually with two transactions: first call redeem(maxRedeem(owner), receiver, owner), then redeemWithQueue(remaining, receiver). withdrawWithQueue exhibits the identical pattern.

Recommendation

Consider redesigning redeemWithQueue and withdrawWithQueue so that the maximum currently-available portion is redeemed immediately and only the residual is enqueued, rather than treating each request as indivisible. This would give users prompt access to whatever liquidity exists, reduce the locked-up totalPendingQueuedAssets so other users keep some immediate-withdrawal capacity, and keep the queue head smaller and more likely to fit on the next processing pass.

Resolution

Acknowledged