Donation Contracts

MarketPoolDonationHandler.donate() increases the accounted pool amount without first checkpointing cumulative borrowing factors. MarketUtils.getNextCumulativeBorrowingFactor() calculates the entire interval since the previous update using a rate derived from the current pool liquidity. Increasing liquidity can therefore apply a lower rate to time preceding the donation.

Note that the same class of issue exists in the SwapUtils.swap() path, but it was previously reported as IOU-2 in the 2023 Guardian Report.

Consider checkpointing the cumulative borrowing factor before changing the poolAmount.

The best way would be to invoke updateCumulativeBorrowingFactor() for both the long and short directions from donation(). However, this would require making MarketPoolDonationHandler inherit from OracleModule and it will also require providing prices on each donation.

GlvDonationHandler synchronizes additional GM holdings without minting GLV shares. This intentionally increases assets per share, but it neither invalidates signed GLV valuation snapshots that predate the increase nor establishes a freshness condition for subsequent GLV operations.

When a signed GLV price is supplied, GlvUtils returns that price multiplied by current supply without valuing the current recorded holdings. That result is used in actual deposit minting and withdrawal conversion. A report that omits an authorized donation can consequently overstate the shares to mint for a deposit or understate a withdrawing holder's share value, subject to the request's minimum-output checks.

Be aware and document this behavior.

Both configuration setters replace donationPeriodDuration while leaving donation state unchanged. State records only a numeric periodId and donatedAmount. Execution and the getters subsequently compare the stored identifier with a quotient calculated using the new duration.

This makes the behavior inconsistent, depending on the periodID. If the new one matches the current periodID, the donated amount will be preserved after the update.

On the other hand, if the configuration is modified with the intention of keeping the same limits with different duration, the donated amount will be reset to 0.

There is also not guarantee that the periodID will be ever-increasing. This makes it possible to transition to a lower period from a higher one.

Decide what the intended behavior of this flow should be and either acknowledge the finding or apply the necessary changes.

ExecuteGlvDepositUtils.executeGlvDeposit() processes a depositor’s GM before calling GlvUtils.getGlvValue(). A GM reward transferred directly to a GLV is absent from GlvToken.tokenBalances() until GlvToken.syncTokenBalance() runs. The stale cache.glvValue then reaches GlvDepositCalc.getMintAmount(), overminting GLV tokens. A JIT depositor can redeem part of that pre-existing reward, diluting incumbent holders.

Fixing this requires changes in the core GMX logic, so be aware of it and either document it or consider applying a fix separately.

ConfigSyncer permits updates to borrowing-curve parameters and forwards them to Config.setUint without first checkpointing cumulative borrowing under the prior curve. The next market action computes a rate from the new parameters and multiplies it by the entire interval since the last checkpoint. A legitimate rate reduction therefore erases fees accrued under the old curve, while an increase retroactively charges a rate that was not in force. RiskOracleConfig's separate update path explicitly settles borrowing before writing these keys, corroborating the missing checkpoint.

Impact: The passing production-fork PoC opens and closes a real $5 million position through the protocol handlers. Reducing the active kink factors after 30 uncheckpointed days, immediately before the identical close, increases the trader's actual payout by about 90.44 WETH ($224,556), directly reducing LP/protocol fee revenue. The error affects all positions on the updated side and can also overcharge or unexpectedly liquidate positions when the curve is increased.

Before ConfigSyncer writes any borrowing-rate key, atomically checkpoint the affected market and side using validated oracle prices under the old configuration, matching RiskOracleConfig._settleBorrowingIfRequired. Otherwise remove borrowing keys from the generic Config.setUint synchronization route.

MarketSwap orders store address(0) as order.market and place their actual markets only in swapPath. BaseOrderHandler resolves swapPathMarkets, but ExecuteOrderUtils checkpoints funding and borrowing only for params.market, which is empty for a pure swap. SwapUtils then changes the real path market's pool amounts. The next position action applies the post-swap utilization rate to the entire interval since the last checkpoint, allowing a user to add the charged-side token, crystallize a retroactively reduced cumulative borrowing factor, and reverse the swap.

The fork PoC uses public request/execution paths and authenticated reports, passes as submitted, and demonstrates a 499.549 WETH net benefit after the complete round trip.

A fix would checkpoint funding and cumulative borrowing for each distinct swapPath market using its pre-swap balances and validated prices, instead of relying on params.market, which is intentionally empty for MarketSwap orders.

