code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

Gas Optimizations #191

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

2022-07-AXELAR

Gas Optimizations Report

++i costs less gas than i++ (same for --i/i--)

Saves 6 gas per instance/loop

There are 5 instances of this issue:

File: contracts/AxelarGateway.sol

207:  for (uint256 i = 0; i < symbols.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L207

File: contracts/deposit-service/AxelarDepositService.sol

114:  for (uint256 i; i < refundTokens.length; i++) {

168:  for (uint256 i; i < refundTokens.length; i++) {

204:  for (uint256 i; i < refundTokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/AxelarDepositService.sol#L114

File: contracts/gas-service/AxelarGasService.sol

123:  for (uint256 i; i < tokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/gas-service/AxelarGasService.sol#L123

public functions not called by the contract should be declared external instead

There are 2 instances of this issue:

File: contracts/deposit-service/AxelarDepositService.sol

241:  function contractId() public pure returns (bytes32) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/AxelarDepositService.sol#L241

File: contracts/deposit-service/DepositBase.sol

41:   function wrappedToken() public view returns (address) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/DepositBase.sol#L41

It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied

Not overwriting the default for stack variables saves 8 gas. Storage and memory variables have larger savings

There are 6 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

68:   uint256 totalWeight = 0;

69:   for (uint256 i = 0; i < weightsLength; ++i) {

94:   uint256 operatorIndex = 0;

95:   uint256 weight = 0;

98:   for (uint256 i = 0; i < signatures.length; ++i) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L68

File: contracts/AxelarGateway.sol

207:  for (uint256 i = 0; i < symbols.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L207

Functions that are access-restricted from most users may be marked as payable

Marking a function as payable reduces gas cost since the compiler does not have to check whether a payment was provided or not. This change will save around 21 gas per function call.

There are 3 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

47:   function transferOperatorship(bytes calldata params) external onlyOwner {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L47

File: contracts/AxelarGateway.sol

204:  function setTokenDailyMintLimits(string[] calldata symbols, uint256[] calldata limits) external override onlyAdmin {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L204

File: contracts/gas-service/AxelarGasService.sol

120:  function collectFees(address payable receiver, address[] calldata tokens) external onlyOwner {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/gas-service/AxelarGasService.sol#L120

++i/i++ should be unchecked{++I}/unchecked{I++} in for-loops

When an increment or any arithmetic operation is not possible to overflow it should be placed in unchecked{} block. \This is because of the default compiler overflow and underflow safety checks since Solidity version 0.8.0. \In for-loops it saves around 30-40 gas per loop

There are 12 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

17:   for (uint256 i; i < recentOperators.length; ++i) {

69:   for (uint256 i = 0; i < weightsLength; ++i) {

98:   for (uint256 i = 0; i < signatures.length; ++i) {

101:  for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}

116:  for (uint256 i; i < accounts.length - 1; ++i) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L17

File: contracts/AxelarGateway.sol

195:  for (uint256 i; i < adminCount; ++i) {

207:  for (uint256 i = 0; i < symbols.length; i++) {

292:  for (uint256 i; i < commandsLength; ++i) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L195

File: contracts/deposit-service/AxelarDepositService.sol

114:  for (uint256 i; i < refundTokens.length; i++) {

168:  for (uint256 i; i < refundTokens.length; i++) {

204:  for (uint256 i; i < refundTokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/AxelarDepositService.sol#L114

File: contracts/gas-service/AxelarGasService.sol

123:  for (uint256 i; i < tokens.length; i++) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/gas-service/AxelarGasService.sol#L123

Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

'When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.' \ https://docs.soliditylang.org/en/v0.8.15/internals/layout_in_storage.html \ Use a larger size then downcast where needed

There are 2 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

14:   uint8 internal constant OLD_KEY_RETENTION = 16;

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L14

File: contracts/AxelarGateway.sol

332:  (string memory name, string memory symbol, uint8 decimals, uint256 cap, address tokenAddress, uint256 dailyMintLimit) = abi.decode(

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L332

Use calldata instead of memory for function parameters

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory. Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

There are 33 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

55:   function _transferOperatorship(bytes memory params) internal {

88:   address[] memory operators,

89:   uint256[] memory weights,

91:   bytes[] memory signatures

115:  function _isSortedAscAndContainsNoDuplicate(address[] memory accounts) internal pure returns (bool) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L55

File: contracts/AxelarGateway.sol

152:  function tokenDailyMintLimit(string memory symbol) public view override returns (uint256) {

156:  function tokenDailyMintAmount(string memory symbol) public view override returns (uint256) {

168:  function tokenAddresses(string memory symbol) public view override returns (address) {

447:  function _unpackLegacyCommands(bytes memory executeData)

460:  function _callERC20Token(address tokenAddress, bytes memory callData) internal returns (bool) {

466:  string memory symbol,

487:  string memory symbol,

539:  function _getTokenDailyMintLimitKey(string memory symbol) internal pure returns (bytes32) {

543:  function _getTokenDailyMintAmountKey(string memory symbol, uint256 day) internal pure returns (bytes32) {

547:  function _getTokenTypeKey(string memory symbol) internal pure returns (bytes32) {

551:  function _getTokenAddressKey(string memory symbol) internal pure returns (bytes32) {

561:  string memory sourceChain,

562:  string memory sourceAddress,

571:  string memory sourceChain,

572:  string memory sourceAddress,

575:  string memory symbol,

597:  function _getTokenType(string memory symbol) internal view returns (TokenType) {

605:  function _setTokenDailyMintLimit(string memory symbol, uint256 limit) internal {

611:  function _setTokenDailyMintAmount(string memory symbol, uint256 amount) internal {

618:  function _setTokenType(string memory symbol, TokenType tokenType) internal {

622:  function _setTokenAddress(string memory symbol, address tokenAddress) internal {

632:  string memory sourceChain,

633:  string memory sourceAddress,

642:  string memory sourceChain,

643:  string memory sourceAddress,

646:  string memory symbol,

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/AxelarGateway.sol#L152

File: contracts/deposit-service/AxelarDepositService.sol

220:  function _depositAddress(bytes32 create2Salt, bytes memory delegateData) internal view returns (address) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/AxelarDepositService.sol#L220

File: contracts/gas-service/AxelarGasService.sol

40:   string memory symbol,

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/gas-service/AxelarGasService.sol#L40

Replace x <= y with x < y + 1, and x >= y with x > y - 1

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =. Using strict comparison operators hence saves gas

There are 3 instances of this issue:

File: contracts/auth/AxelarAuthWeighted.sol

36:   if (operatorsEpoch == 0 || epoch - operatorsEpoch >= OLD_KEY_RETENTION) revert InvalidOperators();

107:  if (weight >= threshold) return;

117:  if (accounts[i] >= accounts[i + 1]) {

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/auth/AxelarAuthWeighted.sol#L36

Use immutable & constant for state variables that do not change their value

There are 1 instances of this issue:

File: contracts/deposit-service/DepositBase.sol

19:   address public refundToken;

https://github.com/code-423n4/2022-07-axelar/tree/main/contracts/deposit-service/DepositBase.sol#L19

GalloDaSballo commented 2 years ago

Less than 300 gas saved

I don't belive the immutable applies

Also for calldata to memory, more detail is necessary