The setUnlockParams function permits non-zero lockingPeriodBlocks even when permittedStaker == address(0).

This behavior could unintentionally impose a vesting period in open deployments where unrestricted staking and unstaking are expected. Enforcing a logical relationship between deployment settings ensures the contract operates as intended and prevents unnecessary restrictions on user actions.

Code Location

Code of setUnlockParams function from Staking.sol contract.

    function setUnlockParams(
        uint256 _unstakingStartBlock,
        uint256 _lockingPeriodBlocks
    ) external onlyOwner {
        unstakingStartBlock = _unstakingStartBlock;
        lockingPeriodBlocks = _lockingPeriodBlocks;
    }

It is recommended to add a validation in the setUnlockParams function to enforce that parameters remain consistent with the deployment mode, ensuring no unintended restrictions are applied in open deployments.

Alternatively, an enforce mechanism can be implemented in the unstake function to automatically set the value to 0 if the deployment is in open mode, preventing unnecessary locking periods.

RISK ACCEPTED: The Plume team has acknowledged and accepted the risk associated with this finding.

The constructors of both the Distributor.sol and Staking.sol contracts do not check if critical addresses (e.g., _signer, _token, _staking, _stakingToken) are zero.

Deploying contracts with zero addresses could lead to unexpected behavior or vulnerabilities, such as misdirected operations or inability to use the contract as intended.

It is recommended to add explicit checks in the constructor to revert if any of the provided addresses are zero.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

The _signatureCheck function in the Distributor.sol contract does not enforce a fixed signature length of 65 bytes, which is the required length for ECDSA signatures.

Accepting invalid or malformed signatures could lead to unexpected behavior, such as failing to validate legitimate transactions.

Code Location

Code of _signatureCheck function from Distributor.sol contract.

function _signatureCheck(
	bytes32 _messageHash,
	bytes calldata _signature
) internal view {
	if (_signature.length == 0) revert InvalidSignature();

	bytes32 prefixedHash = ECDSA.toEthSignedMessageHash(_messageHash);
	address recoveredSigner = ECDSA.recoverCalldata(
		prefixedHash,
		_signature
	);

	if (recoveredSigner != signer) revert InvalidSignature();
}

It is recommended to replace the check if (_signature.length == 0) with strict validation, enforcing the signature length to be exactly 65 bytes.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

The Staking.sol contract uses a floating pragma version (e.g., ^0.8.0) and does not match the pragma version of the Distributor.sol contract.

Floating pragmas can lead to unexpected behavior due to differences in compiler versions, and mismatched pragma versions between related contracts may introduce compatibility issues.

Code Location

Code from Staking.sol contract.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

It is recommended to set the pragma version in Staking.sol to exactly match the one in Distributor.sol (e.g., pragma solidity 0.8.25;). Avoid using floating pragmas to ensure consistency and reliability during compilation.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

In both contracts, important operations such as unlock, setSigner, withdrawTokens, setClaimRoot (in Distributor.sol) and setPermittedStaker, setUnlockParams (in Staking.sol) lack corresponding events.

This omission makes it difficult to track state changes, monitor operations, or debug the contract effectively, reducing transparency and auditability.

It is recommended to introduce events for all main state-changing operations. Emit these events with appropriate parameters to log the changes in a structured and traceable manner.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

Several functions in the Distributor.sol and Staking.sol contracts lack complete or accurate NatSpec documentation, reducing clarity and usability for developers and external tools.

Missing tags such as @notice, @dev, @param, and @return make it harder to understand the purpose, behavior, and inputs/outputs of the functions.

Affected Functions:

• In Distributor.sol:

  • unlock

  • toggleActive

• In Staking.sol:

  • constructor

  • setPermittedStaker

  • setUnlockParams

  • unstake

  • toggleActive

  • getStakeInfo

It is recommended to ensure that all public and external functions include comprehensive NatSpec documentation with all relevant tags to improve clarity, usability, and integration with automated tools.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

The logic in the stake function includes two inefficiencies:

1. Redundant Balance Comparison: The function calculates stakingAmount by comparing the contract’s token balance before and after the transfer. This approach is unnecessary and gas-inefficient. Instead, the _amount parameter can be used directly, or it can be replaced with more accurate validations, such as checking the sender's balance and allowance.

   
   

2. Unnecessary Conditional Update: The user’s stake (userStake.amount) is updated with conditional logic based on whether userStake.amount > 0. This separation is redundant, as a direct addition would achieve the same result with less complexity.

Code Location

Code of stake function from Staking.sol contract.

Stake storage userStake = stakeInfo[_beneficiary];

uint256 balanceBefore = IERC20(stakingToken).balanceOf(address(this));
IERC20(stakingToken).safeTransferFrom(
	msg.sender,
	address(this),
	_amount
);
uint256 balanceAfter = IERC20(stakingToken).balanceOf(address(this));

uint256 stakingAmount = balanceAfter - balanceBefore;

// Update user's stake
if (userStake.amount > 0) {
	userStake.amount += stakingAmount;
} else {
	userStake.amount = stakingAmount;
}

totalStaked += stakingAmount;

It is recommended to:

1. Replace the balance comparison logic with either directly using the _amount parameter or validating the sender’s balance and allowance beforehand to ensure they have sufficient tokens and approval.

   
   

2. Simplify the logic for updating userStake.amount by directly adding the staking amount to the existing value, regardless of whether it is initially zero.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

The NotPermittedStaker error is reused for various scenarios in the unstake function, including cases where _onBehalfOf is invalid.

This lack of specificity makes debugging and tracing errors more difficult.

Code Location

Code of unstake function from Staking.sol contract.

if (permittedStaker != address(0)) {
	if (permittedStaker != msg.sender) {
		revert NotPermittedStaker();
	}
} else {
	_unlockDelayReduction = 0;
	if (msg.sender != _onBehalfOf) {
		revert NotPermittedStaker();
	}
}

It is recommended to introduce specific and descriptive errors for each failure scenario, such as InvalidOnBehalfOf, to clearly differentiate the issues.

ACKNOWLEDGED: The Plume team has acknowledged this finding.

The Stake struct declared in the IStaking.sol interface contains parameters lastAccruedBlock and accruedInterest, which are not used in any logic or function in the Staking.sol contract.

Including unused parameters increases gas costs, reduces code clarity, and may mislead developers regarding their purpose.

Code Location

Code from IStaking.sol interface:

    struct Stake {
        uint256 amount; // Amount of tokens staked
        uint256 lastAccruedBlock; // Block number when stake was created/last updated
        uint256 accruedInterest;
    }

It is recommended to remove the unused parameters lastAccruedBlock and accruedInterest from the Stake struct if they are not necessary for future functionality.

ACKNOWLEDGED: The Plume team has acknowledged this finding.