PoolTogether Aave v3 contest - miguelmtzinf's results

A protocol for no loss prize savings on Ethereum.

General Information

Platform: Code4rena

Start Date: 29/04/2022

Pot Size: $22,000 USDC

Total HM: 6

Participants: 40

Period: 3 days

Judge: Justin Goro

Total Solo HM: 2

Id: 114

League: ETH

PoolTogether

Findings Distribution

Researcher Performance

Rank: 22/40

Findings: 1

Award: $110.78

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

110.7777 USDC - $110.78

Labels

bug
G (Gas Optimization)

External Links

There are multiple variables that can be immutable and their use can be "optimized".

First of all, aToken, rewardsController and poolAddressesProviderRegistry could be immutable: there are no setters for them and all of them are proxies: in case of a change, the implementation address will change, not the proxy address.

In the case of the poolAddressesProviderRegistry:

poolAddressesProviderRegistry is only needed to fetch the pool address, but there is no need to store this address, only the address of the pool (and again, is a proxy). So, there are 2 options: - Instead of passing thepoolAddressesProviderRegistry, you can pass the poolProviderAddress directly and store its address as immutable. - You can pass the poolAddressesProviderRegistry, but only store the poolProviderAddress (making the call at the constructor and not in every call).

So, in supplyTokenTo() and redeemToken() there is a call to _pool() which calls _poolProvider() and getPool() --> 4 calls + 1 SLOAD in total. This can be done with 0 calls and 0 SLOAD just storing the pool address at the constructor.

In the case of aToken:

The aToken address does not change (proxy) and its underlying wont never change. In supplyTokenTo() and redeemToken() there is a call to _tokenAddress() that can be skipped (1 SLOAD + 1 CALL) just storing the underlying address as immutable (doing it at the constructor; the aToken address can also be stored as immutable, just for transparency).

Gas comparison reducing the cost as much as possible: https://gist.github.com/miguelmtzinf/fb6d5266fb2fd3d057028b84fc0f66e1 (also provides files with changes)

These changes reduces the gas cost across the board, and makes the most important functions way cheaper:

  • supplyTokenTo: 40k less
  • redeemToken: 40k less

Bonus track (not applied in the gist):

  • The function balanceOfToken() can be view.
  • Typo in L154. Instead of (inhereted ERC20), `(inherited ERC20).
  • In transferERC20() at L225 you are not using the already built _requireNotAToken() internal function. This function could have a more generic revert message and be used also here.

#0 - PierrickGT

2022-05-04T19:45:24Z

First of all, aToken, rewardsController and poolAddressesProviderRegistry could be immutable

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/1

poolAddressesProviderRegistry is only needed to fetch the pool address

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/17

The aToken address does not change (proxy) and its underlying wont never change.

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/19

The function balanceOfToken() can be view.

Can't because we inherit from the yield source interface.

Typo in L154. Instead of (inhereted ERC20), `(inherited ERC20).

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/22

In transferERC20() at L225 you are not using the already built _requireNotAToken() internal function.

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/4

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter