Submission Details
Severity:
Multiple cred transfers for a single unstake
Summary
The unstake() function contains inefficient logic when sending rewards to the user, resulting in multiple transfer events being emitted.
Vulnerability Details
In the unstake() function, the user is sent 1 cred token for each day in staking, up to a maximum of 4. This is calculated day by day, so if they have been staking for 4 days, 4 transactions will be emitted.
In this test, we verify that a single unstake() generates 4 cred token transfer events, along with an unstake event and an NFT transfer event.
function testUnstakeEmitsMultipleEvents() public mintRapper {
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(4 days + 1);
vm.recordLogs();
streets.unstake(0);
Vm.Log[] memory entries = vm.getRecordedLogs();
bytes32 sigUnstaked = bytes32(keccak256("Unstaked(address,uint256,uint256)"));
bytes32 sigTransfer = bytes32(keccak256("Transfer(address,address,uint256)"));
assertEq(entries.length, 6);
assertEq(entries[0].topics[0], sigUnstaked); // unstaked
assertEq(entries[1].topics[0], sigTransfer); // cred transfer
assertEq(entries[2].topics[0], sigTransfer); // cred transfer
assertEq(entries[3].topics[0], sigTransfer); // cred transfer
assertEq(entries[4].topics[0], sigTransfer); // cred transfer
assertEq(entries[5].topics[0], sigTransfer); // NFT transfer
}
Test passes
Ran 1 test for test/OneShotTestAudit.t.sol:RapBattleTest
[PASS] testUnstakeEmitsMultipleEvents() (gas: 255720)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.49ms
Impact
There is an great inefficiency in gas usage, and there will be duplicity in the information communicated by the events.
Tools Used
Foundry, Manual review
Recommendations
Calculate the amount and send it all in one transfer:
function unstake(uint256 tokenId) external {
require(stakes[tokenId].owner == msg.sender, "Not the token owner");
uint256 stakedDuration = block.timestamp - stakes[tokenId].startTime;
uint256 daysStaked = stakedDuration / 1 days;
// Assuming RapBattle contract has a function to update metadata properties
- IOneShot.RapperStats memory stakedRapperStats = oneShotContract.getRapperStats(tokenId);
emit Unstaked(msg.sender, tokenId, stakedDuration);
delete stakes[tokenId]; // Clear staking info
+ // Only call the update function if the token was staked for at least one day
+ if (daysStaked >= 1) {
+ IOneShot.RapperStats memory stakedRapperStats = oneShotContract.getRapperStats(tokenId);
// Apply changes based on the days staked
if (daysStaked >= 1) {
stakedRapperStats.weakKnees = false;
- credContract.mint(msg.sender, 1);
}
if (daysStaked >= 2) {
stakedRapperStats.heavyArms = false;
- credContract.mint(msg.sender, 1);
}
if (daysStaked >= 3) {
stakedRapperStats.spaghettiSweater = false;
- credContract.mint(msg.sender, 1);
}
if (daysStaked >= 4) {
stakedRapperStats.calmAndReady = true;
- credContract.mint(msg.sender, 1);
}
- // Only call the update function if the token was staked for at least one day
- if (daysStaked >= 1) {
+ uint256 credToMint = daysStaked <= 4 ? daysStaked : 4;
+ credContract.mint(msg.sender, credToMint);
oneShotContract.updateRapperStats(
tokenId,
stakedRapperStats.weakKnees,
stakedRapperStats.heavyArms,
stakedRapperStats.spaghettiSweater,
stakedRapperStats.calmAndReady,
stakedRapperStats.battlesWon
);
}
// Continue with unstaking logic (e.g., transferring the token back to the owner)
oneShotContract.transferFrom(address(this), msg.sender, tokenId);
}```
Now only 1 cred token transfer event is executed, in addition to 1 unstake event and 1 NFT transfer event.
Modify the previous test:
```javascript
function testUnstakeEmitsMultipleEvents() public mintRapper {
vm.startPrank(user);
oneShot.approve(address(streets), 0);
streets.stake(0);
vm.warp(4 days + 1);
vm.recordLogs();
streets.unstake(0);
Vm.Log[] memory entries = vm.getRecordedLogs();
bytes32 sigUnstaked = bytes32(keccak256("Unstaked(address,uint256,uint256)"));
bytes32 sigTransfer = bytes32(keccak256("Transfer(address,address,uint256)"));
assertEq(entries.length, 3);
assertEq(entries[0].topics[0], sigUnstaked);
assertEq(entries[1].topics[0], sigTransfer);
assertEq(entries[2].topics[0], sigTransfer);
}
Ran 1 test for test/OneShotTestAudit.t.sol:RapBattleTest
[PASS] testUnstakeEmitsMultipleEvents() (gas: 242814)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.12ms
Rewards breakdown
nSLOC
201This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.