Skip to main content

2023-07-24 – Hold Fees Calculation Error

Author: filipv
Date: 2023-07-24
Severity: Medium
Status: Resolved

On 24th July 2023, a discrepancy in "hold fees" calculations resulted in a minor redemption discrepancy for Legends project supporters. In total, 0.006157049375371805 ETH of held fees were not refunded due to a calculation error in JBPayoutRedemptionPaymentTerminal3_1. This summary details the sequence of events, explains the technical error, and notes measures taken for mitigation and remediation.

Sequence of Events

  1. To raise funds for an auction, 🧠🧠🧠.eth deployed the Legends project with an unlimited payout to the project owner and "hold fees" enabled.
  2. The project received 12.35 ETH.
  3. 10.097560975609756097 ETH was paid out to 🧠🧠🧠.eth.
  4. The project received an additional 1 ETH.
  5. 10.097560975609756 ETH was returned to the project (only 97 wei less than what had been paid out).
  6. The project only had an overflow of 11.343842950624628098 ETH when it should have had 11.35 ETH (minus 97 wei) of overflow.

Because of this, the project's supporters did not receive a full refund when redeeming their tokens. As demonstrated by this simulation, the remaining 0.006157049375371805 ETH are still being held as fees.


The terminal represents held fees as a mapping:

/// @notice Fees that are being held to be processed later.
/// @custom:param _projectId The ID of the project for which fees are being held.
mapping(uint256 => JBFee[]) internal _heldFeesOf;

If a project has "hold fees" enabled, _takeFeeFrom adds a JBFee struct to its _heldFeesOf mapping:

_heldFeesOf[_projectId].push(JBFee(_amount, uint32(fee), uint32(_feeDiscount), _beneficiary));

Note that the _amount here is based on the full amount being paid out from the project, including the fee. These held fees can be restored later with _refundHeldFees:

// Process each fee.
for (uint256 _i; _i < _heldFeesLength; ) {
if (leftoverAmount == 0) _heldFeesOf[_projectId].push(_heldFees[_i]);
else if (leftoverAmount >= _heldFees[_i].amount) {
unchecked {
leftoverAmount = leftoverAmount - _heldFees[_i].amount;

The _refundHeldFees function is invoked when _addToBalanceOf is called with _shouldRefundHeldFees set to true:

uint256 _refundedFees = _shouldRefundHeldFees ? _refundHeldFees(_projectId, _amount) : 0;

But the _amount used when calling _refundHeldFees is based on the amount being returned to the project – not the amount which was taken out. Even if a project creator returns all received funds to a project, it will only refund 97.5% (100% minus the protocol fee) of the amount paid out, as they never received the "fee amount".


Jango has created a pull request to address these issues:

@filipviz and I discovered a bug July 24, 2023 while helping project #548 raise funds for an auction. The project had heldFees on. After failing to win the auction and upon returned the funds, the project was not made whole. The bug is that the protocol expected a deposit equivalent to the amount paid out. In other words, if 10 ETH was paid out (before the fee), the protocol was expected a deposit of 10 ETH to return the full fee amount. The problem is the recipient doesn't have the full 10 ETH, they only have the amount after the fee. The protocol should only expect the a deposit of the amount paid out after fees... the amount the recipient actually has.

This PR introduces JBPayoutRedemptionTerminal3_1_2 that fixes this issue.

It also

  • moved fee calculations into a JBFees library
  • removes the isTerminalOf check from pay and addToBalance to reduce contract size to be deployable. In pay these checks are already made when minting tokens. Clients are now responsible for making sure this check is correct, otherwise the project can access the funds from the terminal by setting distribution limits in a subsequent cycle.


I (filipv) refunded Legends supporters according to their contributions in transaction 0x54cff0ab259f8f10fa562bf6fff0999ed668a5d6b96a23994610a0a33333406e:

BeneficiaryContribution (ETH)Refund (ETH)