Juicebox Buyback Delegate - 0x4non's results

QA

L-01 Ensure 18 decimals

According to juicebox glossary

Projects can issue their own ERC-20 token directly from the protocol to use as its token. Projects can also bring their own token as long as it conforms to the IJBToken interface and uses 18 decimal fixed point accounting. This makes it possible to use ERC-1155's or custom tokens.

Recommendation

Add check to ensure _projectToken has 18 decimals.

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..5743e05 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -121,6 +121,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         IUniswapV3Pool _pool,
         IJBPayoutRedemptionPaymentTerminal3_1 _jbxTerminal
     ) {
+        require(IJBToken(address(_projectToken)).decimals() == 18, "JBXBuybackDelegate: project token decimals must be 18");
         projectToken = _projectToken;
         pool = _pool;
         jbxTerminal = _jbxTerminal;

L-02 Add check to ensure that projectId == _data.projectId on payParams

According to juicebox glossary

Projects can issue their own ERC-20 token directly from the protocol to use as its token. Projects can also bring their own token as long as it conforms to the IJBToken interface and uses 18 decimal fixed point accounting. This makes it possible to use ERC-1155's or custom tokens.

Note that in test you are using Tickets on 0x3abf2a4f8452ccc2cf7b4c1e4663147600646f66 this token doesnt respect the IJBToken

I think payParams shold implement this check require(projectId == _data.projectId, "projectId mismatch");

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..0a20e3f 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -78,6 +78,9 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      */
     IERC20 public immutable projectToken;
 
+    /// @notice The project id
+    uint256 private immutable _projectId;
+
     /**
      * @notice The uniswap pool corresponding to the project token-other token market
      *         (this should be carefully chosen liquidity wise)
@@ -122,6 +125,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         IJBPayoutRedemptionPaymentTerminal3_1 _jbxTerminal
     ) {
         projectToken = _projectToken;
+        projectId = JBXToken(address(_projectToken)).projectId();
         pool = _pool;
         jbxTerminal = _jbxTerminal;
         _projectTokenIsZero = address(_projectToken) < address(_weth);
@@ -146,6 +150,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         override
         returns (uint256 weight, string memory memo, JBPayDelegateAllocation[] memory delegateAllocations)
     {
+        require(projectId == _data.projectId, "projectId mismatch");
         // Find the total number of tokens to mint, as a fixed point number with 18 decimals
         uint256 _tokenCount = PRBMath.mulDiv(_data.amount.value, _data.weight, 10 ** 18);

L-03 Since projectToken can be any token custom token its possible to reenter

projectToken could be a token with hooks, opening the possibility of reentrancy from _data.beneficiary after the token is transfered to _data.beneficiary in the _swap function on JBXBuybackDelegate.sol#L286

L-04 Since projectToken can be any token custom token its possible to dos

projectToken could be a token with hooks, opening the possibility of DOS. The attacker in this case is the _data.beneficiary and he just have to craft a contract that consume all the gas on the hook on transfer triggered on JBXBuybackDelegate.sol#L286

NC Use IERC20 instead of IJBToken

According to juicebox glossary

Projects can issue their own ERC-20 token directly from the protocol to use as its token. Projects can also bring their own token as long as it conforms to the IJBToken interface and uses 18 decimal fixed point accounting. This makes it possible to use ERC-1155's or custom tokens.

Therefore i infer that IERC20 _projectToken is a IJBToken and would be clear if you just use IJBToken and remove IERC20

Recommendation Out of scope but important, interface IJBToken should extend the IERC20; interface IJBToken is IERC20

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..ab53cd7 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -13,7 +13,6 @@ import "@jbx-protocol/juice-contracts-v3/contracts/structs/JBDidPayData.sol";
 import "@jbx-protocol/juice-contracts-v3/contracts/structs/JBPayParamsData.sol";
 
 import "@openzeppelin/contracts/access/Ownable.sol";
-import "@openzeppelin/contracts/interfaces/IERC20.sol";
 
 import "@paulrberg/contracts/math/PRBMath.sol";
 
@@ -76,7 +75,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      * 
      * @dev In this context, this is the tokenOut
      */
-    IERC20 public immutable projectToken;
+    IJBToken public immutable projectToken;
 
     /**
      * @notice The uniswap pool corresponding to the project token-other token market
@@ -116,7 +115,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      * @dev No other logic besides initializing the immutables
      */
     constructor(
-        IERC20 _projectToken,
+        IJBToken _projectToken,
         IWETH9 _weth,
         IUniswapV3Pool _pool,
         IJBPayoutRedemptionPaymentTerminal3_1 _jbxTerminal

</details>

NC Remove unused imports

@jbx-protocol/juice-contracts-v3/contracts/interfaces/IJBFundingCycleBallot.sol is not being use and can be remove

NC Use named imports

Instead of import "file.sol"; use import {File} from "file.sol", example;

import {IJBController} from "@jbx-protocol/juice-contracts-v3/contracts/interfaces/IJBController3_1.sol";

NC Use fixed pragma

In JBXBuybackDelegate.sol#L2 Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Take a look into SWC-103

NC Ownable is not used and should be remove

There is no function using the onlyOwner modifier from Ownable imported in JBXBuybackDelegate.sol#L15 i think this import should be removed.

NC Better vatiable naming

• Variable SLIPPAGE_DENOMINATOR should be renamed to BPS, JBXBuybackDelegate.sol#L68
• Variable mintedAmount should be renamed mutexMintedAmount, JBXBuybackDelegate.sol#L106
• Variable reservedRate should be renamed mutexReservedRate, JBXBuybackDelegate.sol#L113

NC Private variables dont respect naming convention

All private variables should start with _ but SLIPPAGE_DENOMINATOR, mintedAmount and JBXBuybackDelegate.sol#L113 dont respect the convetion.

NC Immutables variables dont respect naming convention

Immutables should be in uppercase like SLIPPAGE_DENOMINATOR, but _projectTokenIsZero, projectToken and pool dont respect the convention

Typo in comment

* @notice Mint the token out, sending back the token in in the terminal on should be;

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..5a59e9a 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -326,7 +326,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
     }
 
     /**
-     * @notice Mint the token out, sending back the token in in the terminal
+     * @notice Mint the token out, sending back the token in the terminal
      *
      * @param  _data the didPayData passed by the terminal
      * @param  _amount the amount of token out to mint

Summary

A majority of the optimizations were benchmarked via the protocol's tests, i.e. using the following config: solc version 0.8.19, optimizer on, and 200 runs. I have turn on the optimizer, and its using solidity version 0.8.19 because it has and open pragma; pragma solidity ^0.8.16;

Gas Optimizations

---

<h3 id="g01"> G-01 Remove `Ownable`, since its not used </h3>

Note, this will also reduce deploy size.

Overall gas change: -90 (-0.001%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..94068b6 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -12,7 +12,6 @@ import "@jbx-protocol/juice-contracts-v3/contracts/libraries/JBTokens.sol";
 import "@jbx-protocol/juice-contracts-v3/contracts/structs/JBDidPayData.sol";
 import "@jbx-protocol/juice-contracts-v3/contracts/structs/JBPayParamsData.sol";
 
-import "@openzeppelin/contracts/access/Ownable.sol";
 import "@openzeppelin/contracts/interfaces/IERC20.sol";
 
 import "@paulrberg/contracts/math/PRBMath.sol";
@@ -36,7 +35,7 @@ import "./interfaces/external/IWETH9.sol";
  *         liquidity, this delegate needs to be redeployed.
  */
 
-contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUniswapV3SwapCallback, Ownable {
+contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUniswapV3SwapCallback {
     using JBFundingCycleMetadataResolver for JBFundingCycle;
 
     //*********************************************************************//

</details>

---

<h3 id="g02"> G-02 `jbxTerminal.directory()` staticcall return can be set as `immutable` </h3>

The jbxTerminal is immutable, and according to documentation JBController3_0_1.directory() is immutable, therefore, jbxTerminal.directory() can be setup in a immutable.

test_swapIfQuoteBetter(uint256) (gas: -752 (-0.067%)) 
test_mintIfSlippageTooHigh() (gas: -752 (-0.069%)) 
test_mintIfPreferClaimedIsFalse() (gas: -752 (-0.076%)) 
test_swapMultiple() (gas: -1504 (-0.110%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -686 (-0.258%)) 
Overall gas change: -4446 (-0.061%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..dab3ab3 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -112,6 +112,9 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      */
     uint256 private reservedRate = 1;
 
+    /// @dev The directory of the terminal
+    IJBDirectory private immutable _TERMINAL_DIRECTORY;
+
     /**
      * @dev No other logic besides initializing the immutables
      */
@@ -126,6 +129,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         jbxTerminal = _jbxTerminal;
         _projectTokenIsZero = address(_projectToken) < address(_weth);
         weth = _weth;
+        _TERMINAL_DIRECTORY = jbxTerminal.directory();
     }
 
     //*********************************************************************//
@@ -287,7 +291,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
 
         // If there are reserved token, add them to the reserve
         if (_reservedToken != 0) {
-            IJBController controller = IJBController(jbxTerminal.directory().controllerOf(_data.projectId));
+            IJBController controller = IJBController(_TERMINAL_DIRECTORY.controllerOf(_data.projectId));
 
             // 1) Burn all the reserved token, which are in this address -> result: 0 here, 0 in reserve
             controller.burnTokensOf({
@@ -332,7 +336,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      * @param  _amount the amount of token out to mint
      */
     function _mint(JBDidPayData calldata _data, uint256 _amount) internal {
-        IJBController controller = IJBController(jbxTerminal.directory().controllerOf(_data.projectId));
+        IJBController controller = IJBController(_TERMINAL_DIRECTORY.controllerOf(_data.projectId));
 
         // Mint to the beneficiary with the fc reserve rate
         controller.mintTokensOf({

</details>

---

<h3 id="g03"> G-03 `didPay(...)` Can be simplified </h3>

Consider the next refactor in the diff to reduce the if branch.

test_swapIfQuoteBetter(uint256) (gas: -11 (-0.001%)) 
test_swapRandomAmountIn(uint256) (gas: -11 (-0.001%)) 
test_mintIfSlippageTooHigh() (gas: -11 (-0.001%)) 
test_swapMultiple() (gas: -22 (-0.002%)) 
test_mintIfPreferClaimedIsFalse() (gas: 27 (0.003%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -11 (-0.004%)) 
Overall gas change: -39 (-0.001%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..122be21 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -196,16 +196,14 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         (,, uint256 _quote, uint256 _slippage) = abi.decode(_data.metadata, (bytes32, bytes32, uint256, uint256));
         uint256 _minimumReceivedFromSwap = _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR);
 
+        uint256 _amountReceived;
         // Pick the appropriate pathway (swap vs mint), use mint if non-claimed prefered
         if (_data.preferClaimedTokens) {
             // Try swapping
-            uint256 _amountReceived = _swap(_data, _minimumReceivedFromSwap, _reservedRate);
-
-            // If swap failed, mint instead, with the original weight + add to balance the token in
-            if (_amountReceived == 0) _mint(_data, _tokenCount);
-        } else {
-            _mint(_data, _tokenCount);
+            _amountReceived = _swap(_data, _minimumReceivedFromSwap, _reservedRate);
         }
+        // If swap failed, mint instead, with the original weight + add to balance the token in
+        if (_amountReceived == 0) _mint(_data, _tokenCount);
     }
 
     /**

</details>

---

<h3 id="g04"> G-04 Ternary evaluation of `_amountReceived` and `_amountToSend` can be optimized </h3>

test_swapIfQuoteBetter(uint256) (gas: -21 (-0.002%)) 
test_swapRandomAmountIn(uint256) (gas: -21 (-0.002%)) 
test_mintIfSlippageTooHigh() (gas: -21 (-0.002%)) 
test_swapMultiple() (gas: -42 (-0.003%)) 
testRevertIfSlippageIsTooMuchWhenSwapping() (gas: -21 (-0.171%)) 
Overall gas change: -126 (-0.002%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..3648c77 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -221,9 +221,9 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         (uint256 _minimumAmountReceived) = abi.decode(data, (uint256));
 
         // Assign 0 and 1 accordingly
-        uint256 _amountReceived = uint256(-(_projectTokenIsZero ? amount0Delta : amount1Delta));
-        uint256 _amountToSend = uint256(_projectTokenIsZero ? amount1Delta : amount0Delta);
-
+        (uint256 _amountReceived, uint256 _amountToSend) = (_projectTokenIsZero)
+            ? (uint256(-amount0Delta), uint256(amount1Delta))
+            : (uint256(-amount1Delta), uint256(amount0Delta));
         // Revert if slippage is too high
         if (_amountReceived < _minimumAmountReceived) revert JuiceBuyback_MaximumSlippage();

</details>

---

<h3 id="g05"> G-05 Expression `""` is cheaper than `new bytes(0)` </h3>

test_mintIfSlippageTooHigh() (gas: -259 (-0.024%)) 
test_mintIfPreferClaimedIsFalse() (gas: -259 (-0.026%)) 
Overall gas change: -518 (-0.007%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..cf7f54d 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -346,7 +346,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
 
         // Send the eth back to the terminal balance
         jbxTerminal.addToBalanceOf{value: _data.amount.value}(
-            _data.projectId, _data.amount.value, JBTokens.ETH, "", new bytes(0)
+            _data.projectId, _data.amount.value, JBTokens.ETH, "", ""
         );
 
         emit JBXBuybackDelegate_Mint(_data.projectId);

</details>

---

<h3 id="g06"> G-06 `sqrtPriceLimitX96` value can be `immutable` </h3>

Since _projectTokenIsZeroand TickMath.MAX_SQRT_RATIO are values that never change, we can assume that the next expression will never change; _projectTokenIsZero ? TickMath.MAX_SQRT_RATIO - 1 : TickMath.MIN_SQRT_RATIO + 1

test_swapIfQuoteBetter(uint256) (gas: -106 (-0.010%)) 
test_swapRandomAmountIn(uint256) (gas: -106 (-0.010%)) 
test_mintIfSlippageTooHigh() (gas: -106 (-0.010%)) 
test_swapMultiple() (gas: -212 (-0.016%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -106 (-0.040%)) 
Overall gas change: -636 (-0.009%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..3c533a8 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -94,6 +94,9 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
      */
     IWETH9 public immutable weth;
 
+    /// @dev used in `_swap` for `pool.swap`
+    uint160 private immutable _SQRTPRICELIMITX96;
+
     //*********************************************************************//
     // --------------------- private stored properties ------------------- //
     //*********************************************************************//
@@ -126,6 +129,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         jbxTerminal = _jbxTerminal;
         _projectTokenIsZero = address(_projectToken) < address(_weth);
         weth = _weth;
+        _SQRTPRICELIMITX96 = _projectTokenIsZero ? TickMath.MAX_SQRT_RATIO - 1 : TickMath.MIN_SQRT_RATIO + 1;
     }
 
     //*********************************************************************//
@@ -264,7 +268,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
             recipient: address(this),
             zeroForOne: !_projectTokenIsZero,
             amountSpecified: int256(_data.amount.value),
-            sqrtPriceLimitX96: _projectTokenIsZero ? TickMath.MAX_SQRT_RATIO - 1 : TickMath.MIN_SQRT_RATIO + 1,
+            sqrtPriceLimitX96: _SQRTPRICELIMITX96,
             data: abi.encode(_minimumReceivedFromSwap)
         }) returns (int256 amount0, int256 amount1) {
             // Swap succeded, take note of the amount of projectToken received (negative as it is an exact input)

</details>

---

<h3 id="g07"> G-07 Use `assembly` on safe math operations </h3>

Instead of uint256 value = _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR); you could do;

uint256 value;
assembly {
  value := mul(_quote, div(_slippage, SLIPPAGE_DENOMINATOR))
}
value = _quote - value;

test_mintIfPreferClaimedIsFalse() (gas: -515 (-0.052%)) 
test_mintIfWeightGreatherThanPrice(uint256) (gas: -860 (-0.090%)) 
test_swapIfQuoteBetter(uint256) (gas: -1065 (-0.096%)) 
testDatasourceDelegateMintIfPreferenceIsNotToClaimTokens() (gas: -1545 (-0.814%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -2231 (-0.840%)) 
testDatasourceDelegateWhenQuoteIsLowerThanTokenCount(uint256) (gas: -1994 (-1.046%)) 
test_swapMultiple() (gas: -20268 (-1.488%)) 
test_swapRandomAmountIn(uint256) (gas: -20337 (-1.876%)) 
testRevertIfSlippageIsTooMuchWhenSwapping() (gas: 474 (3.856%)) 
Overall gas change: -48860 (-0.673%)
test_mintIfWeightGreatherThanPrice(uint256) (gas: -128 (-0.013%)) 
test_swapIfQuoteBetter(uint256) (gas: -269 (-0.024%)) 
test_swapRandomAmountIn(uint256) (gas: -269 (-0.025%)) 
test_mintIfSlippageTooHigh() (gas: -269 (-0.025%)) 
test_mintIfPreferClaimedIsFalse() (gas: -269 (-0.027%)) 
test_swapMultiple() (gas: -538 (-0.040%)) 
testDatasourceDelegateWhenQuoteIsLowerThanTokenCount(uint256) (gas: -128 (-0.067%)) 
testDatasourceDelegateMintIfPreferenceIsNotToClaimTokens() (gas: -128 (-0.067%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -269 (-0.101%)) 
Overall gas change: -2267 (-0.031%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..9509faf 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -152,8 +152,14 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
         // Unpack the quote from the pool, given by the frontend
         (,, uint256 _quote, uint256 _slippage) = abi.decode(_data.metadata, (bytes32, bytes32, uint256, uint256));
 
+        uint256 _minimumReceivedFromSwap;
+        assembly {
+            _minimumReceivedFromSwap := mul(_quote, div(_slippage, SLIPPAGE_DENOMINATOR))
+        }
+        _minimumReceivedFromSwap = _quote - _minimumReceivedFromSwap;
+
         // If the amount swapped is bigger than the lowest received when minting, use the swap pathway
-        if (_tokenCount < _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR)) {
+        if (_tokenCount < _minimumReceivedFromSwap) {
             // Pass the quote and reserve rate via a mutex
             mintedAmount = _tokenCount;
             reservedRate = _data.reservedRate;
@@ -194,7 +200,11 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
 
         // The minimum amount of token received if swapping
         (,, uint256 _quote, uint256 _slippage) = abi.decode(_data.metadata, (bytes32, bytes32, uint256, uint256));
-        uint256 _minimumReceivedFromSwap = _quote - (_quote * _slippage / SLIPPAGE_DENOMINATOR);
+        uint256 _minimumReceivedFromSwap;
+        assembly {
+            _minimumReceivedFromSwap := mul(_quote, div(_slippage, SLIPPAGE_DENOMINATOR))
+        }
+        _minimumReceivedFromSwap = _quote - _minimumReceivedFromSwap;
 
         // Pick the appropriate pathway (swap vs mint), use mint if non-claimed prefered
         if (_data.preferClaimedTokens) {

</details>

---

<h3 id="g08"> G-08 Use `msg.sender` instead of `pool` </h3>

We can infeer that msg.sender is pool, because of this validadation

Same can be done with jbxTerminal in function didPay there is an access control that guarantee that msg.sender is jbxTerminal. I didnt suggest it because G-02 tackle this issue.

test_swapIfQuoteBetter(uint256) (gas: -7 (-0.001%)) 
test_swapRandomAmountIn(uint256) (gas: -7 (-0.001%)) 
test_swapMultiple() (gas: -14 (-0.001%)) 
Overall gas change: -28 (-0.000%)

Recommendation:

<details> <summary>Diff</summary>

diff --git a/juice-buyback/contracts/JBXBuybackDelegate.sol b/juice-buyback/contracts/JBXBuybackDelegate.sol
index 0ee751b..3490ac3 100644
--- a/juice-buyback/contracts/JBXBuybackDelegate.sol
+++ b/juice-buyback/contracts/JBXBuybackDelegate.sol
@@ -229,7 +229,7 @@ contract JBXBuybackDelegate is IJBFundingCycleDataSource, IJBPayDelegate, IUnisw
 
         // Wrap and transfer the weth to the pool
         weth.deposit{value: _amountToSend}();
-        weth.transfer(address(pool), _amountToSend);
+        weth.transfer(msg.sender, _amountToSend);
     }
 
     function redeemParams(JBRedeemParamsData calldata _data)

</details>

---

<h3 id="g09"> G-09 Consider activate `via-ir` for deploying </h3>

The IR-based code generator was introduced with an aim to not only allow code generation to be more transparent and auditable but also to enable more powerful optimization passes that span across functions.

You can enable it on the command line using --via-ir or with the option {"viaIR": true}.

This will take longer to compile, but you can just simple test it before deploying and if you got a better benchmark then you can add --via-ir to your deploy command

More on: https://docs.soliditylang.org/en/v0.8.17/ir-breaking-changes.html

test_mintIfSlippageTooHigh() (gas: -519 (-0.048%)) 
test_mintIfPreferClaimedIsFalse() (gas: -515 (-0.052%)) 
test_mintIfWeightGreatherThanPrice(uint256) (gas: -860 (-0.090%)) 
test_swapIfQuoteBetter(uint256) (gas: -1065 (-0.096%)) 
testDatasourceDelegateMintIfPreferenceIsNotToClaimTokens() (gas: -1545 (-0.814%)) 
testDatasourceDelegateSwapIfPreferenceIsToClaimTokens() (gas: -2231 (-0.840%)) 
testDatasourceDelegateWhenQuoteIsLowerThanTokenCount(uint256) (gas: -1994 (-1.046%)) 
test_swapMultiple() (gas: -20268 (-1.488%)) 
test_swapRandomAmountIn(uint256) (gas: -20337 (-1.876%)) 
testRevertIfSlippageIsTooMuchWhenSwapping() (gas: 474 (3.856%)) 
Overall gas change: -48860 (-0.673%)

Usage

# Take a snapshot
forge snapshot --snap=baseSnap
# Use diff a against 
forge snapshot --diff=baseSnap --via-ir