Skip to content

Commit

Permalink
feat: remove backend signature from the exchange
Browse files Browse the repository at this point in the history
  • Loading branch information
@adjisb
adjisb committed Sep 27, 2023
1 parent 1e8206b commit 272d35e
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 264 deletions.
10 changes: 3 additions & 7 deletions packages/marketplace/contracts/exchange/Exchange.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {ExchangeCore} from "./ExchangeCore.sol";
/// @dev TransferManager is used to execute token transfers
contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, TransferManager, ERC2771HandlerUpgradeable {


/// @notice role erc1776 trusted meta transaction contracts (Sand for example).
/// @return hash for ERC1776_OPERATOR_ROLE
bytes32 public constant ERC1776_OPERATOR_ROLE = keccak256("ERC1776_OPERATOR_ROLE");
Expand Down Expand Up @@ -53,7 +52,6 @@ contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, Tran
bool newMetaNative
) external initializer {
__ERC2771Handler_init(newTrustedForwarder);
// TODO: Switch to a version that takes an admin address
__AccessControl_init();
__TransferManager_init_unchained(
newProtocolFeePrimary,
Expand Down Expand Up @@ -107,15 +105,13 @@ contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, Tran

/// @notice direct purchase orders - can handle bulk purchases
/// @param direct array of purchase order
/// @param signature array of signed message specifying order details with the buyer
/// @dev The buyer param was added so the function is compatible with Sand approveAndCall
function directPurchase(
address buyer,
LibDirectTransfer.Purchase[] calldata direct,
bytes[] calldata signature
LibDirectTransfer.Purchase[] calldata direct
) external payable {
for (uint256 i; i < direct.length;) {
_directPurchase(_msgSender(), buyer, direct[i], signature[i]);
_directPurchase(_msgSender(), buyer, direct[i]);
unchecked {
i++;
}
Expand All @@ -125,7 +121,7 @@ contract Exchange is Initializable, AccessControlUpgradeable, ExchangeCore, Tran
/// @notice cancel order
/// @param order to be canceled
/// @dev require msg sender to be order maker and salt different from 0
function cancel(LibOrder.Order memory order, bytes32 orderHash) external {
function cancel(LibOrder.Order calldata order, bytes32 orderHash) external {
require(_msgSender() == order.maker, "ExchangeCore: not maker");
_cancel(order, orderHash);
}
Expand Down
21 changes: 2 additions & 19 deletions packages/marketplace/contracts/exchange/ExchangeCore.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
/// @notice cancel order
/// @param order to be canceled
/// @dev require msg sender to be order maker and salt different from 0
function _cancel(LibOrder.Order memory order, bytes32 orderHash) internal {
function _cancel(LibOrder.Order calldata order, bytes32 orderHash) internal {
require(order.salt != 0, "ExchangeCore: 0 salt can't be used");
bytes32 orderKeyHash = LibOrder.hashKey(order);
require(orderHash == orderKeyHash, "ExchangeCore: Invalid orderHash");
Expand All @@ -122,26 +122,9 @@ abstract contract ExchangeCore is Initializable, TransferExecutor, ITransferMana
function _directPurchase(
address from,
address buyer,
LibDirectTransfer.Purchase calldata direct,
bytes calldata signature
LibDirectTransfer.Purchase calldata direct
) internal {
LibAsset.AssetType memory paymentAssetType = getPaymentAssetType(direct.paymentToken);

LibOrder.OrderBack memory orderBack = LibOrder.OrderBack(
buyer,
direct.sellOrderMaker,
LibAsset.Asset(LibAsset.AssetType(direct.nftAssetClass, direct.nftData), direct.sellOrderNftAmount),
address(0),
LibAsset.Asset(paymentAssetType, direct.sellOrderPaymentAmount),
direct.sellOrderSalt,
direct.sellOrderStart,
direct.sellOrderEnd,
direct.sellOrderDataType,
direct.sellOrderData
);

require(orderValidator.isPurchaseValid(orderBack, signature), "INVALID_PURCHASE");

LibOrder.Order memory sellOrder = LibOrder.Order(
direct.sellOrderMaker,
LibAsset.Asset(LibAsset.AssetType(direct.nftAssetClass, direct.nftData), direct.sellOrderNftAmount),
Expand Down
33 changes: 0 additions & 33 deletions packages/marketplace/contracts/exchange/OrderValidator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,8 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh
using ECDSAUpgradeable for bytes32;
using AddressUpgradeable for address;

address private _signingWallet;

bytes4 internal constant MAGICVALUE = 0x1626ba7e;

/// @notice event for when a new _signingWallet is set
/// @param newSigningWallet The new address of the signing wallet
event SigningWallet(address indexed newSigningWallet);

/// @notice initializer for OrderValidator
/// @param newTsbOnly boolean to indicate that only The Sandbox tokens are accepted by the exchange contract
/// @param newPartners boolena to indicate that partner tokens are accepted by the exchange contract
Expand All @@ -39,33 +33,6 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh
__Whitelist_init(newTsbOnly, newPartners, newOpen, newErc20);
}

/// @notice Update the signing wallet address
/// @param newSigningWallet The new address of the signing wallet
function setSigningWallet(address newSigningWallet) external onlyOwner {
require(newSigningWallet != address(0), "WALLET_ZERO_ADDRESS");
require(newSigningWallet != _signingWallet, "WALLET_ALREADY_SET");
_signingWallet = newSigningWallet;

emit SigningWallet(newSigningWallet);
}

/// @notice Get the wallet authorized for signing purchase-messages.
/// @return _signingWallet the address of the signing wallet
function getSigningWallet() external view returns (address) {
return _signingWallet;
}

/// @notice verifies if backend signature is valid
/// @param orderBack order to be validated
/// @param signature signature of order
/// @return boolean comparison between the recover signature and signing wallet
function isPurchaseValid(LibOrder.OrderBack memory orderBack, bytes memory signature) external view returns (bool) {
bytes32 hash = LibOrder.backendHash(orderBack);
address recoveredSigner = _hashTypedDataV4(hash).recover(signature);

return recoveredSigner == _signingWallet;
}

/// @notice verifies order
/// @param order order to be validated
/// @param signature signature of order
Expand Down
6 changes: 0 additions & 6 deletions packages/marketplace/contracts/interfaces/IOrderValidator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ interface IOrderValidator {
/// @param sender order sender
function validate(LibOrder.Order memory order, bytes memory signature, address sender) external view;

/// @notice verifies if backend signature is valid
/// @param orderBack order to be validated
/// @param signature signature of order
/// @return boolean comparison between the recover signature and signing wallet
function isPurchaseValid(LibOrder.OrderBack memory orderBack, bytes memory signature) external view returns (bool);

/// @notice if ERC20 token is accepted
/// @param tokenAddress ERC20 token address
function verifyERC20Whitelist(address tokenAddress) external view;
Expand Down
40 changes: 0 additions & 40 deletions packages/marketplace/contracts/lib-order/LibOrder.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ library LibOrder {
"Order(address maker,Asset makeAsset,address taker,Asset takeAsset,uint256 salt,uint256 start,uint256 end,bytes4 dataType,bytes data)Asset(AssetType assetType,uint256 value)AssetType(bytes4 assetClass,bytes data)"
);

bytes32 internal constant ORDER_BACK_TYPEHASH =
keccak256(
"OrderBack(address buyer,address maker,Asset makeAsset,address taker,Asset takeAsset,uint256 salt,uint256 start,uint256 end,bytes4 dataType,bytes data)Asset(AssetType assetType,uint256 value)AssetType(bytes4 assetClass,bytes data)"
);

bytes4 internal constant DEFAULT_ORDER_TYPE = 0xffffffff;

struct Order {
Expand All @@ -32,19 +27,6 @@ library LibOrder {
bytes data;
}

struct OrderBack {
address buyer;
address maker;
LibAsset.Asset makeAsset;
address taker;
LibAsset.Asset takeAsset;
uint256 salt;
uint256 start;
uint256 end;
bytes4 dataType;
bytes data;
}

/// @notice calculate the remaining fill from orders
/// @param order order that we will calculate the remaining fill
/// @param fill to be subtracted
Expand Down Expand Up @@ -115,28 +97,6 @@ library LibOrder {
);
}

/// @notice calculate hash from backend order
/// @param order object to be hashed
/// @return hash key of order
function backendHash(OrderBack memory order) internal pure returns (bytes32) {
return
keccak256(
abi.encode(
ORDER_BACK_TYPEHASH,
order.buyer,
order.maker,
LibAsset.hash(order.makeAsset),
order.taker,
LibAsset.hash(order.takeAsset),
order.salt,
order.start,
order.end,
order.dataType,
keccak256(order.data)
)
);
}

/// @notice validates order time
/// @param order whose time we want to validate
function validateOrderTime(LibOrder.Order memory order) internal view {
Expand Down
119 changes: 3 additions & 116 deletions packages/marketplace/test/exchange/OrderValidator.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import {deployFixtures} from '../fixtures';
import {loadFixture} from '@nomicfoundation/hardhat-network-helpers';
import {expect} from 'chai';
import {AssetERC20, AssetERC721, AssetETH} from '../utils/assets.ts';
import {AssetERC20, AssetETH} from '../utils/assets.ts';

import {OrderDefault, OrderBack, DEFAULT_ORDER_TYPE} from '../utils/order.ts';
import {OrderDefault} from '../utils/order.ts';
import {ZeroAddress} from 'ethers';
import {signOrder, signOrderBack} from '../utils/signature';
import {signOrder} from '../utils/signature';

// keccak256("TSB_ROLE")
const TSBRole =
Expand All @@ -18,119 +18,6 @@ const ERC20Role =
'0x839f6f26c78a3e8185d8004defa846bd7b66fef8def9b9f16459a6ebf2502162';

describe('OrderValidator.sol', function () {
it('should not set signing wallet if caller is not owner', async function () {
const {OrderValidatorAsUser, user1} = await loadFixture(deployFixtures);
await expect(
OrderValidatorAsUser.setSigningWallet(user1.address)
).to.revertedWith('Ownable: caller is not the owner');
});

it('should not be able to set signing wallet as zero address', async function () {
const {OrderValidatorAsDeployer} = await loadFixture(deployFixtures);
await expect(
OrderValidatorAsDeployer.setSigningWallet(ZeroAddress)
).to.revertedWith('WALLET_ZERO_ADDRESS');
});

it('should be able to set signing wallet', async function () {
const {OrderValidatorAsDeployer, deployer} = await loadFixture(
deployFixtures
);
await expect(OrderValidatorAsDeployer.setSigningWallet(deployer.address))
.to.emit(OrderValidatorAsDeployer, 'SigningWallet')
.withArgs(deployer.address);
});

it('should not be able to set same signing wallet again', async function () {
const {OrderValidatorAsDeployer, deployer} = await loadFixture(
deployFixtures
);
await OrderValidatorAsDeployer.setSigningWallet(deployer.address);
await expect(
OrderValidatorAsDeployer.setSigningWallet(deployer.address)
).to.revertedWith('WALLET_ALREADY_SET');
});

it('should return signing wallet address', async function () {
const {OrderValidatorAsDeployer, deployer} = await loadFixture(
deployFixtures
);
await OrderValidatorAsDeployer.setSigningWallet(deployer.address);
expect(await OrderValidatorAsDeployer.getSigningWallet()).to.be.equal(
deployer.address
);
});

it('should not validate a purchase when signature is invalid', async function () {
const {
OrderValidatorAsDeployer,
ERC20Contract,
ERC721Contract,
deployer,
user2,
user1,
} = await loadFixture(deployFixtures);

const makerAsset = await AssetERC721(ERC721Contract, 1);
const takerAsset = await AssetERC20(ERC20Contract, 100);
const order = await OrderBack(
user2,
user1,
makerAsset,
ZeroAddress,
takerAsset,
1,
0,
0,
DEFAULT_ORDER_TYPE,
'0x'
);
const signature = await signOrderBack(
order,
user1,
OrderValidatorAsDeployer
);
await OrderValidatorAsDeployer.setSigningWallet(deployer.address);
expect(
await OrderValidatorAsDeployer.isPurchaseValid(order, signature)
).to.be.equal(false);
});

it('should validate a purchase when the order and signature are valid', async function () {
const {
OrderValidatorAsDeployer,
ERC20Contract,
ERC721Contract,
deployer,
user2,
user1,
} = await loadFixture(deployFixtures);

const makerAsset = await AssetERC721(ERC721Contract, 1);
const takerAsset = await AssetERC20(ERC20Contract, 100);
const order = await OrderBack(
user2,
user1,
makerAsset,
ZeroAddress,
takerAsset,
1,
0,
0,
DEFAULT_ORDER_TYPE,
'0x'
);
const signature = await signOrderBack(
order,
deployer,
OrderValidatorAsDeployer
);
await OrderValidatorAsDeployer.setSigningWallet(deployer.address);
expect(
await OrderValidatorAsDeployer.isPurchaseValid(order, signature)
).to.be.equal(true);
});

it('should validate when assetClass is not ETH_ASSET_CLASS', async function () {
const {OrderValidatorAsUser, ERC20Contract, user1} = await loadFixture(
deployFixtures
Expand Down
43 changes: 0 additions & 43 deletions packages/marketplace/test/utils/order.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,6 @@ export const ORDER_TYPEHASH = keccak256(
)
);

export const ORDER_BACK_TYPEHASH = keccak256(
Buffer.from(
'OrderBack(address buyer,address maker,Asset makeAsset,address taker,Asset takeAsset,uint256 salt,uint256 start,uint256 end,bytes4 dataType,bytes data)Asset(AssetType assetType,uint256 value)AssetType(bytes4 assetClass,bytes data)'
)
);

const ORDER_DATA_BUY = bytes4Keccak('BUY');
const ORDER_DATA_SELL = bytes4Keccak('SELL');

Expand All @@ -36,43 +30,6 @@ export type Order = {
data: BytesLike;
};

export type OrderBack = {
buyer: string;
maker: string;
makeAsset: Asset;
taker: string;
takeAsset: Asset;
salt: Numeric;
start: Numeric;
end: Numeric;
dataType: string;
data: BytesLike;
};

export const OrderBack = async (
buyer: Signer,
maker: Signer,
makeAsset: Asset,
taker: Signer | ZeroAddress,
takeAsset: Asset,
salt: Numeric,
start: Numeric,
end: Numeric,
dataType: string,
data: string
): Promise<OrderBack> => ({
buyer: await buyer.getAddress(),
maker: await maker.getAddress(),
makeAsset,
taker: taker === ZeroAddress ? ZeroAddress : await taker.getAddress(),
takeAsset,
salt,
start,
end,
dataType: DEFAULT_ORDER_TYPE,
data: '0x',
});

export const OrderDefault = async (
maker: Signer,
makeAsset: Asset,
Expand Down

1 comment on commit 272d35e

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for this commit

31.08%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
packages/marketplace/contracts/exchange
   AssetMatcher.sol78.08%73.33%100%79.49%49–50, 50, 50–51, 53, 56, 59, 73–74, 74, 74–75, 77, 80–81
   Exchange.sol57.58%54.55%56.25%60.71%103, 113–116, 131, 131–132, 138, 138–139, 144, 174, 174, 174, 182, 53, 94, 94, 96–97
   ExchangeCore.sol45.02%28.75%73.68%51.79%127–128, 140, 151–153, 160, 162, 174, 186–187, 205–206, 206, 206, 206, 206, 208–209, 209, 209, 209, 209, 257, 257, 257–258, 258, 258–259, 259, 259, 261–262, 262, 262–263, 263, 263–264, 264, 264, 266, 268–269, 269, 269–270, 270, 270, 272, 300–301, 303–304, 361, 370–372, 372, 372–374, 374, 374–375, 375–377, 377, 377, 379, 381, 381, 381, 383, 394, 397–398, 402–403, 406, 429, 431–433, 439–441, 466–467, 484, 501–502, 502, 502–503, 505–506, 508, 515, 515, 515–516, 518, 518, 518–519, 521, 85, 93
   ExchangeMeta.sol0%0%0%0%37, 37–40, 46, 56, 60, 65, 65–66, 66, 66, 68
   OrderValidator.sol69.81%57.14%100%80.95%31, 47, 54–55, 55, 55, 65, 75–76, 81, 83, 83, 83, 83–84, 86
   WhiteList.sol97.62%93.75%100%100%50
packages/marketplace/contracts/exchange/libraries
   LibDirectTransfer.sol100%100%100%100%
   LibFill.sol55%33.33%66.67%63.64%48–49, 61, 71–72, 72, 72–73
   LibOrderDataGeneric.sol21.67%22.22%40%18.92%18–28, 30–31, 33, 46, 48, 48, 48–49, 51–52, 55, 55, 55–56, 58, 61, 61, 61–62, 64, 67, 71, 73, 73, 73–75, 78, 87, 87, 87–89
packages/marketplace/contracts/exchange/mocks
   ERC1155LazyMintTest.sol100%100%100%100%
   ERC721LazyMintTest.sol100%100%100%100%
   ExchangeTestImports.sol100%100%100%100%
   LibFillTest.sol0%100%0%0%17
   LibOrderTest.sol0%100%0%0%13, 17, 21, 25, 35, 47
   MockTrustedForwarder.sol0%0%0%0%14, 17–18, 18, 18–19
   OrderValidatorTest.sol0%0%0%0%13, 8, 8–9
   RaribleTestHelper.sol0%100%0%0%11, 15, 19, 23, 33
   SimpleTransferManager.sol0%100%0%0%13–16
   TestAssetMatcher.sol0%0%0%0%12, 12, 12–15, 15, 15–16, 19
   TestERC1155WithRoyaltyV2981.sol100%100%100%100%
   TestERC1271.sol0%0%0%0%24, 28, 28, 28
   TestERC20.sol100%100%100%100%
   TestERC721.sol100%100%100%100%
   TestERC721WithRoyaltyV2981.sol100%100%100%100%
   TestERC721WithRoyaltyV2981Multi.sol100%100%100%100%
   TestMinimalForwarder.sol0%0%0%0%12–15, 15, 15–16
   TestRoyaltiesRegistry.sol18%16.67%25%17.86%21–23, 23, 23–24, 24, 24–26, 28, 28, 28–29, 33, 38–39, 42–46, 50, 62–65, 65, 65–66, 66, 66–68, 70, 70, 70–71
   TransferManagerTest.sol0%0%0%0%106, 109–110, 114–115, 118, 125–126, 128, 21, 21–22, 34, 45–46, 71, 71, 71, 77, 80–82, 82, 82–84, 84, 84–85, 85–87, 87, 87, 89, 91, 91, 91, 93
packages/marketplace/contracts/exchange/mocks/tokens
   ERC2981.sol0%0%0%0%106, 38, 38, 38, 45, 47, 47, 47–48, 51, 53, 62, 74, 74, 74–75, 75, 75, 77, 84, 96, 96, 96–97, 97, 97, 99
   MintableERC1155.sol0%100%0%0%10, 14
   MintableERC1155WithRoyalties.sol0%0%0%0%13, 17, 21, 25, 9, 9, 9
   MintableERC20.sol0%100%0%0%10
   MintableERC721.sol0%100%0%0%10
   MintableERC721WithRoyalties.sol0%0%0%0%13, 17, 21, 25, 9, 9,

Please sign in to comment.