Market pool valuation caps each side's positive pending PnL separately before netting it against the opposite side's uncapped loss. An equal-sized long and short can therefore produce positive aggregate market value once the winning side exceeds its cap, although their true aggregate PnL is approximately zero.

The intentional per-side cap does not make this composed NAV result realizable: settling the solvent losing leg first credits its loss to the winning leg's payout-token pool, increases that leg's dynamic cap, and permits the winner to recover the formerly capped PnL. The pool therefore never realizes the marked surplus.

Aggregate-priced GLVs nevertheless include it, and a holder can redeem the inflated GLV value through another healthy constituent because execution validates only the selected withdrawal market's PnL.

The main impact of the finding is about the core GMX system, but GlvDonationHandler also uses the inflated aggregate value for its factor allowance.

Consider whether the impact shown in the POC - withdrawing value from constituent market due to another inflated market's valuation - needs to be mitigated.

If not, just document the issue.

Both donate functions reject disabled configurations before checking fundingAccount against zero. Their private configuration mappings can only be written through setters, and an enabled configuration cannot be stored with a zero funding account. The repeated zero-address branches are therefore unreachable under the current implementation.

Be aware and consider to remove superfluous code.

Both handlers define the same DONATION_KEEPER hash and onlyDonationKeeper modifier locally instead of using the shared Role/Role2 libraries and RoleModule.

Be aware and consider to define DONATION_KEEPER once in the shared role library and onlyDonationKeeper once in RoleModule.

The donate comment says the function pulls the configured market token. However the market's pool token is pulled in reality.

Replace 'configured market token' with 'configured market's underlying pool token' in the donate documentation.

setMarketPoolDonationConfig verifies market existence and, when enabling donations, single-token eligibility and absence of virtual inventory. It does not reject a globally disabled market. In contrast, setGlvDonationConfig calls MarketUtils.validateEnabledMarket inside its isEnabled branch.

The pool setter can therefore store and emit an enabled donation configuration that cannot currently execute.

Be aware and for consistency with the GLV setter, consider to call MarketUtils.validateEnabledMarket(dataStore, market) inside the isEnabled branch of setMarketPoolDonationConfig.

MarketPoolDonationHandler.donate increases poolAmount and calls validatePoolAmount, but omits the validatePoolUsdForDeposit check used for normal GM deposits.

Be aware and document this behavior or consider to enforce MarketUtils.validatePoolUsdForDeposit after updating poolAmount.

MarketPoolDonationHandler.donate() applies maxDonationFactor to the raw poolAmount, without respecting the traders max pnl cap. Part of the donated value may therefore be offset by newly recognized trader PnL rather than increasing GM value. Similarly, the newly donated amount may also increase the capped value of the positive price-impact received.

On the other hand, withdrawals are valued using the net pool value calculated by MarketUtils.getPoolValueInfo(), which accounts for trader PnL and the position impact pool. When net GM equity is below the gross pool value, a maximum-sized donation can increase the GM price by more than maxDonationFactor.

Consequently, setting withdrawalFeeFactor above maxDonationFactor does not by itself prove that an immediate deposit-donation-withdrawal strategy is unprofitable. With a 0.5% donation factor and 4% withdrawal fee, the strategy remains unprofitable only while net GM equity is at least approximately 12% of gross pool value. This is likely true during ordinary operation but is not enforced.

Apply maxDonationFactor to the pre-donation net GM equity using the same valuation applied to withdrawals. Alternatively, document and enforce a minimum net-to-gross pool-value ratio with an appropriate safety margin.

Also document that every accepted pool donation immediately increases the pool value used by capPositiveImpactUsdByPositionImpactPool(), potentially increasing payouts for pre-donation positive-impact claims and recording the additional payout in lentPositionImpactPoolAmount.

GlvDonationHandler.donate() maximizes both execution.glvValue and execution.marketPoolValue. Maximizing execution.glvValue increases maxDonationUsd, making the proportional donation limit less conservative. Additionally, _getMarketPoolValue() checks only the market’s maximized value, allowing a market with a negative minimized value to pass.

Calculate execution.glvValue with maximize = false. In _getMarketPoolValue(), call getPoolValueInfo() with maximize = false for the negative-value check, then return a second calculation using maximize = true for donationUsd and balance-limit validation.

GlvDonationHandler obtains the factor-cap denominator through the reverting GlvUtils.getGlvValue. Aggregate valuation aborts when any supported market with a nonzero GLV balance has negative pool value, so that one market prevents donations of unrelated healthy GM to the same GLV. The restriction persists until the market recovers or governance disables it, configures the residual-balance dust threshold, and removes it. The PoC demonstrates both the revert and this governance recovery path.

