code-423n4 / 2022-05-cudos-findings

1 stars 0 forks source link

Gas Optimizations #147

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Gas Optimizations

Summary

Title Instances
1 State variables only set in the constructor should be declared immutable 3
2 State variables should be cached in stack variables rather than re-reading them from storage 8
3 <array>.length should not be looked up in every loop of a for-loop 7
4 require()/revert() strings longer than 32 bytes cost extra gas 16
5 Using bools for storage incurs overhead 1
6 Use a more recent version of solidity 2
7 It costs more gas to initialize variables to zero than to let the default of zero be applied 9
8 ++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too) 7
9 Splitting require() statements that use && saves gas 4
10 Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead 4
11 Duplicated require()/revert() checks should be refactored to a modifier or function 4
12 Functions guaranteed to revert when called by normal users can be marked payable 1
13 public functions not called by the contract should be declared external instead 10
14 Return from function rather than breaking out of loop 1
15 require() or revert() statements that check input arguments should be at the top of the function 3
16 Remove test code to save deployment gas 1

Total: 81 instances over 16 classes

1. State variables only set in the constructor should be declared immutable

Avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

File: solidity/contracts/Gravity.sol   #1

60      bytes32 public state_gravityId;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L60

File: solidity/contracts/Gravity.sol   #2

61      uint256 public state_powerThreshold;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L61

File: solidity/contracts/Gravity.sol   #3

63      CudosAccessControls public cudosAccessControls;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L63

2. State variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching will replace each Gwarmaccess (100 gas) with a much cheaper stack read. Less obvious fixes/optimizations include having local storage variables of mappings within state variable mappings or mappings within state variable structs, having local storage variables of structs within mappings, having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

File: solidity/contracts/Gravity.sol   #1

352             state_lastEventNonce,

state_lastEventNonce https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L352

File: solidity/contracts/Gravity.sol   #2

466             emit TransactionBatchExecutedEvent(_batchNonce, _tokenContract, state_lastEventNonce);

state_lastEventNonce https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L466

File: solidity/contracts/Gravity.sol   #3

590                 state_lastEventNonce

state_lastEventNonce https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L590

File: solidity/contracts/Gravity.sol   #4

607             state_lastEventNonce

state_lastEventNonce https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L607

File: solidity/contracts/Gravity.sol   #5

628             state_lastEventNonce

state_lastEventNonce https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L628

File: solidity/contracts/Gravity.sol   #6

321         bytes32 newCheckpoint = makeCheckpoint(_newValset, state_gravityId);

state_gravityId https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L321

File: solidity/contracts/Gravity.sol   #7

431                         state_gravityId,

state_gravityId https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L431

File: solidity/contracts/Gravity.sol   #8

533                 state_gravityId,

state_gravityId https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L533

3. <array>.length should not be looked up in every loop of a for-loop

The overheads outlined below are PER LOOP, excluding the first loop

Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset

File: solidity/contracts/Gravity.sol   #1

