synthetix-v3
synthetix-v3 copied to clipboard
Moss audit fixes
Informal audit report found below. Some of these issues were already fixed on the gov v3 branch, so all should be present on this branch, but not all will be found in the diff.
Other changes include:
- modified
GAS_LIMITfrom a constant to a storage var & added getter / setter fns - added the
SnapshotRewardsDistributor.solcontract in full. - added the ability to scale Snapshot contracts, to enable voting power from different systems and collateral to be scaled appropriately.
- added some view fns to help the frontend including:
getPreparedBallot,getSupportedNetworks,getRegisteredEmitters,hasProcessedMsg, andgetGasLimit
report:
- The
tweakEpochSchedule()function needs to be marked payable as thetransmitfunction requires payment for the cross-chain broadcast. (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModule.sol#L150) - The
evaluate()function needs to be marked payable as thetransmitfunction requires payment for the cross-chain broadcast. (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModule.sol#L379) - The
dismissMembers()function is incorrectly passing throughmsg.valueas the value to be sent cross-chain. No value should be transferred, therefore a value of0should be used. (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModule.sol#L242) - The
cast()function is incorrectly passing throughmsg.valueas the value to be sent cross-chain. No value should be transferred, therefore a value of0should be used. (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModuleSatellite.sol#L117) - The
withdrawVote()function is incorrectly passing throughmsg.valueas the value to be sent cross-chain. No value should be transferred, therefore a value of0should be used. (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModuleSatellite.sol#L122) - The
dismissMembers()function currently only transmits to the mothership chain. However, originally the function was intended to be called on the mothership chain and then transmit to all the other chains to notify that members had been dismissed. Instead of transmitting to the mothership chain, the message should be broadcasted to all supported chains (https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModule.sol#L231) - The Wormhole receiver interface is incorrect and should implement a different function signature. (incorrect: https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/utils/core-modules/contracts/interfaces/IWormholeReceiver.sol#L44, correct: https://github.com/wormhole-foundation/wormhole/blob/d146f82bcacdbba03b00b1f74abae694f07dcafa/relayer/ethereum/contracts/interfaces/relayer/IWormholeReceiver.sol#L44)
- As it is necessary to transmit to all supported networks several times throughout the codebase, an additional
broadcastfunction should be added to WormholeCrossChainModule.sol to handle such a case and reduce code repetition. Furthermore, a special case for transmitting to the chain itself is not necessarily (e.g.tweakEpochSchedulehttps://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/protocol/governance/contracts/modules/core/ElectionModule.sol#L173), as transmit will simply perform an external call into itself on the local network. - Two forms of refunds need to be added in
transmitregarding the comment on https://github.com/Synthetixio/synthetix-v3/blob/15ddce226263eb990a20da1f1f32ab54d55258a3/utils/core-modules/contracts/modules/WormholeCrossChainModule.sol#L101. The first is if thecostvariable exceeded the gas cost on the target chain, which can be refunded by specifying the refund address and chain per https://github.com/wormhole-foundation/wormhole/blob/d146f82bcacdbba03b00b1f74abae694f07dcafa/relayer/ethereum/contracts/interfaces/relayer/IWormholeRelayer.sol#L114. The second refund that needs to be done is formsg.value - cost - receiverValue, which should be refunded to the function caller in the event thatmsg.valueexceeds the amount required by the function. If a broadcast function is implemented, special care should be taken to ensure that the total value across all chains is considered during the refund.