Contracts Not Well Designed for Upgrades
Inconsistent Storage Layout\\nThe Contracts introduce some buffer space in the storage layout to cope with the scenarios where new storage variables can be added if a need exists to upgrade the contracts to a newer version. This helps in reducing the chances of potential storage collisions. However, the storage layout concerning the buffer space is inconsistent, and multiple variations have been observed.\\n`PauseManager`, `RateLimitter`, and `MessageServiceBase` adds a buffer space of 10, contrary to other contracts which define the space as 50.\\n```\\nuint256[10] private \\_gap;\\n```\\n\\n```\\nuint256[10] private \\_gap;\\n```\\n\\n```\\nuint256[10] private \\_\\_base\\_gap;\\n```\\n\\n`L2MessageService` defines the buffer space prior to its existing storage variables.\\n```\\nuint256[50] private \\_\\_gap\\_L2MessageService;\\n```\\n\\nIf there exists a need to inherit from this contract in the future, the derived contract has to define the buffer space first, similar to `L2MessageService`. If it doesn't, `L2MessageService` can't have more storage variables. If it adds them, it will collide with the derived contract's storage slots.\\n2. `RateLimiter` and `MessageServiceBase` initializes values without the modifier `onlyInitializing`\\n```\\nfunction \\_\\_RateLimiter\\_init(uint256 \\_periodInSeconds, uint256 \\_limitInWei) internal {\\n```\\n\\n```\\nfunction \\_init\\_MessageServiceBase(address \\_messageService, address \\_remoteSender) internal {\\n```\\n\\nThe modifier `onlyInitializing` makes sure that the function should only be invoked by a function marked as `initializer`. However, it is absent here, which means these are normal internal functions that can be utilized in any other function, thus opening opportunities for errors.
Define a consistent storage layout. Consider a positive number `n` for the number of buffer space slots, such that, it is equal to any arbitrary number `d - No. of occupied storage slots`. For instance, if the arbitrary number is 50, and the contract has 20 occupied storage slots, the buffer space can be 50-20 = 30. It will maintain a consistent storage layout throughout the inheritance hierarchy.\\nFollow a consistent approach to defining buffer space. Currently, all the contracts, define the buffer space after their occupied storage slots, so it should be maintained in the `L2MessageService` as well.\\nDefine functions `__RateLimiter_init` and `_init_MessageServiceBase` as `onlyInitializing`.
```\\nuint256[10] private \\_gap;\\n```\\n