In the contract, `SpinStakable`, the role, `_owner`, has authority over the functions shown in the diagram below.

Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and [fixme, describe what hacker can do and the impact].

![](https://accelerator-tasks-prod.s3.amazonaws.com/7e03eb62165141f3bf7e65cf73faa036/diagrams/centralization-SpinStakable-_owner.svg)

Centralization Risk in SpinStakable.sol

There is no check in the function `recoverERC20()` to ensure that enough `rewardsToken` balance is maintained, meaning a user may be unable to gain their allocated rewards.

Missing Check for `rewardsToken`

If both the `rewardsToken` and `stakingToken` reference the same token, then calling the `balanceOf()` function would return the amount staked and allocated for rewards. This could potentially lead to issues by allowing higher a `rewardRate` than expected.

`rewardRate` Overflow

When calling the function `getReward()`, the source of the `rewardToken` is unknown for how the contract will guarantee user rewards, as no automatic token transfer is made.

Unknown Source of `rewardsToken`  Balance

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user stakes 100 deflationary tokens (with a 10% transaction fee) in a MasterChef, only 90 tokens actually arrived in the contract. However, the user can still withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. 

The MasterChef takes the pool token balance(the `lpSupply`) into account when calculating the users' reward. An attacker can repeat the process of deposit and withdraw to lower the token balance(`lpSupply`) in a deflationary token pool and cause the contract to increase the reward amount. 

Reference:
https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f

Incompatibility With Deflationary Tokens (Staking)

Solidity version >=0.8.0 includes checked arithmetic operations and underflow/overflow by default, making SafeMath redundant.  

Redundant SafeMath Usage

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

Missing error messages

Spintop Staking

SpinStakable.sol

Staking

contracts

In the contract `SpintopToken`, the role `_owner` has the authority over the following functions:
- `addAddressToBlacklist()`
- `addAddressesToBlacklist()`
- `removeAddressFromBlacklist()`
- `removeAddressesFromBlacklist()`
- `burn()`
- `pauseContract()`
- `unpauseContract()`
- `mint()`


In the contract `FarmSPIN`, the role `_owner` has the authority over the following functions:
- `addContractBalance()`
- `disburseRewardTokens()`
- `burnRewardToken()`
- `transferAnyERC20Token()`
- `transferAnyOldERC20Token()`

Any compromise to the `_owner` account may allow the hacker to take advantage of these functions.

Centralization Risk

The variables `contractDeployTime` and `adminClaimableTime` assigned in the constructor can declare with `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since will not be stored in storage. Still, values will directly insert the values into the runtime code.

Declare Variable as Immutable

The linked functions invocations do not check the return value of the function call which should yield a value in case of a proper call.

```solidity
holders.add(msg.sender);
holders.remove(msg.sender);
holders.remove(msg.sender);
pancakeswapRouterV2.swapExactTokensForTokens(_tokensToBeSwapped, amountOutMin, SWAP_PATH, address(this), block.timestamp);
```

Return Value Ignored

In the function `getDepositorList()`, there is no check to see if the current index `i` has exceeded the size of `holders`. The function will revert when querying an end index out of bounds.

Missing Check For Exceeding AddressSet Size

The contract is serving as the underlying entity to interact with third party PancakeSwap protocols. The scope of the audit treats 3rd party entities as black boxes and assume their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, etc.

Third Party Dependencies

The functions that affects the status of sensitive variables should be able to emit events as notifications to alert listeners of changes.

State variable `totalTokens`
- `deposit()`
- `withdraw()`
- `emergencyWithdraw()`

State variable `contractBalance`
- `addContractBalance()`
- `disburseTokens()`

Missing Emit Events

The value `tokensToBeSwapped.sub(diff)` is equal to `maxSwappableAmount` which can be assigned to `_tokensToBeSwapped`, saving gas. 

Redundant Mathematical Operation

In the following solidity code:
```solidity=1151
uint pendingDisburse = disburseAmount
                            .mul(disbursePercentX100)
                            .mul(timeDiff)
                            .div(disburseDuration)
                            .div(10000);
```
The mathematical operations `.mul(disbursePercentX100)` and `.div(10000)` cancel each other's action because the constant `disbursePercentX100` is set to `100e2` or `10000`.

Canceling Mathematical Operation.

The functions `increaseAllowance()` and `decreaseAllowance()` do not have the modifier `isNotBlacklisted` which is included on the function `approve()`. By calling functions `increaseAllowance()` or `decreaseAllowance()`, a blacklisted user can bypass the `approve()` function, enabling transferring their tokens.

Missing Function Modifier

The internal state variables `_hardCap`, `_nextUnlockAt`, and `_lastUnlockAt` are initialized but never utilized outside of this.

Variable Not Utilized

State variables `_mintingAmount1`, `_mintingAmount2`, `_mintingAmount3`, `_mintingAmount4`, `_mintingAmount5`, `_mintingAmount6`, `address1`, `address2`, `address3`, `address4`, `address5`, `address6` could be declared as `constant` since these state variables are never to be changed.

Declare Variable `constant`

The following functions are declared as `public`, contain array function arguments, and are not invoked in any of the contracts contained within the project's scope. The functions that are never called internally within the contract should have external visibility. 

Spintop Network

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

7

0

7

0

0