6.1 Staking on behalf should source VeVana tokens from the Stake owner's account
VeVana tokens can be staked by anyone on behalf of another user. These VeVana tokens should be sourced from the stake owner's account and not from the caller of the staking function. In the below two functions, the caller is creating a stake on behalf of stakeOwner. Hence, the stake amount should be sourced from the stakeOwner's account.
function createStakeOnBehalf(
uint256 dlpId,
address stakeOwner
) external payable override nonReentrant whenNotPaused {
dlpRootEpoch.createEpochs();
_createStake(stakeOwner, dlpId, msg.value, block.number, false);//@audit
}
function createStakeOnBehalfWithVeVANA(
uint256 dlpId,
address stakeOwner,
uint256 amount
) external override nonReentrant whenNotPaused {
dlpRootEpoch.createEpochs();
_createStake(stakeOwner, dlpId, amount, block.number, false);//@audit
}
While stakeOwner was passed as a parameter to _createStake function below, at the time of sourcing the VeVana tokens, the logic is actually sourcing the funds from _msgSender() account instead of the stakeOwner's account.
function _createStake(address stakerAddress, uint256 dlpId,
uint256 amount, uint256 startBlock,
bool isRestake) internal {
//...
if (!isRestake) {
if (msg.value > 0) {
dlpRootStakesTreasury.depositVana{value: msg.value}();//@audit
} else {
IVeVANA veVANA = dlpRootStakesTreasury.veVANA();//@audit
veVANA.safeTransferFrom(
==> _msgSender(),
address(dlpRootStakesTreasury),
amount
);
}
}
//...
}
As a result, the tokens of the caller will get locked in the stake instead of the stake owner.
Recommendation
Take approval from stakeOwner to spend the tokens and pass stakeOwner in safeTransferFrom function. This will also prevent a case where the caller executes a staking on behalf and ends up locking his tokens while the stakeOwner did not deposit the amount into the caller's account.
Resolution
The intention of the function is to allow anyone to create a stake in favor of another user using the function caller's tokens. It is like gifting tokens to another user.
6.2 migrateStake function will not work as expected
The migrateStake function of DLPRootImplementation allows the owner to migrate the staked amount from one DLP to another DLP. The process closes the current stake and creates a new stake with the new DLP specified as a parameter.
function migrateStake(
uint256 stakeId,
uint256 newDlpId,
uint256 newAmount
) external override nonReentrant whenNotPaused {
dlpRootEpoch.createEpochs();
Stake storage stake = _stakes[stakeId];
==> if (newDlpId != stake.dlpId) {
revert InvalidDlpId();
}
if (newAmount > stake.amount) {
revert InvalidStakeAmount();
}
stake.movedAmount = newAmount;
_closeStake(_msgSender(), stakeId);
_createStake(_msgSender(), newDlpId, newAmount, stake.startBlock, true);
emit StakeMigrated(stakeId, stakesCount, newDlpId, newAmount);
}
However, the validation of newDlpId expects it to be the same as the stake’s dlpId, which defeats the purpose of passing newDlpId as a parameter.
As a result, in the current implementation, the migrateStake function will revert if newDlpId is not the same as the current stake's dlpId.
Recommendation
Review the condition for DlpId validation.
Resolution
Yes, it is for restaking to another Dlp. However, we block the restaking to a different Dlp currently and only allow restaking to the same Dlp (which is equivalent to partial unstaking).
6.3 Use Beacon Proxy pattern to keep Rewards and Stakes proxies in Sync
DLPRootTreasuryImplementation is an implementation contract that is used by two proxies, one for Rewards and the other for Stakes. In the case of proxy patterns, proxy contracts are the owners of the storage data, and hence each proxy will have their storage.
abstract contract DLPRootTreasuryStorageV1 is IDLPRootTreasury {
IDLPRoot public override dlpRoot;
IVeVANA public override veVANA;//@audit
}
Hence, every time veVana is updated with a new address, it should be updated for both instances of the proxies.
Recommendation
Beacon Proxy is a smart contract upgrade pattern where multiple proxies use the same implementation contract, and all the proxies can be upgraded in a single transaction.
Resolution
Will consider the recommendation.
