Platform: Code4rena
Start Date: 16/09/2021
Pot Size: $50,000 USDC
Total HM: 26
Participants: 30
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 17
Id: 36
League: ETH
Rank: 4/30
Findings: 4
Award: $4,370.29
🌟 Selected for report: 18
🚀 Solo Findings: 0
131.4735 USDC - $131.47
0xRajeev
ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require() statements or use safe wrapper functions implementing return value/data checks to handle these failures. For reference, see similar Medium-severity finding from Consensys Diligence Audit of Aave Protocol V2: https://consensys.net/diligence/audits/2020/09/aave-protocol-v2/#unhandled-return-values-of-transfer-and-transferfrom
While the contract uses OpenZeppelin’s SafeERC20 functions in most places, it misses using safeTransfer() in two cases. The use of transfer() for basketAsERC20 token is ok because it is this protocol’s token. However, the use of transfer() for bounty token transfers is risky because bounty tokens are arbitrary tokens added by any one, including a malicious publisher.
The impact can be that for an arbitrary ERC20 token, this transfer() call may return failure but the bounty logic misses that, assumes it was successfully transferred to user.
Manual Analysis
Use safeTransfer instead of transfer given that the bounty token is untrusted.
#0 - GalloDaSballo
2021-12-19T22:24:27Z
Duplicate of #35
270.5216 USDC - $270.52
0xRajeev
The withdrawBounty() loops through the _bounties array looking for active bounties and transferring amounts from active ones. However, the data location specifier used for bounty is memory which makes a copy of the _bounties array member instead of a reference. So when bounty.active is set to false, this is changing only the memory copy and not the array element of the storage variable. This results in bounties never being set to inactive, keeping them always active forever and every withdrawBounty() will attempt to transfer bounty amount from the Auction contract to the msg.sender.
Therefore, while the transfer will work the first time, subsequent attempts to claim this bounty will revert on transfer (because the Auction contract will not have required amount of bounty tokens) causing withdrawBounty() to always revert and therefore preventing settling of any auction.
A malicious attacker can add a tiny bounty on any/every Auction contract to prevent any reindexing on that contract to happen because it will always revert on auction settling. This can be used to cause DoS on any auctionBonder so as to make them lose their bondAmount because their bonded auction cannot be settled.
https://docs.soliditylang.org/en/v0.8.7/types.html#data-location-and-assignment-behaviour
Manual Analysis
Recommend changing storage specifier of bounty to "storage" instead of “memory".
#0 - GalloDaSballo
2021-12-19T15:46:41Z
Great find, adding the same bounty multiple times can allow the exploiter to drain the entire contract as long as the bounties are denominated in the underlying token
#1 - GalloDaSballo
2021-12-21T22:30:10Z
Because the warden shows a way to cause DOS, which is contingent on setting it as well as redeeming that bounty (which can be avoided by simply not adding it to the bountiesID calldata), I will rate this finding as Medium Severity
The DOS is low severity as it can be sidestep very easily, see #82
However, the warden has identified a technical issue with the protocol (unflagging of bounty), as such I agree with a medium severity
90.1739 USDC - $90.17
0xRajeev
Zero-address checks are in general a best-practice. While this is performed in most places, the Factory constructor is missing these checks.
Manual Analysis
Add zero-address checks.
#0 - frank-beard
2021-10-19T17:34:50Z
not an exploit
#1 - GalloDaSballo
2021-12-01T22:30:59Z
Duplicate of #86
43.8245 USDC - $43.82
0xRajeev
With the use of untrusted tokens proposed by the publisher, coupled with the lack of reentrancy guards and absence of adherence to the checks-effects-interactions pattern, reentrancies are possible in multiple places. While it is not clear that they can be obviously abused to cause vulnerabilities, it may nevertheless be critical to evaluate assumptions and mitigate reentrancy risks.
Manual Analysis
Evaluate trust/threat assumptions and mitigate reentrancy risks by using CEI pattern and/or reentrancy guards.
#0 - frank-beard
2021-10-19T17:25:21Z
#1 - GalloDaSballo
2021-12-20T01:05:20Z
Duplicate of #270
90.1739 USDC - $90.17
0xRajeev
Lack of OpenZeppelin's initializer modifier or a contract variable (as in Auction initialized tracking) to prevent multiple initializations using initialize() function allows anyone to initialize after contract deployment/initializing (from Factory) with arbitrary addresses/parameters, e.g. for factory/auction/publisher addresses, and completely take over contract management and funds.
Manual Analysis
Allow initializing only once using an initialized bool logic similar to Auction contract.
#0 - GalloDaSballo
2021-12-04T17:38:26Z
Agree with finding, while the change can look trivial, allowing initialize to be called multiple times allows for a hostile takeover from any external caller, as such this is a high severity finding
#1 - GalloDaSballo
2021-12-04T17:41:11Z
Actually, after checking: #50
There is already an initializer check in __ERC20_init as such the initialze function cannot be called more than once
This means the exploit is not there, will downgrade to low and set #50 to be the main issue
#2 - GalloDaSballo
2021-12-04T17:42:06Z
Duplicate of #50
150.2898 USDC - $150.29
0xRajeev
Missing equality check on lengths of tokens and their weights in multiple places.
Example: Different lengths of inputTokens and inputWeights could cause reverts (inputTokens.length > inputWeights.length) or undefined behavior in case of mismatches (inputTokens.length < inputWeights.length).
Manual Analysis
Add require(tokens.length == weights.length) in all places where users input tokens and their weights.
#0 - frank-beard
2021-10-19T17:33:13Z
not an exploit
#1 - GalloDaSballo
2021-12-12T16:53:58Z
Duplicate of #111
150.2898 USDC - $150.29
0xRajeev
The protocol timelock duration assumes 4 blocks per minute i.e. 15s blocks whereas the current block time is ~13s and which is variable. This granularity/accuracy should be ok for current timelock purposes where a few seconds difference should not matter but recommend using seconds and block.timestamp instead of block.number for more granular/accurate time-based accounting.
https://etherscan.io/chart/blocktime
Manual Analysis
Consider using block.timestamp instead of block.number for more granular/accurate time-based accounting.
#0 - GalloDaSballo
2021-12-12T16:36:42Z
Duplicate of #218
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
There are multiple places where individual fields of struct are reset to default values. This is risky because it may lead to use of stale values left over in other struct fields in other/later logic/flows.
A cleaner approach is to delete the entire struct to make sure all its fields are set to default values and minimize the risk of any stale values being used in any flows. This also clearly indicates the intent of the developer.
Manual Analysis
Use delete on entire structs (recognizing that any mapping fields are not cleared as per Solidity semantics).
#0 - GalloDaSballo
2021-12-12T16:34:05Z
Agree with finding, not sure if informational (non-critical) as there's no poc here
Will leave as low severity for now as I believe that this can cause issues, but we don't have a specific example of how
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
Renouncing ownership is desirable in certain scenarios and is typically allowed by libraries such as Ownable. The same may be true of the publisher role in this protocol as well to prevent changing the license fee or re-indexing the basket forever.
This is typically done by assigning a zero address to such a role i.e. burning it. However, by requiring any new proposed publisher address to be != zero address, the current implementation does not provide an option to renounce a publisher role by burning it.
Manual Analysis
Consider adding support to renounce the publisher role or specify why this is not a desirable requirement for the protocol.
#0 - GalloDaSballo
2021-12-12T16:34:58Z
Agree with finding, this flags up an admin privilege which cannot be renounced with the current contract
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
While safeApprove is used in the Factory contract, the use of ERC20 approve in approveUnderlying() (instead of safeApprove) is presumably to handle the reapprovals during changing of index but is susceptible to the historical ERC20 approve() race condition.
Manual Analysis
Be aware that this is susceptible to race-condition but this it unlikely a concern because the spender is always the auction contract which is cloned and therefore trusted.
#0 - GalloDaSballo
2021-12-12T16:32:10Z
Valid finding as it discusess the race condition of changing approvals
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
While the documentation mentions that “The protocol is designed for standard ERC20 tokens, it is not currently concerned with the potential effects of rebasing or non-standard ERC20 implementations”, there is no logic, e.g. via whitelisting, that prevents a publisher from choosing such tokens for baskets. Accidental inclusion may result in undefined behavior, accounting miscalculations or even fund loss for interacting users.
https://github.com/code-423n4/2021-09-defiProtocol#info
Manual Analysis
Add logic to support inclusion/exclusion of such tokens or document the non-support warning explicitly to publishers and users.
#0 - GalloDaSballo
2021-12-12T16:41:40Z
Because of the lack of explicit blocking, even though the documentation mentions to exclude these tokens, the protocol allows any type of token due to it's simplicity, as such the code is inconsistent with the documentation and the finding is a valid low severity finding
🌟 Selected for report: 0xRajeev
0xRajeev
Critical contract parameters of non-address types, i,e. integers should have sanity/threshold checks in constructor/initialize/setter to ensure they are relevant from the application-logic’s perspective. Accidentally setting arbitrary values may lead to reverts or financial loss by transferring/withholding more rewards/fees than specified.
In the setters of minLicenseFee, auctionDecrement, auctionMultiplier, bondPercentDiv or ownerSplit, only the setOwnerSplit() setter has a threshold check against a maximum of 20%. None of the others have any checks.
Manual Analysis
Add sanity/threshold checks in constructor/initialize/setter for all critical contract parameters of non-address types.
#0 - GalloDaSballo
2021-12-12T16:45:55Z
Agree with the finding because some of these can hinder the functionality of the protocol, additionally adding boundaries gives more well defined security guarantees to the users
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
The current implementation does not check that tokenName & tokenSymbol are not the same as other ones already registered. This is required to prevent scams from using popular basket names but with random tokens/weights underneath. The only offchain monitoring for baskets is the tokenName emitted.
Exploit scenario: A basket of DeFi tokens lists as DPI and becomes very popular with a large TVL. A malicious publisher creates a basket with questionable DeFi tokens under the same basket token name/symbol of DPI. Users get tricked into depositing with this scam basket instead of the original one and risk losing their funds to underperformance of the duplicate basket token composition.
Manual Analysis
Add logic to check that tokenName & tokenSymbol are not the same as other ones already registered by keeping track of basket token names and symbols.
#0 - GalloDaSballo
2021-12-12T16:43:38Z
Definitely a feature of the protocol at this time I'll leave as Low Severity, personally would recommend adding a check for Symbol and Name as I've been told Etherscan will simply sort tokens by date rather than by "legitimacy" That said if the sponsor had disputed, this is a property of the system that users need to be aware of
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
A big aspect of a 2-step change, such as done with changePublisher() and changeLicenseFee(), is to allow any incorrectly used new addresses/values to be changed during the timelock period. This requires allowing the newPublisher or newLicenseFee to be a different value from the one used during the earlier approve and resetting the timelock again.
The current implementation only allows setting it once to a non-zero address/value and prevents any such corrections from being made (by checking that the address/value used is the same as that used during the first approve) which enforces the timelock to prevent surprises to users but does not provide the other accident benefits of using a timelock.
Manual Analysis
Recommend adding "&& pendingPublisher.publisher == newPublisher” and "&& pendingLicenseFee.licenseFee == newLicenseFee" to the if conditional predicate expression along with removing of the require() statement for equality check inside the conditional, to allow resetting the pending address/value to a new one if previously used one was incorrect.
#0 - GalloDaSballo
2021-12-19T22:20:24Z
The finding is valid, personally I'd mark this as low severity as it's in line with the "lack of input validation for setters" category
🌟 Selected for report: 0xRajeev
333.9773 USDC - $333.98
0xRajeev
While publishers are enforced a one day timelock for changePublisher() and changeLicenseFee() functions, Factory owner setter functions should also have timelocks (along with events) so that users and other privileged roles (publishers) can detect upcoming changes and have the time to react to them.
Changes to minLicenseFee, auctionDecrement, auctionMultiplier, bondPercentDiv or ownerSplit may have a financial or trust impact on users/publishers who should be given an opportunity to react to them by exiting/engaging without being surprised when such changes are made effective immediately.
None of the onlyOwner setters in the protocol have a timelock controller functionality to delay enforcement of the critical changes they make.
Manual Analysis
Consider adding timelocks to such contracts with critical setter functions.
#0 - GalloDaSballo
2021-12-12T16:46:59Z
This is nuanced, as the invariant of having a timelock could be enforced at the governance / owner level. However, since there's an implementation of timelock for other parameters, I do agree that these parameters should be held by the same standard
150.2898 USDC - $150.29
0xRajeev
Given that Factory contract is derived from Ownable, the ownership management of this contract defaults to Ownable’s transferOwnership() and renounceOwnership() methods which are not overridden here. Such critical address transfer/renouncing in one-step is very risky for Factory contract which controls the entire protocol because it is irrecoverable from any mistakes.
Interestingly, a 2-step change is used for changing of Basket’s Publisher and LicenseFee but it is more critical for the Factory contract.
Scenario: If an incorrect address, e.g. for which the private key is not known, is used accidentally then it prevents the use of all the onlyOwner() functions forever, which includes the changing of various critical addresses and parameters. This use of incorrect address may not even be immediately apparent given that these functions are probably not used immediately. When noticed, due to a failing onlyOwner() function call, it will force the redeployment of these contracts and require appropriate changes and notifications for switching from the old to new address. This will diminish trust in the protocol and incur a significant reputational damage.
See similar High Risk severity finding from Trail-of-Bits Audit of Hermez: https://github.com/trailofbits/publications/blob/master/reviews/hermez.pdf
See similar Medium Risk severity finding from Trail-of-Bits Audit of Uniswap V3: https://github.com/Uniswap/uniswap-v3-core/blob/main/audits/tob/audit.pdf
Manual Analysis
Override the inherited methods to null functions and use separate functions for a two-step address change: 1) Approve a new address as a pendingOwner 2) A transaction from the pendingOwner address claims the pending ownership change. This mitigates risk because if an incorrect address is used in step (1) then it can be fixed by re-approving the correct address. Only after a correct address is used in step (1) can step (2) happen and complete the address/ownership change.
Also, consider adding a timelock delay for such sensitive actions (similar to what’s done in Basket functions). And at a minimum, use a multisig (with mutually independent and trustworthy owners) and not an EOA.
#0 - frank-beard
2021-09-30T18:51:19Z
it is assumed the owner is trustworthy in this version of the protocol, however we will add mitigations and further decentralization in future updates
#1 - GalloDaSballo
2021-12-20T00:52:12Z
Duplicate of #89
25.9609 USDC - $25.96
0xRajeev
While it is generally a good idea to explicitly initialize variables, the default values of uints are 0 and do not require initializations to default values. Avoiding these will save gas.
Manual Analysis
Avoid initializations to default values
#0 - GalloDaSballo
2021-11-30T22:40:21Z
Duplicate of #39
4.3799 USDC - $4.38
0xRajeev
Rearranging the declaration order of state variables in Auction.sol by moving all 3 bool's next to each other will pack them in one slot because bool’s are represented as uint8 internally. This will save slots and gas if they're used together in functions by allowing optimizer to combine their SLOADs/SSTOREs. If they are not used together, this may increase gas costs due to the required masking of shared slot variables.
Manual Analysis
Rearrange state variable declaration order, by considering their usage in functions, to allow slot packing and save gas.
#0 - GalloDaSballo
2021-11-30T22:35:09Z
Duplicate of #109
15.5766 USDC - $15.58
0xRajeev
Auction and Basket implementation state variable addresses set in Factory’s constructor are never modified later. Making such state variables immutable will save their storage slots and avoid expensive SLOADs/SSTOREs for accessing them. Compiler replaces their usages in contract bytecode at construction time.
Manual Analysis
Add immutable specifier for auctionImpl and basketImpl.
#0 - GalloDaSballo
2021-11-30T22:41:16Z
Duplicate of #15
25.9609 USDC - $25.96
0xRajeev
For Auction:initialize(), there is no need to pass Factory.address as an argument when Auction.initialize() can determine that by using msg.sender just like Basket.initialize(). These initialize functions are only called by the Factory contract.
Manual Analysis
Remove the second parameter of factory address for Auction.initialize() function.
#0 - GalloDaSballo
2021-11-30T22:26:45Z
Duplicate of #137
25.9609 USDC - $25.96
0xRajeev
Using unchecked{} where the underlying arithmetic can be determined to not overflow/underflow will save gas from skipping the built-in Solidity checks of compiler version >= 0.8.0.
Example 1: based on the control flow, we can determine that lastFee will always be less than block.timestamp and therefore we can use unchecked{} for their subtraction on Basket:L116 because there is no risk of underflow. Example 2: Given the mints, we can be assured that totalSupply() >= startSupply and therefore the RHS on L124 will be <= ibRatio and therefore we can use unchecked{} for the calculation.
Manual Analysis
Use unchecked{} where the underlying arithmetic can be determined to not overflow/underflow
#0 - GalloDaSballo
2021-11-30T22:29:37Z
Agree with finding
#1 - GalloDaSballo
2021-11-30T22:29:51Z
Duplicate of #135
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
Given that there is no removal of claimed/inactive bounties, the bounty list could grow very long over time requiring a lot of gas for traversal.
Manual Analysis
Recommend pruning the claimed bounties by deleting them from the list.
#0 - GalloDaSballo
2021-11-30T22:24:48Z
Agree with the finding, have seen in other contest that when going above 100 entries, gas costs tend to get extremely high
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
Choosing either named return or explicit instead of specifying both may reduce gas due to unnecessary bytecode introduced. proposeBasketLicense() uses a named return variable which is never assigned and instead uses an explicit return statement.
Manual Analysis
Choose either explicit return or named return, not both
#0 - GalloDaSballo
2021-11-30T22:27:55Z
Agree with the finding, not sure about the exact cost, I'm assuming you end up paying the additional cost of allocating 0 (default value) to an unused variable, as well as the extra bytecode deployed for said declaration
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
The unique token checking loop can be skipped for i == 0 as indicated in the code comment but never implemented. This will save a few bytecode in loop set up and index check.
Manual Analysis
Add if (i != 0) {loop}
#0 - GalloDaSballo
2021-11-30T22:31:14Z
The finding is valid, almost odd resolution
Would recommend having a require tokens.length != 0
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
Unnecessary return of argument value via state variable which costs a SLOAD, returns the same value as argument back to caller where the return value is ignored.
Manual Analysis
Remove return value for this function.
#0 - GalloDaSballo
2021-11-30T22:29:10Z
Valid finding, these can look trivial but end up saving potentially up to 2k gas
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
For loop indices across the contract functions use explicit 0 initializations which are not required because the default value of uints is 0. Removing this explicit unnecessary initialization will save a little gas.
Manual Analysis
Remove unnecessary initialization of loop index variable
#0 - GalloDaSballo
2021-11-30T22:34:05Z
Agree with finding, I appreciate that the warden listed out all instances
Assigning the same value to a variable in memory ends up costing 3 gas Assigning to a value in storage costs a lot more (seems to be 1k gas: https://ethereum.stackexchange.com/questions/86991/zero-initialisation-of-storage-values-in-constructor-or-gas-optimization)
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
There are places across contracts where the same external calls are made multiple times within a function. Caching return values of such calls in local/memory variables avoids CALLs to save gas. CALLs cost 2600 gas after Berlin upgrade. MLOADs cost only 3 gas units.
Cache factory.ownerSplit() return value to save 2600 gas in this function which gets called at every mint/burn.: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L120-L121
Hoist basketAsERC20.totalSupply() external call out of the loop because it remains the same and each call costs 2600 gas: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L96-L99
Manual Analysis
Cache return values of external calls in local/memory variables
#0 - GalloDaSballo
2021-11-30T22:37:06Z
The warden is pointing out to something important, which the importance of caching external calls
However the gas values do not seem to be up to date The cost of STATICCALL is not 2.6k additionally the repeated STATICCALL will cost about 100 gas
However, the warden is correct in saying that caching the value will save gas (as reading from memory only costs 3 gas)
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
There are numerous places across contracts where the same state variables are read multiple times within a function. Caching state variables in local/memory variables avoids SLOADs to save gas. Warm SLOADs cost 100 gas after Berlin upgrade. MLOADs cost only 3 gas units.
Cache basket, bondTimestamp, factory: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L78-L99
Cache basket and bondAmount: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Auction.sol#L116-L121
Cache pendingWeights: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L50
Cache pendingPublisher: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L136-L139
Cache pendingLicenseFee: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L154-L157
Cache auction & pendingWeights: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L173-L182
Cache publisher and auction: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Basket.sol#L208-L213
Cache proposals: https://github.com/code-423n4/2021-09-defiProtocol/blob/52b74824c42acbcd64248f68c40128fe3a82caf6/contracts/contracts/Factory.sol#L62
Manual Analysis
Cache state variables in local/memory variables to save gas.
#0 - GalloDaSballo
2021-11-30T22:37:55Z
Thank you Rajeev for the list, 100% agree with caching these instead of reading from storage
Even if you read just 2 times, using the cache-in-memory pattern will save gas
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
Public/external setter functions or any others that emit events to indicate the newly set values can use the function parameters (from memory/calldata) or their cached local versions as emit arguments instead of the state variables that were just written to because SLOADs cost 2100/100 gas depending on cold/warm reads from Berlin upgrade onwards.
Manual Analysis
Use function parameters or cached local variables in emits to save gas.
#0 - GalloDaSballo
2021-11-30T22:39:21Z
Absolutely agree with the warden, I appreciate the list of links as well as the note on the 2.1k (cold SLOAD) and (100 hot SLOAD)
🌟 Selected for report: 0xRajeev
57.691 USDC - $57.69
0xRajeev
Unused constant BLOCK_DECREMENT may be an indication of missing logic or redundant code. In this case, this appears to be a redundant constant same as Factory.auctionDecrement.
Manual Analysis
Use the constant or remove it.
#0 - frank-beard
2021-10-19T17:33:50Z
not an exploit
#1 - GalloDaSballo
2021-12-12T17:05:57Z
Changing to gas
4.3799 USDC - $4.38
0xRajeev
There are many public visibility calls across contracts that could be made external because they are not called from within contracts. Public visibility forces a copy of function parameters from calldata to memory which consumes gas depending on the number of parameters.
Manual Analysis
Change public visibility to external to save gas.
#0 - GalloDaSballo
2021-11-30T22:38:37Z
Duplicate of #240