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

Consolidating library functions can save gas by preventing external calls #42

Open
code423n4 opened this issue Jul 11, 2021 · 1 comment
Assignees
Labels
bug Something isn't working G (Gas Optimization) sponsor confirmed Yes, this is a problem and we intend to fix it.

Comments

@code423n4
Copy link
Contributor

Handle

0xRajeev

Vulnerability details

Impact

While code modularity is generally a good practice and creating libraries of functions commonly used across different contracts can increase maintainability and reduce contract deployment size/cost, it comes at the increased cost of gas usage at runtime because of the external calls. EIP-2929 in Berlin fork increased the gas costs of CALL* family opcodes to 2600. Making a delegatecall to a library function therefore costs 2600.

Impact: A LibAsset.transferAsset() call from TransactionManager.sol makes LibERC20.transfer() call for ERC20 which in turn makes another external call to LibUtils.revertIfCallFailed() in wrapCall. So an ERC20 transfer effectively makes 3 additional (besides the ERC20 token contract function call assetId.call(..) external calls -> LibAsset -> LibERC20 -> LibUtils, which costs 2600*3 = 7800 gas.

Combining these functions into a single library or making them all internal to TransactionManager.sol can convert these delegatecalls into JMPs to save gas.

Proof of Concept

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/lib/LibAsset.sol#L58

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/lib/LibAsset.sol#L44

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/lib/LibERC20.sol#L64

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/lib/LibERC20.sol#L20

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L128

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L369

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L378

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L406

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L425

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L504

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L514

https://github.com/code-423n4/2021-07-connext/blob/8e1a7ea396d508ed2ebeba4d1898a748255a48d2/contracts/TransactionManager.sol#L525

And other Lib* calls.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider moving all the library functions internal to this contract or to a single library to save gas from external calls each of which costs 2600 gas.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jul 11, 2021
code423n4 added a commit that referenced this issue Jul 11, 2021
@LayneHaber LayneHaber added the sponsor confirmed Yes, this is a problem and we intend to fix it. label Jul 13, 2021
@LayneHaber LayneHaber self-assigned this Jul 13, 2021
@LayneHaber
Copy link
Collaborator

connext/monorepo#22

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 confirmed Yes, this is a problem and we intend to fix it.
Projects
None yet
Development

No branches or pull requests

2 participants