Neo Tokyo contest - ayden's results

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1668 _assetType has only 4 distinct values 0123, If there are no modifications, When assetType is equal to 4, the code will continue to execute and failed with error " Calling a zero initialized variable of internal function type"

Delete unused variables and functions: Unused variables and functions like _withdrawS1Citizen, _withdrawS2Citizen, and _withdrawLP can be removed to reduce redundant code

-if (uint8(_assetType) == 2 || uint8(_assetType) > 4) {
+if (uint8(_assetType) == 2 || uint8(_assetType) > 3) {
	revert InvalidAssetType(uint256(_assetType));
}


function () _withdraw;
assembly {
	switch _assetType
		case 0 {
			_withdraw := _withdrawS1Citizen
		}
		case 1 {
			_withdraw := _withdrawS2Citizen
		}
		case 3 {
			_withdraw := _withdrawLP
		}
}

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1205 _assetType has only 4 distinct values 0123, If there are no modifications, When assetType is equal to 4, the code will continue to execute and failed with error " Calling a zero initialized variable of internal function type"

		- if (uint8(_assetType) > 4) {
		+ if (uint8(_assetType) > 3) {
		  	revert InvalidAssetType(uint256(_assetType));
		 }



		// Store references to each available staking function.
		- function (uint256) _s1 = _stakeS1Citizen;
		- function (uint256) _s2 = _stakeS2Citizen;
		- function (uint256) _b = _stakeBytes;
		- function (uint256) _lp = _stakeLP;

		// Select the proper staking function based on the asset type being staked.
		function (uint256) _stake;
		assembly {
			switch _assetType
				case 0 {
					_stake := _stakeS1Citizen
				}
				case 1 {
					_stake := _stakeS2Citizen
				}
				case 2 {
					_stake := _stakeBytes
				}
				case 3 {
					_stake := _stakeLP
				}
		-	    default {}
		}

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L588#L606 constructor function lack of a zero address check

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L711 address _staker lacks of a zero address check

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1410 check _recipient is a valid address

require(_recipient!=address(0),"zero address")

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1066#L1068 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1087#L1089

save 61 ~ 141 gas

- if (citizenStatus.stakedBytes + amount > cap) {
-  	 revert AmountExceedsCap(citizenStatus.stakedBytes + amount, cap);
- }

+ uint256 afterSkatedBytes;
+ unckeck{
+   afterSkatedBytes = citizenStatus.stakedBytes + amount;
+ }

+ if (afterSkatedBytes > cap) {
+  	 revert AmountExceedsCap(afterSkatedBytes, cap);
+ }

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L710#L752 for loop with a length variable instead of the for loop with the array length

	function getStakerPositions (
		address _staker
	) external view returns (StakerPosition memory) {

		// Compile the S1 Citizen details.
		StakedS1CitizenOutput[] memory stakedS1Details =
			new StakedS1CitizenOutput[](_stakerS1Position[_staker].length);
+		uint256 lenS1 = _stakerS1Position[_staker].length;
		for (uint256 i; i < lenS1; ) {
			uint256 citizenId = _stakerS1Position[_staker][i];
			StakedS1Citizen memory citizenDetails = stakedS1[_staker][citizenId];
			stakedS1Details[i] = StakedS1CitizenOutput({
				citizenId: citizenId,
				stakedBytes: citizenDetails.stakedBytes,
				timelockEndTime: citizenDetails.timelockEndTime,
				points: citizenDetails.points,
				hasVault: citizenDetails.hasVault,
				stakedVaultId: citizenDetails.stakedVaultId
			});
			unchecked { i++; }
		}

		// Compile the S2 Citizen details.
		StakedS2CitizenOutput[] memory stakedS2Details =
			new StakedS2CitizenOutput[](_stakerS2Position[_staker].length);
+		uint256 lenS2 = _stakerS2Position[_staker].length;
		for (uint256 i; i < lenS2; ) {
			uint256 citizenId = _stakerS2Position[_staker][i];
			StakedS2Citizen memory citizenDetails = stakedS2[_staker][citizenId];
			stakedS2Details[i] = StakedS2CitizenOutput({
				citizenId: citizenId,
				stakedBytes: citizenDetails.stakedBytes,
				timelockEndTime: citizenDetails.timelockEndTime,
				points: citizenDetails.points
			});
			unchecked { i++; }
		}

		// Return the final output position struct.
		return StakerPosition({
			stakedS1Citizens: stakedS1Details,
			stakedS2Citizens: stakedS2Details,
			stakedLPPosition: stakerLPPosition[_staker]
		});
	}

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1264#L1396 check AssetType is valid value first save more gas

+require(
+ _assetType == AssetType.S1_CITIZEN ||
+ _assetType == AssetType.S2_CITIZEN ||
+ _assetType == AssetType.LP,
+"Invalid asset type"
+);

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1517#L1521 Use delete instead of set the value to zero

When updating the information of stakedCitizen, the delete keyword can be used to remove stakedCitizen.stakedBytes instead of setting it to zero. Using the delete keyword can prevent data stored in the contract from being exposed to potential attackers and improve the security of the contract.

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1819#L1842 Multiple calls to _inputs[i].assetType within the for loop may result in some redundant calculations. It could be beneficial to store it in a local variable and reuse it in the code.

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L983 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1035 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1112 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1170 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1526 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1589 https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1641 immutable address is constant value after deployd and it's not necessary to emit this value

https://github.com/code-423n4/2023-03-neotokyo/blob/main/contracts/staking/NeoTokyoStaker.sol#L1834

-  if (j != 0 && _inputs[i].rewardWindows[j].startTime <= lastTime) {  //@audit split the &&
-	  revert RewardWindowTimesMustIncrease();
-  }

+  if (j != 0) {  //@audit split the &&
	  if(_inputs[i].rewardWindows[j].startTime <= lastTime){
+ 		revert RewardWindowTimesMustIncrease();
+	  }
+  }