Literals with many digits are difficult to read and review.

Too Many Digits

The order of external call/transfer, storage manipulation and event emission must follow the check-effect-interaction pattern.

Check Effect Interaction Pattern Violated

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

Variables That Could Be Declared As `constant`

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

Improper Usage Of `public` And `external` Type

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 linked `for` loops execute external calls inside a loop, which can lead to high gas costs. 

If a user calls the `stake()`, `unstake()`, or `withdrawYield()` functions, this will call the `updateYield()` function. They will have to update each user's yields, instead of just their own. 

External Calls Inside A loop

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

Initial Token Distribution

The comparison between `isStaking[msg.sender] = true` in the `require` statement is faulty. The comparison will always return `true` no matter the value stored in `isStaking[msg.sender]`.

Misuse Of A Boolean Constant

Once the `Vault.sol` has run out of emission tokens, users will not be able to call `stake()`, `unstake()`, or `withdrawYield()`. Each of these functions will follow these steps:
  1. `updateYield()`, which then calls
  2. `vault.sendToFarm()`, which then calls
  3. `calculateEmissions()`, which will set state variable `emissions = 0` if the vault has no more emission tokens
  4.  Which causes `sendToFarm()` to revert on the require statement: `require(emissions > 0 , "Nothing to withdraw" );`

This will lock users out of removing their tokens from staking and out of withdrawing their yield.

Unable To Call `stake()`, `unstake()` or `withdrawYield()` Once Emissions Run Out

Potential Reentrancy Risks

Address should be checked before assignment to make sure it is not zero addresses. If either variable `tokenAddress` or `vaultAddress` are set to a zero address, neither `Farm.sol` or `Vault.sol` will work properly.

Missing Zero Address Validation

Boolean constants can be used directly and do not need to be compared to `true` or `false`.

Boolean Equality

Before the state variable `farmPercent` is assigned a value, the code checks: 
- `if(isFarmActive(farmAddr) == true)`.

Then `farmPercent` is assigned the result of function call `getActiveFarmPercents(farmAddr)`, which checks:
- `if(isFarmActive(farmAddr) == true)` again.

Checking this parameter twice leads to more gas usage in the contract.

Unnecessary Function Call `getActiveFarmPercents()`

In the contract, `Vault`, 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. The hacker could: 
- delete all pre-existing farms
- create new farms will malicious tokens
- manipulate yield allocation
- lock users out of staking, unstaking and withdrawing their yield. 

\
If the pre-existing farms are deleted using function `killActiveFarms()`, users will be locked out of unstaking tokens and withdrawing yield. Calling `stake()`, `unstake()` or `withdrawYield()` will eventually call the function `caclulateUserTotalYield()` in the `Farm.sol` contract. In the function `caclulateUserTotalYield()`, the line `uint256 farmSecondsPerToken = vault.getFarmSecondsPerToken(address(this));` will return 0 because the farm does not exist. The proceeding line will divide by 0 and revert.
``` solidity=141
  function calculateUserTotalYield(address staker) public view returns(uint256) {
      uint256 secondsPassed = calculateYieldTime() * 10**18;
      uint256 stakingPercent = userStakingPercent(staker);
      uint256 farmSecondsPerToken = vault.getFarmSecondsPerToken(address(this));
      uint256 newYield = (stakingPercent * secondsPassed) / (farmSecondsPerToken);///How much yield does this staker receive 
      uint256 totalYield = crownYield[staker] + newYield;
      return totalYield;
  } 
```


\
If the farm address in the vault is changed to a different address, users will be unable to call `stake()`, `unstake()` or `withdrawYield()`. The following functions follow these steps:

1. `updateYield()`, which then calls
2. `calculateUserTotalYield(staker)`, which sets
3. `farmSecondsPerToken = vault.getFarmSecondsPerToken(address(this))`, which will return 0 because the address of this farm is no longer considered an active farm by the vault
4. The next line `uint256 newYield = (stakingPercent * secondsPassed) / (farmSecondsPerToken)` will revert due to division by 0.

