Contract state for staked positions from S1 and S2 citizens are represented in part using arrays, which are iterated in different functions that are part of the NeoTokyoStaker contract. Given these arrays are unbounded, traversing these may eventually lead to a denial of service issue due to an unbounded usage of gas.
This function iterates both _stakerS1Position and _stakerS2Position, and copies all the values from the items to calculate a summary of the positions of a particular staker.
Iterates _stakerS1Position or _stakerS2Position based on the asset type to calculate the sum of all the points from the stake positions. For each element in the array, the function will fetch the full corresponding stake struct from storage and copy it to memory (lines 1281 and 1290).
These withdraw actions loop over the corresponding positions array to find the index of the item that needs to be removed, and eventually swap it for the last element and pop the array.
A different strategy can be applied based on the case,
getStakerPositions: since this is an external view, using a pagination with an offset and a length should be enough. This will also need a way to know the length in advance, which should be exposed with another function (i.e.getStakerPositionsLength).getPoolReward: instead of calculating the sum on the fly, have a precalculated counter by staker and asset type that is updated whenever the user stakes or unstakes._withdrawS1Citizenand_withdrawS2Citizen: have an additional mapping that tracks the position of eachcitizenIdin each array. For example,mapping(uint256 => uint256) _stakerS1PositionIndiciesthat associates thecitizenId(mapping key) to the index of the corresponding position in the_stakerS1Positionarray. Removing an item can then be implemented by querying this mapping to find the element that needs to be removed from the array, without looping.