Be aware and document this behavior.

MarketUtils.validateMarketTokenBalance() reverts when the market has balance < aggregate claimableFundingAmount (documented late-liquidation edge in MarketUtils L3652-L3655). Because of this normal funding claims revert.

MarketPoolDonationHandler.donate() performs no pre-transfer solvency check. This makes it possible to donate to a market in the described state and make the balance > claimableFundingAmount, which enables claims again.

Be aware and document this behavior.

donate() treats any nonzero MarketToken.totalSupply as proof that a redeemable beneficiary exists. GMX's configured first-deposit path, however, requires bootstrap GM to be minted to inaccessible address(1). After all ordinary LPs withdraw, totalSupply can consist solely of dead bootstrap shares. Donations still transfer approved underlying, increase accounted pool liquidity, and consume quota without minting any controllable GM. Later deposits are priced against the elevated pool and dead supply, so they acquire only a claim equal to their new contribution; they do not recover the prior donation.

Impact: A permissionless transition from a normally configured live market to address(1)-only supply can cause an honest scheduled keeper execution to strand funding assets through every normal user flow. The fork PoC creates a real market, performs the protocol-mandated bootstrap deposit, proves 100% of GM is at address(1), and executes two cap-valid donations that consume 3 WETH and grow dead backing from 1 to 4 WETH. It then deposits 1 WETH for a fresh live LP and withdraws all newly minted GM through the real withdrawal handler: exactly that 1 WETH returns while the entire original 4 WETH remains dead-backed. Repeated donation periods can therefore expand the irrecoverable minimum far beyond the bootstrap floor until privileged governance/controller recovery or migration occurs.

Be aware and document this behavior.

In same-token markets, the long and short sides share one stored pool amount. The amount is divided by two for each side, so when it is odd, integer rounding causes the two halves to add up to one atomic token unit less than the stored amount.

This can make a minimum-size donation temporarily absent from valuation, leave one unit behind after a full GM withdrawal, and understate the GLV per-market balance cap by the value of one unit. The discrepancy is limited to the token's smallest unit (for example, 1 wei of WETH) and is economically negligible.

Be aware and consider to handle same-token markets as one amount, or assign the division remainder to one side so the two sides always sum to the stored pool amount.

When setMarketPoolDonationConfig() disables a configuration, the existing marketPoolDonationStates entry is deleted only if donationPeriodDuration changes. Consequently, disabling with the existing duration preserves the consumed quota, whereas disabling with a different duration resets it. Because disabled configurations do not validate these parameters, the resulting state-reset behavior depends on otherwise inactive configuration values.

This behavior may be desired if the team needs a way to disable a configuration without resetting its state. If that's the case, consider documenting it.

MarketPoolDonationHandler._donate() and GlvDonationHandler.donate() prevent donations when the entire GM or GLV supply is held by the inaccessible bootstrap receiver, address(1). However, both checks only subtract the token balance held directly by address(1).

For a multichain deposit, ExecuteDepositUtils._executeDeposit() and ExecuteGlvDepositUtils.executeGlvDeposit() mint the resulting GM or GLV tokens to the MultichainVault and record ownership through Keys.multichainBalanceKey(receiver, token). Consequently, a first deposit whose required receiver is address(1) can produce a nonzero token supply while balanceOf(address(1)) remains zero. The checked live Arbitrum and Avalanche deployments have no enabled source-chain IDs, and all multichain balances for address(1) across registered GM and GLV tokens were zero at review time.

There is also one more condition where all the shares of a GM vault are dead.

  gm.totalSupply() == gm.balanceOf(address(1)) + gm.balanceOf(glv) && glv.totalSupply() == glv.balanceOf(address(1))

In that state, all GMs are held by one or more GLV vaults and all the glv shares are locked at address(1).

Before enabling multichain deposits, either document this interaction or include dataStore.getUint(Keys.multichainBalanceKey(address(1), token)) in the bootstrap dead-supply calculation for both donation handlers.

Also consider whether a change is needed in order to account for GM held in dead GLV vaults.

setMarketPoolDonationConfig() configures maxDonationFactor, donationPeriodDuration, and maxDonationAmountPerPeriod independently of the market’s withdrawal fee. During _donate(), maxDonationFactor limits each donation relative to the current pre-donation poolValue, while maxDonationAmountPerPeriod limits the cumulative token amount within a fixed Unix-time period.

