From 24cc6b8d483b54109958ee9efbbcbe3c05e80032 Mon Sep 17 00:00:00 2001 From: Charles Crain Date: Wed, 10 Jun 2026 16:18:30 -0400 Subject: [PATCH] fix(MarketUtilsV2): pay out full remaining amount on indivisible ETH splits payoutWithMarketplaceFee floored each split recipient's share but passed the un-floored remainingAmount as the total to performPayouts. When the floored amounts don't sum back to remainingAmount (the common case for non-round amounts, e.g. 1%-increment auction bids), Payments.payout's require(msg.value == sum(amounts)) reverts on the ETH path. For RareBatchAuctionHouse.settleAuction this is unrecoverable: the winning bid and NFT are escrowed and settlement is the only release path, so the funds and token are locked permanently. RareBatchListingMarketplace. buyWithMerkleProof hits the same revert (recoverable by re-registering). v1 MarketUtils.payout passed the summed amounts; v2 regressed this. Fix: assign the rounding remainder to the last recipient so the per- recipient amounts sum to remainingAmount exactly, distributing the full proceeds with no dust stranded in the contract. Also adds upgrade scripts to swap the implementation behind each UUPS proxy. Co-Authored-By: Claude Opus 4.8 --- .../RareBatchAuctionHouseUpgrade.s.sol | 33 +++++++ .../RareBatchListingMarketplaceUpgrade.s.sol | 33 +++++++ src/test/v2/utils/MarketUtilsV2.t.sol | 88 +++++++++++++++++++ src/v2/utils/MarketUtilsV2.sol | 11 +++ 4 files changed, 165 insertions(+) create mode 100644 script/auctionhouse/rare-batch-auctionhouse-upgrade/RareBatchAuctionHouseUpgrade.s.sol create mode 100644 script/marketplace/rare-batch-listing-marketplace-upgrade/RareBatchListingMarketplaceUpgrade.s.sol diff --git a/script/auctionhouse/rare-batch-auctionhouse-upgrade/RareBatchAuctionHouseUpgrade.s.sol b/script/auctionhouse/rare-batch-auctionhouse-upgrade/RareBatchAuctionHouseUpgrade.s.sol new file mode 100644 index 0000000..194c170 --- /dev/null +++ b/script/auctionhouse/rare-batch-auctionhouse-upgrade/RareBatchAuctionHouseUpgrade.s.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + +import "forge-std/Script.sol"; + +import "../../../src/v2/auctionhouse/RareBatchAuctionHouse.sol"; + +/// @title RareBatchAuctionHouseUpgrade +/// @notice Deploys a new RareBatchAuctionHouse implementation and upgrades the +/// existing UUPS proxy to point at it. Must be broadcast by the proxy +/// owner (see _authorizeUpgrade -> onlyOwner). +/// @dev Required env vars: +/// PRIVATE_KEY - deployer/owner key +/// RARE_BATCH_AUCTIONHOUSE - address of the deployed proxy to upgrade +contract RareBatchAuctionHouseUpgrade is Script { + function run() external { + uint256 privateKey = vm.envUint("PRIVATE_KEY"); + address proxy = vm.envAddress("RARE_BATCH_AUCTIONHOUSE"); + + vm.startBroadcast(privateKey); + + // Deploy the new implementation. + RareBatchAuctionHouse newImplementation = new RareBatchAuctionHouse(); + + // Point the proxy at the new implementation (no re-initialization needed). + RareBatchAuctionHouse(payable(proxy)).upgradeTo(address(newImplementation)); + + console.log("RareBatchAuctionHouse proxy:", proxy); + console.log("New RareBatchAuctionHouse implementation:", address(newImplementation)); + + vm.stopBroadcast(); + } +} diff --git a/script/marketplace/rare-batch-listing-marketplace-upgrade/RareBatchListingMarketplaceUpgrade.s.sol b/script/marketplace/rare-batch-listing-marketplace-upgrade/RareBatchListingMarketplaceUpgrade.s.sol new file mode 100644 index 0000000..f987bec --- /dev/null +++ b/script/marketplace/rare-batch-listing-marketplace-upgrade/RareBatchListingMarketplaceUpgrade.s.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + +import "forge-std/Script.sol"; + +import "../../../src/v2/marketplace/RareBatchListingMarketplace.sol"; + +/// @title RareBatchListingMarketplaceUpgrade +/// @notice Deploys a new RareBatchListingMarketplace implementation and upgrades +/// the existing UUPS proxy to point at it. Must be broadcast by the proxy +/// owner (see _authorizeUpgrade -> onlyOwner). +/// @dev Required env vars: +/// PRIVATE_KEY - deployer/owner key +/// RARE_BATCH_LISTING_MARKETPLACE - address of the deployed proxy to upgrade +contract RareBatchListingMarketplaceUpgrade is Script { + function run() external { + uint256 privateKey = vm.envUint("PRIVATE_KEY"); + address proxy = vm.envAddress("RARE_BATCH_LISTING_MARKETPLACE"); + + vm.startBroadcast(privateKey); + + // Deploy the new implementation. + RareBatchListingMarketplace newImplementation = new RareBatchListingMarketplace(); + + // Point the proxy at the new implementation (no re-initialization needed). + RareBatchListingMarketplace(address(proxy)).upgradeTo(address(newImplementation)); + + console.log("RareBatchListingMarketplace proxy:", proxy); + console.log("New RareBatchListingMarketplace implementation:", address(newImplementation)); + + vm.stopBroadcast(); + } +} diff --git a/src/test/v2/utils/MarketUtilsV2.t.sol b/src/test/v2/utils/MarketUtilsV2.t.sol index 5e8ac0a..3f37413 100644 --- a/src/test/v2/utils/MarketUtilsV2.t.sol +++ b/src/test/v2/utils/MarketUtilsV2.t.sol @@ -885,6 +885,94 @@ contract MarketUtilsV2Test is Test { assertEq(bobBalanceAfter, bobExpectedBalance, "incorrect second split receiver balance after payout"); } + function test_payout_multipleSplits_indivisibleAmount() public { + // Regression test for the split-payout rounding bug. + // + // payoutWithMarketplaceFee floors each recipient's cut + // (splitAmounts[i] = remainingAmount * ratio_i / 100) but passes the + // un-floored remainingAmount as the total to performPayouts. When the + // floors don't sum back to remainingAmount (the common case for non-round + // amounts), Payments.payout reverts with "payout::not enough sent", + // permanently bricking batch auction settlement / merkle-proof buys. + address originContract = address(0xaaaa); + uint256 tokenId = 1; + address currencyAddress = address(0); + + // Odd-wei amount: a 50/50 split floors to (amount-1)/2 each, leaving the + // sum 1 wei short of remainingAmount. + uint256 amount = 1 ether + 1; + + address payable[] memory splitAddrs = new address payable[](2); + uint8[] memory splitRatios = new uint8[](2); + splitAddrs[0] = payable(charlie); + splitAddrs[1] = payable(bob); + splitRatios[0] = 50; + splitRatios[1] = 50; + + // setup getRewardAccumulatorAddressForUser call + vm.mockCall( + stakingRegistry, + abi.encodeWithSelector(IRareStakingRegistry.getRewardAccumulatorAddressForUser.selector, charlie), + abi.encode(address(0)) + ); + + // setup calculateStakingFee call + vm.mockCall( + stakingSettings, + abi.encodeWithSelector(IStakingSettings.calculateStakingFee.selector, amount), + abi.encode(0) + ); + + // setup getMarketplaceFeePercentage call + vm.mockCall( + marketplaceSettings, + abi.encodeWithSelector(IMarketplaceSettings.getMarketplaceFeePercentage.selector), + abi.encode(3) + ); + + // secondary sale so remainingAmount stays == amount (no primary fee) + vm.mockCall( + marketplaceSettings, + abi.encodeWithSelector(IMarketplaceSettings.hasERC721TokenSold.selector, originContract, 1), + abi.encode(true) + ); + + // no royalties -> remainingAmount is the full amount + vm.mockCall( + royaltyEngine, + abi.encodeWithSelector(IRoyaltyEngineV1.getRoyalty.selector, originContract, tokenId, amount), + abi.encode(new address payable[](0), new uint256[](0)) + ); + + uint256 charlieBalanceBefore = charlie.balance; + uint256 bobBalanceBefore = bob.balance; + + uint256 marketplaceFee = (amount * 3) / 100; + + // Must not revert: the full remaining amount has to be distributable. + vm.prank(deployer); + tc.payout{value: amount + marketplaceFee}( + originContract, + tokenId, + currencyAddress, + amount, + charlie, + splitAddrs, + splitRatios + ); + + // First recipient gets its floored share; the last recipient absorbs the + // rounding remainder so the entire remaining amount is paid out. + uint256 charlieShare = (amount * 50) / 100; + uint256 bobShare = amount - charlieShare; + + assertEq(charlie.balance, charlieBalanceBefore + charlieShare, "first split incorrect"); + assertEq(bob.balance, bobBalanceBefore + bobShare, "last split should absorb remainder"); + assertEq(charlieShare + bobShare, amount, "splits must sum to the full remaining amount"); + // Nothing should be stranded in the market contract. + assertEq(address(tc).balance, 0, "no funds should remain in market contract"); + } + function test_payout_tooManyRoyaltyRecipients() public { address originContract = address(0xaaaa); uint256 tokenId = 1; diff --git a/src/v2/utils/MarketUtilsV2.sol b/src/v2/utils/MarketUtilsV2.sol index d2a245b..1c26660 100644 --- a/src/v2/utils/MarketUtilsV2.sol +++ b/src/v2/utils/MarketUtilsV2.sol @@ -296,8 +296,19 @@ library MarketUtilsV2 { // Calculate and pay out splits uint256[] memory splitAmounts = new uint256[](_splitRatios.length); + uint256 distributed = 0; for (uint256 i = 0; i < _splitRatios.length; i++) { splitAmounts[i] = (remainingAmount * _splitRatios[i]) / 100; + distributed += splitAmounts[i]; + } + + // Flooring each share can leave a rounding remainder so the per-recipient + // amounts sum to less than remainingAmount. performPayouts forwards + // remainingAmount as the total, and Payments.payout requires the total to + // equal the sum of the amounts -- so assign the remainder to the last + // recipient to keep them equal and distribute the full amount. + if (_splitRatios.length > 0) { + splitAmounts[_splitRatios.length - 1] += remainingAmount - distributed; } performPayouts(_config, _currencyAddress, remainingAmount, _splitAddrs, splitAmounts);