During staking, the error message in `require (_msgSender() == _playerContract, "must unstake through player contract");` does not describe the error correctly.

Incorrect Error Message

The internal `_checkEpoch()` updates the `currentEpoch` number according to the current time and is called in each player operation. 
`_epochStart` is a uint to uint mapping that returns 0 by default that may lead to `currentEpoch` being increased in every call if the next epoch had not been added.

Logical Issue In `_checkEpoch()`

In the contract `MWStaking`, the role `_multiSig` has the authority over the following function:
- `togglePlayersAllowed()`
- `toggleStaking()`
- `setPlayerContract()`
- `addEpochs()`
- `failsafeETHtransfer()`
Any compromise to the `_multiSig` account may allow the hacker to take advantage of this and
- enable anyone to register themselves as players through `togglePlayersAllowed()`
- change the `stakingEnabled` variable through `toggleStaking()`
- set the player contract through `setPlayerContract()`
- add weeklong epochs up to twenty through `addEpochs()`
- take ETH on this contract through `failsafeETHtransfer()`

The role `_epochRobot` has the authority over the following function:
- `setPoolSize()`
Any compromise to the `_multiSig` account may allow the hacker to take advantage of this and
- set pool size of the current epoch through `setPoolSize()`

The role `_playerContract` has the authority over the following function:
- `sync()`
- `stake()`
- `unstake()`
- `claimPlayer()`
Any compromise to the `_playerContract` account may allow the hacker to take advantage of this and
- adjust a player‘s staking to any uint value through `sync()`, `stake()` or `unstake()`
- take ETH from the contract for a player through `claimPlayer()`

Centralization Risk in `MWStaking`

If the robot does not set the pool size correctly, users' cumulative stake may exceed the pool size thus users' total share exceeds 100% and the epoch reward may not be sufficient to pay all staking non-players.
Similarly, there is no restriction on the input amounts, which seems to allow the player to claim any amount of ETH they want on this contract and the balance may not be enough for the following claims. 

Cumulative `weiToPay` May Exceed Available Reward

The boolean `stakingEnabled` is controlled by the multiSig account and appears to be a flag that enables or disables staking in the `WEAPON` contract yet it is unused anywhere else in `MWStaking`. Likewise, the `stakeSignatureRequired` variable is not used in the contract.

Unused Variables

`_epochStart` is a uint to uint mapping that returns zero by default. During setting, `_epochStart[firstAdd + i - 1]` should be checked to be greater than 0, otherwise, if it had not been correctly set, the next `_epochStart` time would set to `604000` in 1970, which does not make sense.

Lack of Validation During `addEpochs()`

The reward has been sent to player's account but the the record is kept for `_msgSender()` which is the player contract. This would allow only the first player to claim reward in a single epoch.

Logical Issue In `claimPlayer()`

The return value `data` is declared but never used in the function body.

Unused Return Value

The current implementation only increases the `currentEpoch` by one in each call which may not have set it to the correct number if the last call had been over an epoch ago.

Updating Epoch through `_checkEpoch()`

`public` functions that are never called by the contract could be declared as `external`. `external` functions are more efficient than `public` functions.

Function Visibility Optimization

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

The linked variables assigned in the constructor can be declared as `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 they will not be stored in storage.

Variables That Could Be Declared as `immutable`

The given input is missing the check for the non-zero address.

Missing Input Validation

A registered player's `_stakeBalances` may be adjusted to an input value through `stake()`, `unstake()` and `sync()` without transferring `WEAPON`.

`_stakeBalances` Can Be Adjusted Without Balance Change

Comparison to A Boolean Constant

The linked and other magic numbers should be set as `constant` and `internal` contract-level variables with a self-explanatory variable name as well as accompanying comments when necessary.

Magic Numbers

The linked functions alters critical parameters on the contract but there are no reverse methods to correct potential mishandling.

Lack of Reverse Methods

In the contract, `WEAPON`, the role, `_multiSig`, has the authority over the functions shown in the diagram below.

Any compromise to the privileged account which has access to `_multiSig` may allow the hacker to take advantage of this and
- disable minting indefinitely after lock period through `freezeMint()` when `mintLockTime` has passed
- enable minting after lock period through `thawMint()`
- mint `MegaWeapon` on any account through `mint()`
- enable or disable staking through `toggleStaking()`
- set the staking contract address through `setStakingContract()`
- set a new buy tax lower than the previous one through `setBuyTax()`
- set a new sell tax lower than the previous one through `setSellTax()`
- set a new reward higher than the previous one through `setRewards()`
- add new pool address through `_isPool()`
- swap any amount of `Megaweapon` tokens on this contract to ETH and send all remaining ETH on this contract to the staking contract and vault through `failsafeTokenSwap()`
- take ETH on this contract through `failsafeETHtransfer()`

![](https://accelerator-tasks-prod.s3.amazonaws.com/6b8e0b0968a1492fb2f629c7d001b7c6/WEAPON-_multiSig.svg)

In the contract, `WEAPON`, the role, `_stakingContract`, has the authority over the functions shown in the diagram below.
- adjust or increase staking through `stake()`
- adjust or decrease staking through `unstake()`
- adjust staking through `sync()`
- enable or disable staking and unstaking through `toggleStaking()`

![](https://accelerator-tasks-prod.s3.amazonaws.com/6b8e0b0968a1492fb2f629c7d001b7c6/WEAPON-_stakingContract.svg)

Centralization Risk in `WEAPON`

The linked variables could be declared as `constant` since these state variables are never modified.

Variables That Could Be Declared as `constant`

The function that affects the status of sensitive variables should be able to emit events as notifications to users.

Missing Emit Events

The linked number does not match 24 hours as stated in the error message.

Wrong Number

All of the `Megaweapon` tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute `Megaweapon` tokens without obtaining the consensus of the community.

Initial Token Distribution

In the `WEAPON` contract, only `_multiSig` and `_stakingContract` can invoke the `toggleStaking` function, but there's no implementation in the `_stakingContract` contract.

`toggleStaking()` Not Called In `_stakingContract`

The contract is serving as the underlying entity to interact with third-party DEX protocols. The scope of the audit would treat those 3rd party entities as black boxes and assume their functional correctness. However, in the real world, 3rd parties may be compromised and lead to assets being lost or stolen.

Third Party Dependencies

There are many hardcode addresses in this codebase.

Hardcode Address

A sandwich attack might happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (before the transaction being attacked) a transaction to purchase one of the assets and make profits by back running (after the transaction being attacked) a transaction to sell the asset.

Currently, a zero percent upper threshold is put on the slippage loss incurred by `UniV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens()` operations. This may revert `_transfer()` when the liquidity pool does not hold many tokens thus the circulating asset is large enough to cause a considerable slippage. An arbitrary zero percent restriction may not suit all strategies and might in effect freeze deposited tokens.

Besides, the following functions are called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large:

- `UniV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens()`

Potential Sandwich Attacks

When both the sender and recipient are true for `_isPool`, the receiving amount is only effectively taxed once but the current contract have received taxed twice, breaking the conservation of total balances without changes in the total supply.

Incorrect Balance When Both the Sender and Recipient Are Pools

If the `_stakingRewards` has been set to above 100, the staking share would exceed total ETH balance on this contract and the transaction could not proceed. This may disable users from selling.

megaweapon

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Audit History