Platform: Code4rena
Start Date: 17/06/2021
Pot Size: $60,000 USDC
Total HM: 12
Participants: 12
Period: 7 days
Judge: LSDan
Total Solo HM: 8
Id: 14
League: ETH
Rank: 2/12
Findings: 6
Award: $15,932.26
🌟 Selected for report: 26
🚀 Solo Findings: 2
0xRajeev
The _withdrawFromVault() calculates the token balance of contract before withdrawal and saves it in previousBalance. It then withdraws from the Yearn vault and calculates the token balance after withdrawal to save it in currentBalance. So currentBalance should be > previousBalance if >0 tokens are withdrawn from the vault.
However, it incorrectly interchanges use of previousBalance and currentBalance in calculating the tokens withdrawn from the vault with the previousBalance.sub(currentBalance) calculation which will always cause _withdrawFromVault() to revert because currentBalance > previousBalance if/when >0 tokens are withdrawn from vault.
Impact: This means that tokens can be supplied to the vault but never redeemed leading to lock/loss of user deposits.
Manual Analysis
Switch the use of previousBalance and currentBalance and change the code to currentBalance.sub(previousBalance) on Line 194.
#0 - asselstine
2021-06-25T22:53:53Z
#1 - dmvt
2021-08-27T22:36:02Z
duplicate of #90
786.6464 USDC - $786.65
0xRajeev
The _depositInVault() function for Yearn yield source uses ERC20 safeApprove() from OpenZeppelin's SafeERC20 library to give maximum allowance to the Yearn Vault address if the current allowance is less than contract’s token balance.
However, the safeApprove function prevents changing an allowance between non-zero values to mitigate a possible front-running attack. It reverts if that is the case. Instead, the safeIncreaseAllowance and safeDecreaseAllowance functions should be used. Comment from the OZ library for this function: “// safeApprove should only be called when setting an initial allowance, // or when resetting it to zero. To increase and decrease it, use // 'safeIncreaseAllowance' and ‘safeDecreaseAllowance'"
Impact: If the existing allowance is non-zero (say, for e.g., previously the entire balance was not deposited due to vault balance limit resulting in the allowance being reduced but not made 0), then safeApprove() will revert causing the user’s token deposits to fail leading to denial-of-service. The condition predicate indicates that this scenario is possible.
Reference: See similar Medium-severity finding M03 here: https://blog.openzeppelin.com/1inch-exchange-audit/
Manual Analysis
Use safeIncreaseAllowance() function instead of safeApprove().
🌟 Selected for report: 0xRajeev
1748.103 USDC - $1,748.10
0xRajeev
The contract uses _msgSender() to denote an operator who is operating on behalf of the user. This is typically used for meta-transactions where the operator is an intermediary/relayer who may facilitate gasless transactions on behalf of the user. They may be the same address but it is safer to assume that they may not be.
While the code handles this separation of role in most cases, it misses doing so in timelockDepositTo() function where it accounts the _timelockBalances to the operator address instead of the user specified to address. It assumes they are the same. The corresponding usage in _mintTimelock() which is called from withdrawWithTimelockFrom() uses the user specified 'from' address and not the _msgSender(). Therefore the corresponding usage in timelockDepositTo() should be the same.
Impact: In the scenario where the operator address != user specified from/to addresses, i.e. meta-transactions, the timelock deposits and withdrawals are made to/from different addresses and so the deposits of timelocked tokens will fail because the operator’s address does not have the required amount of _timelockBalances.
Manual Analysis
Change operator to from on L281 of timelockDepositTo(). Specify the scenarios where the role of the operator is applicable and document/implement those accordingly.
#0 - asselstine
2021-06-26T00:09:04Z
In the function timelockDepositTo() the msg.sender is using their timelocked funds to re-enter the pool. They can only spend their own funds; they should not be able to spend other user's funds.
The warden is saying the timelockDepositTo should be callable by anyone and allow them to transfer other user's funds from the timelock back into tickets. This actually introduces an attack vector.
#1 - dmvt
2021-08-27T21:59:39Z
I think sponsor is misunderstanding warden's concern here. The issue is not that msg.sender is being checked, but that _msgSender is being checked. Happy to discuss this more if sponsor still disagrees, but I think the concern raised is valid.
🌟 Selected for report: 0xRajeev
1748.103 USDC - $1,748.10
0xRajeev
Low-level calls call/delegatecall/staticcall return true even if the account called is non-existent (per EVM design). Solidity documentation warns: "The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed.”
The staticcall here will return True even if the _yieldSource contract doesn't exist at any incorrect-but-not-zero address, e.g. EOA address, used during initialization by accident. Impact: The hack, as commented, to check if it’s an actual yield source contract will fail if the address is indeed a contract account which doesn’t implement the depositToken function. However, if the address is that of an EOA account, the check will pass here but will revert in all future calls to the yield source forcing contract redeployment after the pool is active. Users will not be able to interact with the pool and abandon it.
Manual Analysis
A contract existence check should be performed on _yieldSource prior to the depositToken function existence hack for determining yield source contract.
0xRajeev
Many contracts use OpenZeppelin’s nonreentrant modifier on functions that interact with external contracts to prevent any reentrancies possible. However, there are a few functions that miss using this modifier.
Impact: Missing nonreentrant modifier may allow reentrancy although the risk is low given that most external contracts are “trusted” ones from the project itself or well-known yield sources. Although, there are a few functions that make calls to unknown/untrusted erc20 tokens but they seem to follow the CEI pattern currently. It is nevertheless safer to add the modifier for consistency given this is used everywhere else, at the cost of some extra gas.
Manual Analysis
Add missing nonreentrant modifier on all functions with external calls.
#0 - asselstine
2021-06-24T23:05:01Z
🌟 Selected for report: 0xRajeev
582.701 USDC - $582.70
0xRajeev
If a function has multiple modifiers they are executed in the order specified. If checks or logic of modifiers depend on other modifiers this has to be considered in their ordering. PrizePool has functions with multiple modifiers with one of them being nonreentrant which prevents reentrancy on the functions. This should ideally be the first one to prevent even the execution of other modifiers in case of reentrancies.
While there is no obvious vulnerability currently with nonreentrant being the last modifier in the list, it is safer to place it in the first. This is of slight concern with the deposit functions which have the canAddLiquidity() modifier (before nonreentrant) that makes external calls to get totalSupply of controlled tokens.
For reference, see similar finding in Consensys’s audit of Balancer : https://consensys.net/diligence/audits/2020/05/balancer-finance/#switch-modifier-order-in-bpool
Manual Analysis
Switch modifier order to consistently place the nonreentrant modifier as the first one to run so that all other modifiers are executed only if the call is nonreentrant.
🌟 Selected for report: 0xRajeev
582.701 USDC - $582.70
0xRajeev
The modifier onlyControlledToken is used for functions that allow the controlledToken address as a parameter to ensure that only whitelisted tokens (ticket and sponsorship) are provided. This is used in all functions except calculateEarlyExitFee().
Impact: The use of a non-whitelisted controlledToken will result in calls to potentially malicious token contract and cause undefined behavior for the from user address specified in the call.
Missing modifier: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/PrizePool.sol#L729-L747
All other functions which accept controlledToken parameter have modifier onlyControlledToken: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/PrizePool.sol#L275
Manual Analysis
Add missing modifier onlyControlledToken to calculateEarlyExitFee().
#0 - kamescg
2021-06-30T07:01:50Z
This would likely break assumptions made by other contracts when used to get the early exit fee.
For example in Pods to calculate the exit fee. Plus this is called statically from JS frontends to get the fee.
/** * @notice Calculate the cost of withdrawing from the Pod if the * @param amount Amount of tokens to withdraw when calculating early exit fee. * @dev Based of the Pod's total token/ticket balance and totalSupply it calculates the pricePerShare. */ function getEarlyExitFee(uint256 amount) external returns (uint256) { uint256 tokenBalance = _podTokenBalance(); if (amount <= tokenBalance) { return 0; } else { // Calculate Early Exit Fee (uint256 exitFee, ) = _prizePool.calculateEarlyExitFee( address(this), address(ticket), amount.sub(tokenBalance) ); // Early Exit Fee return exitFee; } }
#1 - asselstine
2021-07-05T23:47:38Z
@kamescg Rajeev is suggesting to add the modifier onlyControlledToken to calculateEarlyExitFee()
That means it would revert on invalid controlled tokens. It would still be a static call!
That being said this isn't a deal breaker. We can skip this one and it wouldn't hurt.
262.2155 USDC - $262.22
0xRajeev
Most contracts use the delegateCall proxy pattern and hence their implementations require the use of initialize() functions instead of constructors. This requires derived contracts to call the corresponding init functions of their inherited base contracts. This is done in most places except a few.
Impact: The inherited base classes do not get initialized which may lead to undefined behavior.
Missing call to __ReentrancyGuard_init: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/yield-source/ATokenYieldSource.sol#L99-L102
Missing call to__ERC20_init: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/yield-source/IdleYieldSource.sol#L59-L61
Manual Analysis
Add missing calls to init functions of inherited contracts.
#0 - PierrickGT
2021-06-28T13:34:00Z
ATokenYieldSource PR: https://github.com/pooltogether/aave-yield-source/pull/18
#1 - PierrickGT
2021-06-30T13:34:14Z
Has been fixed already for IdleYieldSource: https://github.com/pooltogether/idle-yield-source/blob/master/contracts/IdleYieldSource.sol#L60-#62
#2 - PierrickGT
2021-07-01T13:37:31Z
YearnV2YieldSource: https://github.com/pooltogether/pooltogether-yearnv2-yield-source/pull/8
157.3293 USDC - $157.33
0xRajeev
The project uses a combination of solc pragmas with most of them using a floating pragma of >=0.6.0 <0.7.0 (which can be simplified to ^0.6.0), Yearn/Sushi/Badger yield contracts using 0.6.12 and Idle yield contract using 0.8.4. The 0.6.x versions are a bit dated but perhaps a carry over from legacy project versions.
Impact: Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the pragma to a specific version (by not specifying a range) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs or newer untested version. Mixing breaking versions such as 0.6.12 and 0.8.4 leads to mixed protections (e.g. 0.8.4 has default overflow/underflow checks while 0.6.12 does not) and syntactic/semantic inconsistencies.
Manual Analysis
Evaluate upgrading all contracts to the latest 0.8.x compiler. Use a single unlocked/non-floating version consistently across codebase.
#0 - asselstine
2021-06-25T23:11:50Z
While locking down the pragma is good, the conclusions are wrong here. We have libraries we depend on so we cannot have a consistent compiler version across everything.
The important thing is to lock down the pragma so that the tested compiler matches the deployment compiler.
Conclusions here are wrong.
#1 - dmvt
2021-08-23T15:59:50Z
duplicate of #109
🌟 Selected for report: 0xRajeev
582.701 USDC - $582.70
0xRajeev
Checking addresses against zero-address during initialization or during setting is a security best-practice. However, such checks are missing in all address variable initializations.
Impact: Allowing zero-addresses will lead to contract reverts and force redeployments if there are no setters for such address variables.
Manual Analysis
Add zero-address checks for all initializations of address state variables.
#0 - kamescg
2021-06-30T06:55:01Z
Core
Aave
Sushi
Idle
Badger
🌟 Selected for report: 0xRajeev
582.701 USDC - $582.70
0xRajeev
The _depositInVault() returns the value returned from its call to the Yearn vault’s deposit() function. However, the return value is ignored at both call sites in supplyTokenTo() and sponsor().
Impact: It is unclear what the intended usage is and how, if any, the return value should be checked. This should perhaps check how much of the full balance was indeed deposited/rejected in the vault by comparing the return value of issued vault shares as commented: “The actual amount of shares that were received for the deposited tokens” because "if deposit limit is reached, tokens will remain in the Yield Source and they will be queued for retries in subsequent deposits.”
Manual Analysis
Check return value appropriately or if not, document why this is not necessary.
#0 - asselstine
2021-06-25T22:57:08Z
Regardless of how much of the deposit made it into the underlying vault, the depositor will hold the correct number shares. It doesn't matter if only a portion of the funds made it into the vault.
#1 - dmvt
2021-08-23T20:21:03Z
I think this is a reasonable finding by the warden. If the return value isn't needed, it should be removed or at least documented that it's there for no reason. If there is a reason to have the return value, the return value should be considered by the calling functions.
🌟 Selected for report: 0xRajeev
582.701 USDC - $582.70
0xRajeev
Most contracts use initialize() functions instead of constructor given the delegatecall proxy pattern. While most of them emit an event in the critical initialize() functions to record the init parameters for off-chain monitoring and transparency reasons, Ticket.sol nor its base class ControlledToken.sol emit such an event in their initialize() functions.
Impact: These contracts are initialized but their critical init parameters (name, symbol, decimals and controller address) are not logged for any off-chain monitoring.
See similar Medium-severity Finding M01 in OpenZeppelin’s audit of UMA protocol: https://blog.openzeppelin.com/uma-audit-phase-4/
Examples of event emission: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/PrizePool.sol#L239-L243
Manual Analysis
Emit an initialised event in Ticket.sol and ControlledToken.sol logging their init parameters.
#0 - asselstine
2021-06-25T23:07:47Z
This is just event emission; it's severity is 0 (Non-critical).
#2 - dmvt
2021-08-27T22:25:35Z
I'm going to split the difference here. Events are important for various reasons. In this case, due to the proxy pattern, the creation of the contract in the initialize function happen at the same time, making it trivial for a user to go back and look at the initialization parameters in the creation transaction.
54.8467 USDC - $54.85
0xRajeev
State variables badger and badgerSett addresses are read two and four times respectively in supplyTokenTo(). Caching them in local variables at the beginning of the function and using those local variables can save 400 gas from avoiding 3 repeated SLOADs for badgerSett and 1 repeated SLOAD for badger.
Impact: Gas savings of 400
Two badger reads: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/yield-source/BadgerYieldSource.sol#L44-L45
Four badgerSett reads:
Manual Analysis
Cache badger and badgerSett state variables in local variables at the beginning of the function and use those local variables instead.
#0 - kamescg
2021-07-01T05:05:21Z
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
The latest version of solc compiler is 8.6. Most contracts (except IdleYieldSource) allow use of solc version >=0.6.0 <0.7.0, which is fairly dated. This may be a carry-over from previous versions of project to minimize porting code to handle breaking changes across solc 0.7.0 or 0.8.0.
Impact: Upgrading the solc compiler to 0.8 will give the latest compiler benefits including bug fixes, security enhancements and overall optimizations especially the in-built overflow/underflow checks which may give gas savings by avoiding expensive SafeMath.
Manual Analysis
Consider porting over code to solc >= 0.8.0 for bug fixes, security enhancements and overall optimizations for gas savings.
#0 - asselstine
2021-06-26T18:27:09Z
We're looking for concrete optimizations, not suggestions to upgrade the version of solc we use.
#1 - dmvt
2021-07-21T15:47:41Z
You can take or leave the suggestion, but he is correct here that upgrading to 0.8 would save some gas.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
Where possible, use equivalent function parameters or local variables in event emits instead of state variables to prevent expensive SLOADs. Post-Berlin, SLOADs on state variables accessed first-time in a transaction increased from 800 gas to 2100, which is a 2.5x increase.
The Initialized event in PrizePool uses state variables maxExitFeeMantissa and maxTimelockDuration instead of using the equivalent function parameters _maxExitFeeMantissa and _maxTimelockDuration which were just used to set these state variables. Using them instead will save 2 extra SLOADs, leading to gas savings of 200.
The StakePrizePoolInitialized event uses state variable stakeToken instead of the function parameter _stakeToken used to set it. Using that instead will save 100 gas.
The IdleYieldSourceInitialized similarly uses idleToken instead of _idleToken.
Manual Analysis
Use equivalent function parameters or local variables in event emits instead of state variables.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
The canAddLiquidity modifier, which is used on all deposits (each deposit costs 0.5M gas), appears to be an expensive modifier because it calculates the sum of all the supplies across controlled tokens (by making external CALLs) and adding that up with reserve and timelock supplies. While this is an extensible implementation that supports arbitrary number of controlled tokens via mapped singly linked list, the prize pools typically have only two controlled tokens: tickets and sponsorship.
Impact: deposits currently cost 0.5M gas.
https://docs.pooltogether.com/protocol/overview#gas-usage
Manual Analysis
Consider gas profiling a fast-path calculation by keeping a separate state variable that tracks the sum of timelock+reserve along with all deposits made towards controlled token supplies and comparing new deposits with that state variable instead of reevaluating totals during each deposit. The extra SLOADs, CALLs and other expensive operations (in linked list and other logic) during reevaluation may add up to more than updating this proposed new state variable across different operations.
#0 - asselstine
2021-06-26T18:25:26Z
Yes; since minting and burning occurs within the prize pool it would be easy to track the total.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
Cache _currentAwardBalance state variable in a local variable for computation to save gas. 4 SLOADs + 1 SSTORE can be reduced to 1 SLOAD and 1 STORE.
Impact: Saves 300 gas from avoid 3 SLOADs because each SLOAD to already accessed storage slot costs 100.
1 SSTORE + 1 SLOAD: https://github.com/code-423n4/2021-06-pooltogether/blob/85f8d044e7e46b7a3c64465dcd5dffa9d70e4a3e/contracts/PrizePool.sol#L465
Manual Analysis
Cache _currentAwardBalance in a local variable in the beginning, use that for computation/return and one updation to state variable at the end.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
EIP-2929 in Berlin fork increased the gas costs of SLOADs and CALL* family opcodes increasing them for not-accessed slots/addresses and decreasing them for accessed slots. EIP-2930 optionally supports specifying an access list in the transition of all slots and addresses accessed by the transaction which reduces their gas cost upon access and prevents EIP-2929 gas cost increases from breaking contracts.
Impact: Considering these changes may significantly impact gas usage for transactions that call functions touching many state variables or making many external calls.
Example 1: Commonly used functions depositTo() and withdraw*() which cost 0.5M gas now can use access lists for all the state variables they touch and addresses of calls made to reduce gas.
Example 2: Functions such as sweepTimelockBalances() which potentially touch many state variables for multiple users and also make multiple transfers may benefit significantly by considering the use of access lists.
Example 3: awardExternalERC721() makes CALLs to externalToken contract which costs 2600 first time and then 100 subsequently. By including this in an access list (per Berlin EIP-2930), we can reduce the cost of the first CALL to externalToken contract from 2600 to 2400 (cost of specifying the address in the access list) + 100 (subsequent CALL which is cheaper now because it is considered as accessed due to the access list inclusion). So it is 2500 instead of 2600 and we save 100 gas.
https://eips.ethereum.org/EIPS/eip-2929
https://eips.ethereum.org/EIPS/eip-2930
https://hackmd.io/@fvictorio/gas-costs-after-berlin
https://github.com/gakonst/ethers-rs/issues/265
Manual Analysis
Evaluate the feasibility of using access lists to save gas due to EIPs 2929 & 2930 post-Berlin hard fork. The tooling support is WIP.
#0 - asselstine
2021-06-26T18:20:33Z
This may or may not help with gas consumption, but it is deeply unmaintainable. While this might introduce improvements we're not going to tackle this.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
Mapping state variable value _timelockBalances[user] is read on consecutive lines 655 and 656 resulting in 2 SLOADS (2100 + 100 gas).
Impact: Caching this in a local variable would save ~= 100 gas savings per user iteration (by converting the use of the second 100-gas costing SLOAD to 1 MSTORE and 1 MLOAD both of which only cost 3 gas). If there are 1000 users in a call to sweepTimelockBalances(), this could be significant savings of 100,000 gas.
Manual Analysis
Cache _timelockBalances[user] in a local variable before using on lines 655 and 656.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
State variable maxTimelockDuration is read twice on consecutive lines 723 and 724 of function _calculateTimelockDuration(). Caching it in a local variable will save 100 gas.
Manual Analysis
Cache maxTimelockDuration in a local variable in the beginning of the function.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
_currentTime() returns the block.timestamp value to multiple call sites in PrizePool contract. This internal call only to get value of block.timestamp seems unnecessary because there isn’t any other way of getting current time on the blockchain which justifies moving this to a separate function for modularity.
Impact: Adds an additional jump and other supporting bytecode of making the internal call which increase gas usage unnecessarily.
Manual Analysis
Use block.timestamp directly to save a little gas by avoiding this unnecessary indirection.
#0 - asselstine
2021-06-26T18:12:06Z
We need to manipulate the time for testing, and changing the EVM time via _increaseTime wasn't working consistently.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
When _tokenTotalSupply() adds up the supplies of all controlled tokens, it checks and skips zero-address tokens. Instead of checking for zero-address every time for every call to _tokenTotalSupply() from captureAwardBalance() and every deposit via canAddLiquidity modifier, preventing zero-address controlled-token addresses from being added in _addControlledToken() during initialization will avoid these checks.
Impact: All deposit calls which cost 0.5M gas currently will be impacted by these unnecessary checks if we instead perform it one time during the addition of tokens in initialization.
Manual Analysis
Move zero-address check from time of use to time of adding the tokens into the list in initialize().
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
The loop iteration in _tokenTotalSupply() ends when currentToken matches _tokens.end() where _tokens is a state variable.
Impact: Checking against the state variable for every iteration costs 100 gas per iteration. Even with only two controlled tokens (tickets & sponsorship), this costs 100 more than caching this in a local memory variable and using that within the while predicate.
Manual Analysis
Cache _tokens.end() in a local memory variable before the loop and using that within the while predicate.
135.4239 USDC - $135.42
0xRajeev
_depositToAave() always returns 0 and neither of the call sites in supplyTokenTo() or sponsor() check for this return value. Given that there are no other return values possible and that the callers never check for this return value, this can be removed safely to optimize and save gas. The function will be considered successful if it returns and if not will revert anyway.
Manual Analysis
Remove “return 0” from _depositToAave() and have it not return anything.
#0 - asselstine
2021-06-25T21:41:05Z
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
YearnV2YieldSource initialize does a zero-address check for value address to detect if it has already been initialized. This is an unnecessary check because vault address default value is zero, it is not initialized/set anywhere else and the initializer modifier will prevent the calling of initialize() a second time. So vault is guaranteed to be zero in initialize().
The impact is gas wastage from an additional SLOAD of vault state variable and the require() check.
Manual Analysis
Remove the zero-address check for vault.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
Using parameter _vault instead of SLOAD of state variable vault in the call to safeApprove() leads to gas savings of 100.
Manual Analysis
Using parameter _vault instead of state variable vault in the call to safeApprove()
#0 - PierrickGT
2021-07-01T14:26:18Z
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
token state variable is used in three places in _depositInVault(). It can be cached in a local variable at the beginning of the function to save 200 gas from two repeated SLOADs.
Manual Analysis
Cache token in a local variable at the beginning of the function and use that instead.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
token state variable is used in two places in _withdrawFromVault(). It can be cached in a local variable at the beginning of the function to save 100 gas from one repeated SLOAD.
Manual Analysis
Cache token in a local variable at the beginning of the function and use that instead.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
maxLosses state variable is used in two places in _withdrawFromVault(). It can be cached in a local variable at the beginning of the function to save 100 gas from one repeated SLOAD.
Manual Analysis
Cache maxLosses in a local variable at the beginning of the function and use that instead.
🌟 Selected for report: 0xRajeev
300.9419 USDC - $300.94
0xRajeev
Caching sushiAddr and sushiBar in local variables right at the beginning of supplyTokenTo() (similar to what's done in redeemToken) can save 100 gas from repeat SLOADs for each of them for a total savings of 200.
Manual Analysis
Caching sushiAddr and sushiBar in local variables at the beginning of supplyTokenTo() and use those instead.
#0 - kamescg
2021-06-30T07:43:31Z