128          for (uint256 i = 0; i < _users.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L128

File: solidity/contracts/Gravity.sol   #2

233         for (uint256 i = 0; i < _currentValidators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L233

File: solidity/contracts/Gravity.sol   #3

263         for (uint256 i = 0; i < _newValset.validators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L263

File: solidity/contracts/Gravity.sol   #4

453                 for (uint256 i = 0; i < _amounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L453

File: solidity/contracts/Gravity.sol   #5

568         for (uint256 i = 0; i < _args.transferAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L568

File: solidity/contracts/Gravity.sol   #6

579         for (uint256 i = 0; i < _args.feeAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L579

File: solidity/contracts/Gravity.sol   #7

660         for (uint256 i = 0; i < _powers.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L660

4. require()/revert() strings longer than 32 bytes cost extra gas

File: solidity/contracts/Gravity.sol   #1

117          require(
118               whitelisted[msg.sender] || cudosAccessControls.hasAdminRole(msg.sender) ,
119               "The caller is not whitelisted for this operation"
120           );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L117-L120

File: solidity/contracts/Gravity.sol   #2

238                 require(
239                     verifySig(_currentValidators[i], _theHash, _v[i], _r[i], _s[i]),
240                     "Validator signature does not match."
241                 );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L238-L241

File: solidity/contracts/Gravity.sol   #3

254         require(
255             cumulativePower > _powerThreshold,
256             "Submitted validator set signatures do not have enough power."
257         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L254-L257

File: solidity/contracts/Gravity.sol   #4

289         require(
290             _newValset.valsetNonce > _currentValset.valsetNonce,
291             "New valset nonce must be greater than the current nonce"
292         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L289-L292

File: solidity/contracts/Gravity.sol   #5

310         require(
311             makeCheckpoint(_currentValset, state_gravityId) == state_lastValsetCheckpoint,
312             "Supplied current validators and powers do not match checkpoint."
313         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L310-L313

File: solidity/contracts/Gravity.sol   #6

315         require(
316             isOrchestrator(_currentValset, msg.sender),
317             "The sender of the transaction is not validated orchestrator"
318         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L315-L318

File: solidity/contracts/Gravity.sol   #7

384             require(
385                 state_lastBatchNonces[_tokenContract] < _batchNonce,
386                 "New batch nonce must be greater than the current nonce"
387             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L384-L387

File: solidity/contracts/Gravity.sol   #8

390             require(
391                 block.number < _batchTimeout,
392                 "Batch timeout must be greater than the current block height"
393             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L390-L393

File: solidity/contracts/Gravity.sol   #9

405             require(
406                 makeCheckpoint(_currentValset, state_gravityId) == state_lastValsetCheckpoint,
407                 "Supplied current validators and powers do not match checkpoint."
408             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L405-L408

File: solidity/contracts/Gravity.sol   #10

416             require(
417                 isOrchestrator(_currentValset, msg.sender),
418                 "The sender of the transaction is not validated orchestrator"
419             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L416-L419

File: solidity/contracts/Gravity.sol   #11

494             require(
495                 state_invalidationMapping[_args.invalidationId] < _args.invalidationNonce,
496                 "New invalidation nonce must be greater than the current nonce"
497             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L494-L497

File: solidity/contracts/Gravity.sol   #12

509             require(
510                 makeCheckpoint(_currentValset, state_gravityId) == state_lastValsetCheckpoint,
511                 "Supplied current validators and powers do not match checkpoint."
512             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L509-L512

File: solidity/contracts/Gravity.sol   #13

515             require(
516                 _args.transferAmounts.length == _args.transferTokenContracts.length,
517                 "Malformed list of token transfers"
518             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L515-L518

File: solidity/contracts/Gravity.sol   #14

525             require(
526                 isOrchestrator(_currentValset, msg.sender),
527                 "The sender of the transaction is not validated orchestrator"
528             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L525-L528

File: solidity/contracts/Gravity.sol   #15

655         require(address(_cudosAccessControls) != address(0), "Access control contract address is incorrect");

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L655

File: solidity/contracts/Gravity.sol   #16

666         require(
667             cumulativePower > _powerThreshold,
668             "Submitted validator set signatures do not have enough power."
669         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L666-L669

5. Using bools for storage incurs overhead

    // Booleans are more expensive than uint256 or any type that takes up a full
    // word because each write operation emits an extra SLOAD to first read the
    // slot's contents, replace the bits taken up by the boolean, and then write
    // back. This is the compiler's defense against contract upgrades and
    // pointer aliasing, and it cannot be disabled.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false

File: solidity/contracts/Gravity.sol   #1

65      mapping(address => bool) public whitelisted;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L65

6. Use a more recent version of solidity

Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath Use a solidity version of at least 0.8.2 to get compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

File: solidity/contracts/CosmosToken.sol   #1

1   pragma solidity ^0.6.6;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/CosmosToken.sol#L1

File: solidity/contracts/Gravity.sol   #2

1   pragma solidity ^0.6.6;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L1

7. It costs more gas to initialize variables to zero than to let the default of zero be applied

File: solidity/contracts/Gravity.sol   #1

128          for (uint256 i = 0; i < _users.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L128

File: solidity/contracts/Gravity.sol   #2

231         uint256 cumulativePower = 0;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L231

File: solidity/contracts/Gravity.sol   #3

233         for (uint256 i = 0; i < _currentValidators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L233

File: solidity/contracts/Gravity.sol   #4

263         for (uint256 i = 0; i < _newValset.validators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L263

File: solidity/contracts/Gravity.sol   #5

453                 for (uint256 i = 0; i < _amounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L453

File: solidity/contracts/Gravity.sol   #6

568         for (uint256 i = 0; i < _args.transferAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L568

File: solidity/contracts/Gravity.sol   #7

579         for (uint256 i = 0; i < _args.feeAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L579

File: solidity/contracts/Gravity.sol   #8

659         uint256 cumulativePower = 0;

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L659

File: solidity/contracts/Gravity.sol   #9

660         for (uint256 i = 0; i < _powers.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L660

8. ++i costs less gas than ++i, especially when it's used in for-loops (--i/i-- too)

Saves 6 gas PER LOOP

File: solidity/contracts/Gravity.sol   #1

128          for (uint256 i = 0; i < _users.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L128

File: solidity/contracts/Gravity.sol   #2

233         for (uint256 i = 0; i < _currentValidators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L233

File: solidity/contracts/Gravity.sol   #3

263         for (uint256 i = 0; i < _newValset.validators.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L263

File: solidity/contracts/Gravity.sol   #4

453                 for (uint256 i = 0; i < _amounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L453

File: solidity/contracts/Gravity.sol   #5

568         for (uint256 i = 0; i < _args.transferAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L568

File: solidity/contracts/Gravity.sol   #6

579         for (uint256 i = 0; i < _args.feeAmounts.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L579

File: solidity/contracts/Gravity.sol   #7

660         for (uint256 i = 0; i < _powers.length; i++) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L660

9. Splitting require() statements that use && saves gas

See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper

File: solidity/contracts/Gravity.sol   #1

301         require(
302             _currentValset.validators.length == _currentValset.powers.length &&
303                 _currentValset.validators.length == _v.length &&
304                 _currentValset.validators.length == _r.length &&
305                 _currentValset.validators.length == _s.length,
306             "Malformed current validator set"
307         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L301-L307

File: solidity/contracts/Gravity.sol   #2

396             require(
397                 _currentValset.validators.length == _currentValset.powers.length &&
398                     _currentValset.validators.length == _v.length &&
399                     _currentValset.validators.length == _r.length &&
400                     _currentValset.validators.length == _s.length,
401                 "Malformed current validator set"
402             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L396-L402

File: solidity/contracts/Gravity.sol   #3

411             require(
412                 _amounts.length == _destinations.length && _amounts.length == _fees.length,
413                 "Malformed batch of transactions"
414             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L411-L414

File: solidity/contracts/Gravity.sol   #4

500             require(
501                 _currentValset.validators.length == _currentValset.powers.length &&
502                     _currentValset.validators.length == _v.length &&
503                     _currentValset.validators.length == _r.length &&
504                     _currentValset.validators.length == _s.length,
505                 "Malformed current validator set"
506             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L500-L506

10. 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.11/internals/layout_in_storage.html Use a larger size then downcast where needed

File: solidity/contracts/CosmosToken.sol   #1

11          uint8 _decimals

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/CosmosToken.sol#L11

File: solidity/contracts/Gravity.sol   #2

91          uint8 _decimals,

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L91

File: solidity/contracts/Gravity.sol   #3

178         uint8 _v,

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L178

File: solidity/contracts/Gravity.sol   #4

615         uint8 _decimals

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L615

11. Duplicated require()/revert() checks should be refactored to a modifier or function

Saves deployment costs

File: solidity/contracts/Gravity.sol   #1

666         require(
667             cumulativePower > _powerThreshold,
668             "Submitted validator set signatures do not have enough power."
669         );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L666-L669

File: solidity/contracts/Gravity.sol   #2

396             require(
397                 _currentValset.validators.length == _currentValset.powers.length &&
398                     _currentValset.validators.length == _v.length &&
399                     _currentValset.validators.length == _r.length &&
400                     _currentValset.validators.length == _s.length,
401                 "Malformed current validator set"
402             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L396-L402

File: solidity/contracts/Gravity.sol   #3

405             require(
406                 makeCheckpoint(_currentValset, state_gravityId) == state_lastValsetCheckpoint,
407                 "Supplied current validators and powers do not match checkpoint."
408             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L405-L408

File: solidity/contracts/Gravity.sol   #4

416             require(
417                 isOrchestrator(_currentValset, msg.sender),
418                 "The sender of the transaction is not validated orchestrator"
419             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L416-L419

12. Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost

File: solidity/contracts/Gravity.sol   #1

124     function manageWhitelist(
125         address[] memory _users,
126         bool _isWhitelisted
127         ) public onlyWhitelisted {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L124-L127

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

Contracts are allowed to override their parents' functions and change the visibility from external to public and can save gas by doing so.

File: solidity/contracts/Gravity.sol   #1

124     function manageWhitelist(
125         address[] memory _users,
126         bool _isWhitelisted
127         ) public onlyWhitelisted {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L124-L127

File: solidity/contracts/Gravity.sol   #2

140     function testMakeCheckpoint(ValsetArgs memory _valsetArgs, bytes32 _gravityId) public pure {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L140

File: solidity/contracts/Gravity.sol   #3

144     function testCheckValidatorSignatures(
145         address[] memory _currentValidators,
146         uint256[] memory _currentPowers,
147         uint8[] memory _v,
148         bytes32[] memory _r,
149         bytes32[] memory _s,
150         bytes32 _theHash,
151         uint256 _powerThreshold

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L144-L151

File: solidity/contracts/Gravity.sol   #4

166     function lastBatchNonce(address _erc20Address) public view returns (uint256) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L166

File: solidity/contracts/Gravity.sol   #5

170     function lastLogicCallNonce(bytes32 _invalidation_id) public view returns (uint256) {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L170

File: solidity/contracts/Gravity.sol   #6

276     function updateValset(
277         // The new version of the validator set
278         ValsetArgs memory _newValset,
279         // The current validators that approve the change
280         ValsetArgs memory _currentValset,
281         // These are arrays of the parts of the current validator's signatures
282         uint8[] memory _v,
283         bytes32[] memory _r,
284         bytes32[] memory _s
285     ) public nonReentrant {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L276-L285

File: solidity/contracts/Gravity.sol   #7

364     function submitBatch (
365         // The validators that approve the batch
366         ValsetArgs memory _currentValset,
367         // These are arrays of the parts of the validators signatures
368         uint8[] memory _v,
369         bytes32[] memory _r,
370         bytes32[] memory _s,
371         // The batch of transactions
372         uint256[] memory _amounts,
373         address[] memory _destinations,
374         uint256[] memory _fees,
375         uint256 _batchNonce,
376         address _tokenContract,
377         // a block height beyond which this batch is not valid
378         // used to provide a fee-free timeout
379         uint256 _batchTimeout
380     ) public nonReentrant {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L364-L380

File: solidity/contracts/Gravity.sol   #8

479     function submitLogicCall(
480         // The validators that approve the call
481         ValsetArgs memory _currentValset,
482         // These are arrays of the parts of the validators signatures
483         uint8[] memory _v,
484         bytes32[] memory _r,
485         bytes32[] memory _s,
486         LogicCallArgs memory _args
487     ) public nonReentrant {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L479-L487

File: solidity/contracts/Gravity.sol   #9

595     function sendToCosmos(
596         address _tokenContract,
597         bytes32 _destination,
598         uint256 _amount
599     ) public nonReentrant  {

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L595-L599

File: solidity/contracts/Gravity.sol   #10

611     function deployERC20(
612         string memory _cosmosDenom,
613         string memory _name,
614         string memory _symbol,
615         uint8 _decimals

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L611-L615

14. Return from function rather than breaking out of loop

Using return rather than break saves gas because the require() outside of the for-loop has the same condition that caused the loop to be broken out of

File: solidity/contracts/Gravity.sol   #1

246                 // Break early to avoid wasting gas
247                 if (cumulativePower > _powerThreshold) {
248                     break;
249                 }
250             }
251         }
252  
253         // Check that there was enough power
254         require(
255             cumulativePower > _powerThreshold,
256             "Submitted validator set signatures do not have enough power."
257         );
258         // Success
259     }

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L246-L259

15. require() or revert() statements that check input arguments should be at the top of the function

Checks that involve constants should come before checks that involve state variables

File: solidity/contracts/Gravity.sol   #1

411             require(
412                 _amounts.length == _destinations.length && _amounts.length == _fees.length,
413                 "Malformed batch of transactions"
414             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L411-L414

File: solidity/contracts/Gravity.sol   #2

514             // Check that the token transfer list is well-formed
515             require(
516                 _args.transferAmounts.length == _args.transferTokenContracts.length,
517                 "Malformed list of token transfers"
518             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L514-L518

File: solidity/contracts/Gravity.sol   #3

520             // Check that the fee list is well-formed
521             require(
522                 _args.feeAmounts.length == _args.feeTokenContracts.length,
523                 "Malformed list of fees"
524             );

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L520-L524

16. Remove test code to save deployment gas

File: solidity/contracts/Gravity.sol   #1

138     // TEST FIXTURES
139     // These are here to make it easier to measure gas usage. They should be removed before production
140     function testMakeCheckpoint(ValsetArgs memory _valsetArgs, bytes32 _gravityId) public pure {
141         makeCheckpoint(_valsetArgs, _gravityId);
142     }
143  
144     function testCheckValidatorSignatures(
145         address[] memory _currentValidators,
146         uint256[] memory _currentPowers,
147         uint8[] memory _v,
148         bytes32[] memory _r,
149         bytes32[] memory _s,
150         bytes32 _theHash,
151         uint256 _powerThreshold
152     ) public pure {
153         checkValidatorSignatures(
154             _currentValidators,
155             _currentPowers,
156             _v,
157             _r,
158             _s,
159             _theHash,
160             _powerThreshold
161         );
162     }
163  
164     // END TEST FIXTURES

https://github.com/code-423n4/2022-05-cudos/blob/de39cf3cd1f1e1cf211819b06d4acf6a043acda0/solidity/contracts/Gravity.sol#L138-L164

V-Staykov commented 2 years ago

I find this report particularly high quality.