The withdrawal fee is intended to prevent a just-in-time depositor from entering before a donation, receiving part of the resulting GM price increase, and withdrawing immediately afterward. For one maximum donation with factor f and withdrawal fee w, the position is unprofitable when:

(1 + f) × (1 - w) ≤ 1

However, a depositor can hold the same GM position across multiple donations while paying the withdrawal fee only once. After n maximum donations, the corresponding condition becomes:

(1 + f)ⁿ × (1 - w) ≤ 1

Equivalently, if D is the cumulative donated USD value and V₀ is the pool value when the depositor enters, the cumulative limit must satisfy:

D ≤ V₀ × w / (1 - w)

maxDonationAmountPerPeriod is an absolute token-denominated limit and therefore does not automatically adjust when poolValue, the donation token price, or the withdrawal fee changes. Additionally, because donation periods reset at fixed boundaries, a position may capture donations immediately before and after a boundary while paying one withdrawal fee.

Permissioned donation execution and appropriately selected configuration values can prevent profitable JIT behavior. Nevertheless, the required relationship is an operational assumption that is not documented or enforced by the contracts.

The same invariant applies to GLV donations. GlvDonationHandler.donate() grants a new factor allowance for every donation and tracks period

usage separately for each (glv, market) pair.

Document the required relationship between maxDonationFactor, maxDonationAmountPerPeriod, donationPeriodDuration.

When configuring donations, derive maxDonationAmountPerPeriod from a conservative minimum expected poolValue and maximum donation-token price such that cumulative donations during any expected deposit-to-withdrawal exposure window satisfy:

D ≤ V₀ × w / (1 - w)

Account for donations across adjacent fixed-period boundaries when determining this limit.

In addition, note that data stream providers may have more than one valid price at a time. This makes it technically possible for a withdrawal to use older price than the donation logic.

MarketPoolDonationHandler._donate() and GlvDonationHandler.donate() reject a donation only when totalSupply() is no greater than the shares held directly by the inaccessible bootstrap receiver, address(1). Once any positive circulating balance exists, however small, each handler permits a donation sized from the value backing the entire token supply, including the bootstrap shares.

If enforcing this onchain is desired, add a narrowly scoped minimum-circulation or maximum-dead-share-ratio configuration and scale the factor-based donation limit by circulatingSupply / totalSupply in both handlers after computing the effective dead supply.

MarketPoolDonationHandler._donate() previously reverted with Errors.EmptyMarketTokenSupply() when marketTokenSupply was zero. The remediation replaces that check with marketTokenSupply <= deadSupply, which reverts with EmptyCirculatingMarketTokenSupply().

Because balanceOf(address(1)) must also be zero when totalSupply() is zero, the replacement still prevents the donation. It no longer distinguishes a market with no issued GM from a market whose entire nonzero supply consists of inaccessible bootstrap shares.

Conversely, GlvDonationHandler.donate() retains the explicit Errors.EmptyGlvTokenSupply() check before applying its analogous glvSupply <= deadSupply check. The two parallel handlers therefore expose different error semantics for the same zero-supply state.

Either restore an explicit marketTokenSupply == 0 check in MarketPoolDonationHandler._donate()or remove the one in GlvDonationHandler.

The code acknowledges that multiple donations to the same GLV within DonationBatcher.multicall() may reuse a signed GLV price that no longer reflects the GLV’s state after the first donation.

The same behavior occurs when MarketPoolDonationHandler.donate() first donates to a GM market held by the GLV. The GM donation increases the value of the GLV’s existing GM balance, but a subsequent GlvDonationHandler.donate() may still use the pre-batch signed GLV price through GlvUtils.getGlvValue().

Consequently, maxDonationUsd is calculated conservatively and may reject an otherwise-valid donation, while the emitted GlvDonation and GlvValueUpdated values may understate the current GLV value.

Document both sources of intra-batch staleness and clarify that the emitted GLV value may not represent the final batch state.

GlvDonationHandler inherits BaseHandler but does not use its request-cancellation, error-handling, data-length, or native-token functionality. In particular, the donation flow never unwraps WNT, making the inherited payable receive() entry point unnecessary.

Either acknowledge or follow the narrower inheritance pattern used by MarketPoolDonationHandler: inherit RoleModule, GlobalReentrancyGuard, and OracleModule directly, and declare and initialize eventEmitter locally.