Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas Optimizations #45

Open
code423n4 opened this issue Aug 1, 2022 · 2 comments
Open

Gas Optimizations #45

code423n4 opened this issue Aug 1, 2022 · 2 comments
Labels
bug Something isn't working G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Gas Report

G-01: use unchecked when incrementing for loop variable

Since the loop's iterator can't realistically overflow because of the loop's condition, you can increment it in an unchecked block to save gas:

./contracts/auth/AxelarAuthWeighted.sol:17:        for (uint256 i; i < recentOperators.length; ++i) {
./contracts/auth/AxelarAuthWeighted.sol:69:        for (uint256 i = 0; i < weightsLength; ++i) {
./contracts/auth/AxelarAuthWeighted.sol:98:        for (uint256 i = 0; i < signatures.length; ++i) {
./contracts/auth/AxelarAuthWeighted.sol:116:        for (uint256 i; i < accounts.length - 1; ++i) {
./contracts/gas-service/AxelarGasService.sol:123:        for (uint256 i; i < tokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/AdminMultisigBase.sol:51:        for (uint256 i; i < adminCount; ++i) {
./contracts/AdminMultisigBase.sol:158:        for (uint256 i; i < adminLength; ++i) {
./contracts/AxelarGateway.sol:195:        for (uint256 i; i < adminCount; ++i) {
./contracts/AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {
./contracts/AxelarGateway.sol:293:        for (uint256 i; i < commandsLength; ++i) {
./contracts/auth/AxelarAuthWeighted.sol:101:            for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}

Just search for "for (" in your .sol files to find all the relevant instances

G-02: increment using ++i instead of i++ consistently

Using ++i saves gas because you don't have to cache the original value in memory. You already use it in some places but not consistently.

./contracts/gas-service/AxelarGasService.sol:123:        for (uint256 i; i < tokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:114:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:168:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/deposit-service/AxelarDepositService.sol:204:        for (uint256 i; i < refundTokens.length; i++) {
./contracts/AxelarGateway.sol:207:        for (uint256 i = 0; i < symbols.length; i++) {

G-03: use solmate's ERC20 contract

solmate's contracts are optimized for gas usage. They are more efficient than OZ's contracts. Consider switching to them. It will save gas for all the ERC20 transfers, burns, etc.

https://github.com/transmissions11/solmate

G-04: remove unnecessary require statement in XC20Wrapper.sol

In XC20Wrapper.unwrap() the function checks whether the caller has enough tokens to burn before calling the burn() function. But, the burn function will simply revert if the user doesn't have enough. Explicitly checking for that is a waste of gas.

https://github.com/code-423n4/2022-07-axelar/blob/main/xc20/contracts/XC20Wrapper.sol#L85

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Aug 1, 2022
code423n4 added a commit that referenced this issue Aug 1, 2022
@re1ro
Copy link
Member

re1ro commented Aug 5, 2022

1

Yup. Dup #2

2

Yup. Dup #2

3

Good spot. We might consider that

4

Good spot.

@GalloDaSballo
Copy link
Collaborator

3

Would like to see a benchmark in difference

Will give the report 350 gas

@re1ro re1ro added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants