Elvishmagic

The `ElvishMagic2` contracts have various address fields with hardcoded addresses. This design is risky as it prevents the dynamic assignment of addresses and uses a predefined address, which may not always be desirable or secure. This needs to be handled especially carefully in this case since many of the addresses refer to testnet addresses.

A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by backrunning (executing after the target) a transaction to sell the asset. The following functions are called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large: - `swap()` ```solidity=555 uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens( tokenAmount, 0, path, address(this), block.timestamp); ``` Furthermore, the attacker could also take advatange of this issue to launch a flashloan attack. For example, supposed currently, the contract's balance is close to threshold for a `swap()`. - The attacker flashloaned EMAGIC tokens and swap for WETH - The attacker triggers the swap by transferring some EMAGIC tokens - The attacker swap back WETH for more EMAGIC tokens.

The contract is serving as the underlying entity to interact with one or more third party protocols. The scope of the audit treats third party entities as black boxes and assumes their functional correctness. However, in the real world, third parties can be compromised and this may lead to lost or stolen assets. In addition, upgrades of third parties can possibly create severe impacts, such as increasing fees of third parties, migrating to new LP pools, etc. ```solidity=377 IUniswapV2Router02 public uniswapV2Router; ``` - The contract `ElvishMagic2` interacts with third party contract with `IUniswapV2Router02` interface via `uniswapV2Router`.

The return values of the `transfer()` and `transferFrom()` calls in the smart contract are not checked. Some ERC-20 tokens' transfer functions return no values, while others return a bool value, they should be handled with care. If a function returns `false` instead of reverting upon failure, an unchecked failed transfer could be mistakenly considered successful in the contract. ```solidity=450 ERC20token.transfer(msg.sender, balance); ```

It is not recommended to use Solidity's `transfer()` and `send()` functions for transferring native tokens, since some contracts may not be able to receive the funds. Those functions forward only a fixed amount of gas (2300 specifically) and the receiving contracts may run out of gas before finishing the transfer. Also, EVM instructions' gas costs may increase in the future. Thus, some contracts that can receive now may stop working in the future due to the gas limitation. ```solidity=562 bool success = payable(marketingWallet).send(newBalance); ``` - `ElvishMagic2.swap` uses `send()`.

The linked variables assigned in the constructor can be declared as `immutable`. Immutable state variables can be assigned during contract creation but will remain constant throughout the lifetime of a deployed contract. A big advantage of immutable variables is that reading them is significantly cheaper than reading from regular state variables since they will not be stored in storage.