Skip to content
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.

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

[REG-1600] - Update SeaportProxyBuyer to prevent Seaport order execution outside of SeaportProxyBuyer call #378

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## v0.9.41
- Add revert if Seaport order is executed outside `SeaportProxyBuyer` contract
- Added a script to batch mint TLDs proposal

## v0.9.40
Expand Down
9 changes: 7 additions & 2 deletions artifacts/SeaportProxyBuyer.json

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion artifacts/abi/SeaportProxyBuyer.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions contracts/marketplace/ISeaportProxyBuyer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {AdvancedOrder, CriteriaResolver, ZoneParameters} from 'seaport-types/src
error OrderIsNotFulfiled();
error RecipientIsZeroAddress();
error InvalidZone();
error InvalidFulfiller();

interface ISeaportProxyBuyer {
/**
Expand Down
8 changes: 4 additions & 4 deletions contracts/marketplace/SeaportProxyBuyer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';
import {ERC2771RegistryContext} from '../metatx/ERC2771RegistryContext.sol';
import {Forwarder} from '../metatx/Forwarder.sol';
import {MinterRole} from '../roles/MinterRole.sol';
import {ISeaportProxyBuyer, OrderIsNotFulfiled, RecipientIsZeroAddress, InvalidZone} from './ISeaportProxyBuyer.sol';
import {ISeaportProxyBuyer, OrderIsNotFulfiled, RecipientIsZeroAddress, InvalidZone, InvalidFulfiller} from './ISeaportProxyBuyer.sol';
import {ConsiderationInterface} from 'seaport-types/src/interfaces/ConsiderationInterface.sol';
import {AdvancedOrder, CriteriaResolver, OrderComponents, OrderParameters, ZoneParameters} from 'seaport-types/src/lib/ConsiderationStructs.sol';

Expand All @@ -26,7 +26,7 @@ contract SeaportProxyBuyer is
ISeaportProxyBuyer
{
string public constant NAME = 'Seaport Proxy Buyer';
string public constant VERSION = '0.1.1';
string public constant VERSION = '0.1.2';

ConsiderationInterface private _seaport;

Expand Down Expand Up @@ -87,11 +87,11 @@ contract SeaportProxyBuyer is
}

function authorizeOrder(ZoneParameters calldata) external view override whenNotPaused returns (bytes4 authorizedOrderMagicValue) {
authorizedOrderMagicValue = ISeaportProxyBuyer.authorizeOrder.selector;
revert InvalidFulfiller();
}

function validateOrder(ZoneParameters calldata) external view override whenNotPaused returns (bytes4 validOrderMagicValue) {
validOrderMagicValue = ISeaportProxyBuyer.validateOrder.selector;
revert InvalidFulfiller();
}

function approve(address token) external onlyOwner nonReentrant {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "uns",
"version": "0.9.40",
"version": "0.9.41",
"description": "UNS contracts and tools",
"repository": "https://github.com/unstoppabledomains/uns.git",
"main": "./dist/index.js",
Expand Down
2 changes: 1 addition & 1 deletion sandbox/state.json

Large diffs are not rendered by default.

71 changes: 68 additions & 3 deletions test/marketplace/SeaportProxyBuyer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,71 @@ describe('SeaportProxyBuyer', async () => {
});
});

describe('Zone security', () => {
it('should not allow to fulfill order outside of the zone', async () => {
const priceToSell = BigInt(ethers.parseUnits('100', 6));
const recipientFeesBasisPoints = BigInt(50);
const { fulfillOrderData, numerator, denominator } = await createSellOrder(
priceToSell,
recipientFeesBasisPoints,
proxyBuyerAddress,
);

await expect(
seaportContract
.connect(coinbase)
.fulfillAdvancedOrder(
{ ...fulfillOrderData, numerator, denominator, extraData: '0x' },
[],
ethers.ZeroHash,
buyer.address,
),
).to.be.revertedWithCustomError(seaportProxyBuyer, 'InvalidFulfiller');
});

it('should not allow to match orders outside of the zone', async () => {
await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
await unsRegistry.connect(seller).setApprovalForAll(await seaportContract.getAddress(), true);

const price = BigInt(ethers.parseUnits('100', 6));
const recipientFeesBasisPoints = BigInt(0);

await usdcMock.mint(buyer.address, price);

const sellOrder = await createSellOrder(price, recipientFeesBasisPoints, proxyBuyerAddress);
const buyOrder = await createBuyOrder(price, recipientFeesBasisPoints);

const orders: AdvancedOrderStruct[] = [
{
...sellOrder.fulfillOrderData,
numerator: sellOrder.numerator,
denominator: sellOrder.denominator,
extraData: '0x',
},
{
...buyOrder.fulfillOrderData,
numerator: buyOrder.numerator,
denominator: buyOrder.denominator,
extraData: '0x',
},
];
const fulfillments: MatchOrdersFulfillment[] = [
{
offerComponents: [{ orderIndex: 0, itemIndex: 0 }],
considerationComponents: [{ orderIndex: 1, itemIndex: 0 }],
}, // NFT from Order 1 to Buyer in Order 2
{
offerComponents: [{ orderIndex: 1, itemIndex: 0 }],
considerationComponents: [{ orderIndex: 0, itemIndex: 0 }],
}, // USDC from Order 2 to Seller in Order 1
];

await expect(
seaportContract.connect(reader).matchAdvancedOrders(orders, [], fulfillments, proxyBuyerAddress),
).to.be.revertedWithCustomError(seaportProxyBuyer, 'InvalidFulfiller');
});
});

describe('Match orders', async () => {
it('should match 2 fullfiling orders from a random caller using ProxyBuyer as a zone', async () => {
await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
Expand Down Expand Up @@ -796,7 +861,7 @@ describe('SeaportProxyBuyer', async () => {
expect(domainOwner).to.be.eq(buyer.address);
expect(initialSeaportProxyBuyerBalance).to.be.eq(seaportProxyBuyerBalance);
expect(readerBalance).to.be.eq(0);
});
}).skip();

it('should match 2 fullfiling orders from a random caller using ProxyBuyer as a zone with a price leftover', async () => {
await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
Expand Down Expand Up @@ -863,7 +928,7 @@ describe('SeaportProxyBuyer', async () => {
expect(domainOwner).to.be.eq(buyer.address);
expect(initialSeaportProxyBuyerBalance).to.be.eq(seaportProxyBuyerBalance - (priceToBuy - priceToSell));
expect(readerBalance).to.be.eq(0);
});
}).skip();

it('should not allow to match orders with paused SeaportProxyBuyer', async () => {
await usdcMock.connect(buyer).approve(await seaportContract.getAddress(), ethers.MaxUint256);
Expand Down Expand Up @@ -908,6 +973,6 @@ describe('SeaportProxyBuyer', async () => {
.connect(reader)
.matchAdvancedOrders(orders, [], fulfillments, await seaportProxyBuyer.getAddress()),
).to.be.revertedWith('Pausable: paused');
});
}).skip();
});
});
Loading