Take a look at https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/StateTransitionManager.sol#L177-L228
Considering this is the logic, for setting chain upgrades, what to note here is the setting applied to the ProposedUpgrade struct, navigating here https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol#L14-L40, we can see all the necessities, attached to this struct, i.e most members of the struct are to kept as 0 in order not to be updated which is rightly, but the logic for upgradeTimestamp and newProtocolVersion is wrong.
For upgradeTimestamp, this value should be set as the timestamp after which the upgrade can be executed, i.e in the future or at least it shouldnt be hardcoded to 0.
For newProtocolVersion the version needs to be greater than the previous protocol version, but execution instead just queries the current version which is wrong, since the attempt to upgrade the version from BaseZkSyncUpgrade.sol would revert if the version is not greater than the current one.
Additionally note that this comment: Note, that the priority operation id is used as nonce for L1->L2 transactions in this case is invalid as the operation is not a PRIORITY_OPERATION_L2_TX_TYPE but rather a SYSTEM_UPGRADE_L2_TX_TYPE.
The logic of having an upgrade be scheduled in the future is non-existent, due to the hardcodes, upgrading all upgrades are automatically immediately possible contrary to protocols intention, this is also heavily dependent on the documentations around this, as generally for GenesisUpgrade, the upgradeTimestamp 0 is an ok value, as it can and should be executed immediately after chain genesis, but thats not the case for all instances
Consider allowing upgradeTimestamp to be set to a different value other than 0, additionally consider allowing protocolVersion to be a passed in value.
