|
| 1 | +# Best Practices |
| 2 | + |
| 3 | +Below, please find a list of best practices when interacting with the Uniswap Protocol Fee system. |
| 4 | + |
| 5 | +## Calldata Nonce |
| 6 | + |
| 7 | +Recall [`Firepit.release()`](../technical-reference/IReleaser#release) requires a `_nonce` parameter that matches the contract storage [`Firepit.nonce()`](../technical-reference/INonce#nonce). |
| 8 | + |
| 9 | +The nonce mechanism prevents front-running attacks, where a successful front-run would cause the victim |
| 10 | +to burn a substantial amount of UNI in exchange for little to no assets. The nonce mechanism ensures sequencing, so **if two competing transactions |
| 11 | +use the same nonce, only one will succeed** |
| 12 | + |
| 13 | +:::danger |
| 14 | +Integrating *contracts* should **NEVER** read the `nonce` at the time of the transaction |
| 15 | + |
| 16 | +```solidity |
| 17 | +contract Vulnerable { |
| 18 | + IFirepit public firepit; |
| 19 | +
|
| 20 | + function vulnerable(Currency[] memory assets) external { |
| 21 | + // ❌ THIS IS VULNERABLE TO FRONT RUNNING ❌ // |
| 22 | + uint256 nonce = firepit.nonce(); |
| 23 | + firepit.release(nonce, assets, address(this)); |
| 24 | + } |
| 25 | +} |
| 26 | +``` |
| 27 | +::: |
| 28 | + |
| 29 | +:::tip |
| 30 | +```solidity |
| 31 | +contract SafeRelease { |
| 32 | + IFirepit public firepit; |
| 33 | +
|
| 34 | + // ✅ THIS IS SAFE FROM FRONT RUNNING ✅ // |
| 35 | + function safeRelease(uint256 _nonce, Currency[] memory assets) external { |
| 36 | + firepit.release(_nonce, assets, address(this)); |
| 37 | + } |
| 38 | +} |
| 39 | +``` |
| 40 | +In the safe pattern, the caller is responsible for reading the `Firepit.nonce()` offchain, assocating it with releasable assets, and passing it as a **calldata** parameter |
| 41 | +::: |
| 42 | + |
| 43 | +## Collect Uniswap v3 Fees |
| 44 | + |
| 45 | +For further reading, please see [Read Asset Balance](./read-asset-balance) |
| 46 | + |
| 47 | +In the Uniswap Protocol Fee system, Uniswap v3 fees *must be collected* to the `AssetSink` before they are eligible for release. |
| 48 | + |
| 49 | +:::tip |
| 50 | +We recommend that integrators check and collect Uniswap v3 fees before calling `Firepit.release()`, to ensure the maximum assets are released. |
| 51 | + |
| 52 | +Collection is available via |
| 53 | +[IV3FeeController.collect()](../technical-reference/IV3FeeController#collect) |
| 54 | + |
| 55 | +```solidity |
| 56 | +IV3FeeController v3FeeController; |
| 57 | +
|
| 58 | +function collectAndRelease( |
| 59 | + IV3FeeController.CollectParams memory collectParams, |
| 60 | + uint256 _nonce, |
| 61 | + Currency[] memory assets, |
| 62 | + address recipient |
| 63 | +) external { |
| 64 | + v3FeeController.collect(collectParams); |
| 65 | + firepit.release(_nonce, assets, recipient); |
| 66 | +} |
| 67 | +``` |
| 68 | +::: |
| 69 | + |
| 70 | +## UNI Approvals |
| 71 | + |
| 72 | +The Uniswap Protocol Fee system allows for an *updatable* [`Firepit.threshold()`](../technical-reference/IResourceManager#threshold) |
| 73 | + |
| 74 | +While the system does not intend to maliciously front-run `.release()` calls, max-approving UNI allowances may lead to an unexpectedly higher burn of UNI. |
| 75 | +The risk only appears if and only if the `thresholdSetter` increases the `threshold` amount while there is a pending `.release()` call. |
| 76 | + |
| 77 | +Integrators can avoid this risk by: |
| 78 | + |
| 79 | +* only holding an amount of UNI they are willing to burn |
| 80 | +* approving only the amount of UNI they intend to burn |
| 81 | +* perform balance checks before and after calling `.release()`, comparing the burned amount against a calldata value |
| 82 | + |
| 83 | +## Payable Contracts |
| 84 | + |
| 85 | +Because the Uniswap Protocol Fee system can release native tokens (Ether), recipients of the tokens should be `payable` |
| 86 | + |
| 87 | +## Pricing Uniswap v2 ERC-20 Tokens |
| 88 | + |
| 89 | +For further reading, please see [Read Asset Balance](./read-asset-balance#uniswap-v2-pool-protocol-fees) |
| 90 | + |
| 91 | +Uniswap v2 Protocol Fees are automatically pushed to the `AssetSink` contract, but are represented as ERC-20 LP tokens. These ERC-20 LP tokens represent a combination of `token0` and `token1` |
| 92 | + |
| 93 | +To compute the underlying amount of `token0` and `token1` represented by the LP tokens, integrators can use the following formula: |
| 94 | + |
| 95 | +```solidity |
| 96 | +IUniswapV2Pair pair; |
| 97 | +
|
| 98 | +function getUnderlyingAmounts(uint256 lpTokens) external view returns (uint256 amount0, uint256 amount1) { |
| 99 | + uint256 totalSupply = pair.totalSupply(); |
| 100 | + uint256 poolBalance0 = IERC20(pair.token0()).balanceOf(address(pair)); |
| 101 | + uint256 poolBalance1 = IERC20(pair.token1()).balanceOf(address(pair)); |
| 102 | +
|
| 103 | + amount0 = (lpTokens * poolBalance0) / totalSupply; |
| 104 | + amount1 = (lpTokens * poolBalance1) / totalSupply; |
| 105 | +} |
| 106 | +``` |
| 107 | + |
| 108 | +:::note |
| 109 | +Integrators should perform additional validation, i.e. pool price and slippage checks as to not |
| 110 | +misrepresent the LP token's underlying token amounts |
| 111 | +::: |
0 commit comments