Skip to content

Fix lb rewarder #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0x0Louis wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix lb rewarder #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c03024f
Fix reentrancy issue
0x0Louis Jul 3, 2026
99e0ee9
run fmt
0x0Louis Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/LBHooksBaseRewarder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,8 @@ abstract contract LBHooksBaseRewarder is LBBaseHooks, Ownable2StepUpgradeable, C

if (accRewardsPerShareX64 > userAccRewardsPerShareX64) {
unchecked {
pendingRewards += (accRewardsPerShareX64 - userAccRewardsPerShareX64).mulShiftRoundDown(
balanceX64, OFFSET_PRECISION
);
pendingRewards += (accRewardsPerShareX64 - userAccRewardsPerShareX64)
.mulShiftRoundDown(balanceX64, OFFSET_PRECISION);
}
}
}
Expand Down Expand Up @@ -372,9 +371,8 @@ abstract contract LBHooksBaseRewarder is LBBaseHooks, Ownable2StepUpgradeable, C

if (accRewardsPerShareX64 > userAccRewardsPerShareX64) {
unchecked {
pendingRewards += (accRewardsPerShareX64 - userAccRewardsPerShareX64).mulShiftRoundDown(
balanceX64, OFFSET_PRECISION
);
pendingRewards += (accRewardsPerShareX64 - userAccRewardsPerShareX64)
.mulShiftRoundDown(balanceX64, OFFSET_PRECISION);
}

bin.userAccRewardsPerShareX64[to] = accRewardsPerShareX64;
Expand All @@ -391,18 +389,16 @@ abstract contract LBHooksBaseRewarder is LBBaseHooks, Ownable2StepUpgradeable, C
* @param rewards The rewards to claim
*/
function _claim(address user, uint256[] memory ids, uint256 rewards) internal virtual {
if (rewards == 0) {
_onClaim(user, ids);
} else {
if (rewards > 0) {
_totalUnclaimedRewards -= rewards;
_unclaimedRewards[user] -= rewards;

emit Claim(user, rewards);

_onClaim(user, ids);

TokenHelper.safeTransfer(_getRewardToken(), user, rewards);
}

_onClaim(user, ids);
}

/**
Expand Down
18 changes: 6 additions & 12 deletions test/LBHooksBaseRewarder.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,12 @@ contract LBHooksBaseRewarderTest is TestHelper {
hooksParameters = Hooks.setHooks(hooksParameters, address(new MockLBHooksRewarder()));

hooks = MockLBHooksRewarder(
payable(
_createAndSetLBHooks(
payable(_createAndSetLBHooks(
pair01,
hooksParameters,
abi.encodePacked(address(pair01), address(rewardToken01)),
abi.encode(address(this))
)
)
))
);

vm.label(address(hooks), "hooks");
Expand Down Expand Up @@ -191,11 +189,9 @@ contract LBHooksBaseRewarderTest is TestHelper {
);

hooks = MockLBHooksRewarder(
payable(
_createAndSetLBHooks(
payable(_createAndSetLBHooks(
pair01, hooksParameters, abi.encodePacked(pair01, address(0)), abi.encode(address(this))
)
)
))
);

(s, d) = address(hooks).call{value: 1e18}(data);
Expand All @@ -213,11 +209,9 @@ contract LBHooksBaseRewarderTest is TestHelper {

function test_NativeReward() public {
hooks = MockLBHooksRewarder(
payable(
_createAndSetLBHooks(
payable(_createAndSetLBHooks(
pair01, hooksParameters, abi.encodePacked(pair01, address(0)), abi.encode(address(this))
)
)
))
);

deal(address(hooks), 1e18);
Expand Down
12 changes: 4 additions & 8 deletions test/LBHooksExtraRewarder.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,23 @@ contract LBHooksExtraRewarderTest is TestHelper {
);

lbHooks = LBHooksMCRewarder(
payable(
address(
payable(address(
lbHooksManager.createLBHooksMCRewarder(
IERC20(address(token0)), IERC20(address(token1)), DEFAULT_BIN_STEP, address(this)
)
)
)
))
);

lbHooksExtra = LBHooksExtraRewarder(
payable(
address(
payable(address(
lbHooksManager.createLBHooksExtraRewarder(
IERC20(address(token0)),
IERC20(address(token1)),
DEFAULT_BIN_STEP,
IERC20(address(rewardToken01)),
address(this)
)
)
)
))
);

vm.label(address(lbHooks), "lbHooksRewarder");
Expand Down
12 changes: 4 additions & 8 deletions test/LBHooksLens.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,27 +27,23 @@ contract LBHooksLensTest is TestHelper {
);

lbHooks = LBHooksMCRewarder(
payable(
address(
payable(address(
lbHooksManager.createLBHooksMCRewarder(
IERC20(address(token0)), IERC20(address(token1)), DEFAULT_BIN_STEP, address(this)
)
)
)
))
);

lbHooksExtra = LBHooksExtraRewarder(
payable(
address(
payable(address(
lbHooksManager.createLBHooksExtraRewarder(
IERC20(address(token0)),
IERC20(address(token1)),
DEFAULT_BIN_STEP,
IERC20(address(rewardToken01)),
address(this)
)
)
)
))
);

lbHooksLens = new LBHooksLens(address(lbHooksManager), address(masterchef));
Expand Down
224 changes: 224 additions & 0 deletions test/LBHooksMCRewarderReentrancy.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import "./TestHelper.sol";

import {ILBPair} from "@lb-protocol/src/interfaces/ILBPair.sol";

import "../src/LBHooksMCRewarder.sol";
import "../src/LBHooksExtraRewarder.sol";
import "../src/interfaces/ILBHooksBaseRewarder.sol";

/**
* @dev Regression tests for the reentrancy issue where a native-paying extra rewarder's
* payout callback reenters the parent MC rewarder while its MOE is still in flight.
*
* The parent settles its MOE (accounting + transfer) before calling the extra rewarder, so
* `balance - _totalUnclaimedRewards` is consistent during any callback. These tests exercise
* both the direct `claim` reentry and the reentry through the unguarded LB hooks.
*/
contract LBHooksMCRewarderReentrancyTest is TestHelper {
LBHooksMCRewarder lbHooks;
LBHooksExtraRewarder lbHooksExtra;
ReentrantClaimer attacker;

function setUp() public override {
super.setUp();

lbHooksManager.setLBHooksParameters(
ILBHooksManager.LBHooksType.MCRewarder,
Hooks.setHooks(hooksParameters, address(new LBHooksMCRewarder(address(lbHooksManager), masterchef, moe)))
);
lbHooksManager.setLBHooksParameters(
ILBHooksManager.LBHooksType.ExtraRewarder,
Hooks.setHooks(hooksParameters, address(new LBHooksExtraRewarder(address(lbHooksManager))))
);

lbHooks = LBHooksMCRewarder(
payable(address(
lbHooksManager.createLBHooksMCRewarder(
IERC20(address(token0)), IERC20(address(token1)), DEFAULT_BIN_STEP, address(this)
)
))
);

lbHooksExtra = LBHooksExtraRewarder(
payable(address(
lbHooksManager.createLBHooksExtraRewarder(
IERC20(address(token0)),
IERC20(address(token1)),
DEFAULT_BIN_STEP,
IERC20(address(0)),
address(this)
)
))
);

vm.label(address(lbHooks), "lbHooksRewarder");
vm.label(address(lbHooksExtra), "lbHooksExtraRewarder");

attacker = new ReentrantClaimer(lbHooks, pair01, ids);
vm.label(address(attacker), "attacker");
}

/// @dev Funds the native extra rewarder and seeds the attacker with a 10% liquidity share.
function _setup() internal {
lbHooks.setDeltaBins(0, 1);
lbHooksExtra.setDeltaBins(0, 1);

vm.deal(address(lbHooksExtra), 100e18);
lbHooksExtra.setRewardPerSecond(1e18, 100);

_addLiquidity(pair01, address(attacker), DEFAULT_ID, 0, 10e18, 10e18);
_addLiquidity(pair01, bob, DEFAULT_ID, 0, 90e18, 90e18);

vm.warp(block.timestamp + 10);
}

function test_HonestClaimWithNativeExtraRewarder() public {
_setup();

uint256 pendingMoe = lbHooks.getPendingRewards(address(attacker), ids);
uint256 pendingNative = lbHooksExtra.getPendingRewards(address(attacker), ids);

assertGt(pendingMoe, 0, "test_HonestClaimWithNativeExtraRewarder::1");
assertGt(pendingNative, 0, "test_HonestClaimWithNativeExtraRewarder::2");

attacker.setMode(ReentrantClaimer.Mode.None);
attacker.claim();

assertApproxEqAbs(moe.balanceOf(address(attacker)), pendingMoe, 1, "test_HonestClaimWithNativeExtraRewarder::3");
assertApproxEqAbs(address(attacker).balance, pendingNative, 1, "test_HonestClaimWithNativeExtraRewarder::4");

_assertBobUnharmed();
}

function test_ReentrantClaimDoesNotInflateMoe() public {
_setup();

uint256 pendingMoe = lbHooks.getPendingRewards(address(attacker), ids);
uint256 pendingNative = lbHooksExtra.getPendingRewards(address(attacker), ids);

attacker.setMode(ReentrantClaimer.Mode.ReenterClaim);
attacker.claim();

// The reentrant claim must not pay the attacker more than its honest share of either token.
assertApproxEqAbs(moe.balanceOf(address(attacker)), pendingMoe, 1, "test_ReentrantClaimDoesNotInflateMoe::1");
assertApproxEqAbs(address(attacker).balance, pendingNative, 1, "test_ReentrantClaimDoesNotInflateMoe::2");

// No phantom pending is observable mid-claim: the MOE is settled before the callback fires.
assertApproxEqAbs(attacker.pendingDuringCallback(), 0, 1, "test_ReentrantClaimDoesNotInflateMoe::3");

_assertBobUnharmed();
}

function test_ReentrantClaimViaHookDoesNotCorruptAccounting() public {
_setup();

uint256 pendingMoe = lbHooks.getPendingRewards(address(attacker), ids);
uint256 pendingNative = lbHooksExtra.getPendingRewards(address(attacker), ids);

// Reenter through an LB hook (self batchTransferFrom) rather than `claim` directly.
// This path is not covered by a `claim` reentrancy guard, so only the settle-before-call
// ordering protects it. swap/mint/burn reach the same `_updateAccruedRewardsPerShare`.
attacker.setMode(ReentrantClaimer.Mode.ReenterHook);
attacker.claim();

assertApproxEqAbs(
moe.balanceOf(address(attacker)), pendingMoe, 1, "test_ReentrantClaimViaHookDoesNotCorruptAccounting::1"
);
assertApproxEqAbs(
address(attacker).balance, pendingNative, 1, "test_ReentrantClaimViaHookDoesNotCorruptAccounting::2"
);
assertApproxEqAbs(
attacker.pendingDuringCallback(), 0, 1, "test_ReentrantClaimViaHookDoesNotCorruptAccounting::3"
);

// On the vulnerable ordering this desyncs `_totalUnclaimedRewards` above the balance and
// the next pending computation underflows and reverts.
_assertBobUnharmed();
}

function test_ReentrantClaimOnlyExtraRewarderEmptyMainPool() public {
// Leave the main pool range empty (default deltaBins 0,0) so the attacker has no MOE to claim,
// exercising the `rewards == 0` branch while the native extra rewarder still fires the callback.
lbHooksExtra.setDeltaBins(0, 1);

vm.deal(address(lbHooksExtra), 100e18);
lbHooksExtra.setRewardPerSecond(1e18, 100);

_addLiquidity(pair01, address(attacker), DEFAULT_ID, 0, 10e18, 10e18);
_addLiquidity(pair01, bob, DEFAULT_ID, 0, 90e18, 90e18);

vm.warp(block.timestamp + 10);

uint256 pendingNative = lbHooksExtra.getPendingRewards(address(attacker), ids);
assertGt(pendingNative, 0, "test_ReentrantClaimOnlyExtraRewarderEmptyMainPool::1");

attacker.setMode(ReentrantClaimer.Mode.ReenterClaim);
attacker.claim();

assertEq(moe.balanceOf(address(attacker)), 0, "test_ReentrantClaimOnlyExtraRewarderEmptyMainPool::2");
assertApproxEqAbs(
address(attacker).balance, pendingNative, 1, "test_ReentrantClaimOnlyExtraRewarderEmptyMainPool::3"
);
}

/// @dev Bob keeps his fair share and the accounting stays readable/claimable after the attack.
function _assertBobUnharmed() internal {
uint256 bobPending = lbHooks.getPendingRewards(bob, ids);
assertApproxEqRel(bobPending, 9e18, 1e14, "_assertBobUnharmed::1");

vm.prank(bob);
lbHooks.claim(bob, ids);

assertApproxEqAbs(moe.balanceOf(bob), bobPending, 1, "_assertBobUnharmed::2");
}
}

contract ReentrantClaimer {
enum Mode {
None,
ReenterClaim,
ReenterHook
}

ILBHooksBaseRewarder public immutable lbHooks;
ILBPair public immutable pair;

uint256[] internal _ids;
Mode internal _mode;
bool internal _entered;

/// @dev `getPendingRewards` for this contract, read from inside the payout callback.
uint256 public pendingDuringCallback;

constructor(ILBHooksBaseRewarder lbHooks_, ILBPair pair_, uint256[] memory ids_) {
lbHooks = lbHooks_;
pair = pair_;
_ids = ids_;
}

function setMode(Mode mode) external {
_mode = mode;
}

function claim() external {
_entered = false;
lbHooks.claim(address(this), _ids);
}

receive() external payable {
if (_entered) return;
_entered = true;

pendingDuringCallback = lbHooks.getPendingRewards(address(this), _ids);

if (_mode == Mode.ReenterClaim) {
lbHooks.claim(address(this), _ids);
} else if (_mode == Mode.ReenterHook) {
pair.batchTransferFrom(address(this), address(this), _ids, new uint256[](_ids.length));
}
}
}
6 changes: 2 additions & 4 deletions test/LBHooksManager.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,11 @@ contract LBHooksManagerTest is TestHelper {
);

LBHooksMCRewarder lbHooks = LBHooksMCRewarder(
payable(
address(
payable(address(
lbHooksManager.createLBHooksMCRewarder(
IERC20(address(token0)), IERC20(address(token1)), DEFAULT_BIN_STEP, address(this)
)
)
)
))
);

assertEq(
Expand Down
Loading