![](https://accelerator-tasks-prod.s3.amazonaws.com/28875e4c1a584612ba88024f034ca0e0/Vault-_owner.svg)

Centralization Risk in `Vault.sol`

Setting the state variable `percent = 0` when initializing a farm will revert calls to `stake()`, `unstake()`, and `withdrawYield()`. Each of these functions will follow these steps:
  1. `updateYield()`, which then calls
  2. `calculateUserTotalYield(staker)`, which then calls
  3. `vault.getFarmSecondsPerToken(address(this))`, which will set state variable `uint256 farmPercent = 0`
  4.  Which finally causes the next line `farmSecondsPerToken = (secondsPerToken*100)/farmPercent;` to revert due to division by 0.

This can lock users from unstacking their tokens and from withdrawing yield if a farm is set to produce 0% of the token emissions.

Also note the state variable `percent`, of type `uint256` is always greater than zero. The check:

```solidity
require(percent >= 0);
```
is redundant. 

Function Revert If `Percent` Is Set To Zero

The linked variables are declared but never used.

Unused Variable

There should always be events emitted in the sensitive functions that are controlled by centralization roles.

Missing Emit Events

The functions which are never called internally within the contract should have external visibility for gas optimization.

Function Should Be Declared External

Comparisons that are always true may be unnecessary.

The linked statements compare a `uint256` variable to be greater than or equal to 0. These statements will always return true because unsigned integers cannot be less than 0.

Tautology 

Crown Capital Dao

Farm.sol

CrownToken.sol

Vault.sol

Crown Capital DAO 

Binance CEO, Coinbase exec feature in Masterclass crypto crash-course

Binance.US Cleared to Buy Voyager Accounts Despite SEC Opposition - WSJ

Bitcoin Price and Ethereum Down 5% amid China Lockdown - Time to Buy?

U.S. made ‘marginal’ gains in internet freedom, report finds

Binance proof-of-reserves audit shows BTC is overcollaterised

CryptoQuant gives Binance clean bill of health

Ordinals Project Launches Enabling NFTs Directly On Bitcoin

Could Elon Musk Make Twitter A Web3.0 Company?

Binance Cuts Ties with WazirX, Cancels Its Wallet Service

Ethereum (ETH), Algorand (ALGO) Try to Cover Losses; Chronoly.io (CRNO) Increases Gains With Triple Digit Growth | NewsBTC

TrueFi (TRU) up 64%, Here's Why and What Binance Had to Do With It

Paradigm funds auditing DAO Code4rena with $6 million token purchase

Bitcoin Price and Ethereum on the Rise, Maker Price Storms 18%

Bitcoin Price and Ethereum Trade Sideways, Huobi Token Storms 58%

13% of Whole Ethereum (ETH) Supply Staked, But There's Serious Problem with Decentralization

Stacks (STX) surges as Bitcoin NFT hype grows, but its blockchain activity raises concern read full article at worldnews365.me

Terra Founder Do Kwon Reportedly Flees Singapore After Arrest Warrant

Australia Could Soon Regulate Stablecoins and the Digital Yuan - DailyCoin

The most influential CEOs in markets today - MarketWatch

Jake Yocom-Piatt, Co-Founder of Decred, on Blockchain Fixing Political Voting, Campaign Spending and Privacy | Ep. 182

Why Orbeon Protocol (ORBN), Bitcoin (BTC), and Ethereum (ETH) Are Market Leaders | NewsBTC

Want To Know How CertiK Performs An Audit?

Want To Secure Your Wallet, Exchange or Dapp? Check Out Our Web 3 Pentesting

Crown Capital DAO

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

19

0

0

1

18

Products & Technology

Bug Bounty

Smart Contract Audit

SkyTrace

L1 Chain Audit

Incident Response

Skynet

KYC

Advisory Services

Penetration Testing

Sky Harbor

Formal Verification

Featured Ecosystems

BNB Chain

Ethereum

Avalanche

Solana

Algorand

NEAR

Cosmos

Polygon

Aptos

Company

About

Testimonials

Partnership

We're Hiring

Disclaimer

Resources

Crown Capital DAO

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

19

0

0

1

18