Miscellaneous fixes to RFQ origin registration (#48)

* Contracts can no longer call registerAllowedRfqOrigins()
* registerAllowedRfqOrigins() is now plural, registers an array of
addresses
This commit is contained in:
Steve Marx 2020-11-25 14:08:15 -05:00 committed by GitHub
parent 7591e99316
commit ad337271d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 83 additions and 24 deletions

View File

@ -89,14 +89,14 @@ interface INativeOrdersFeature {
uint256 minValidSalt
);
/// @dev Emitted when a new address is registered or unregistered to fill
/// @dev Emitted when new addresses are allowed or disallowed to fill
/// orders with a given txOrigin.
/// @param origin The address doing the registration.
/// @param addr The address being registered.
/// @param origin The address doing the allowing.
/// @param addrs The address being allowed/disallowed.
/// @param allowed Indicates whether the address should be allowed.
event RfqOrderOriginAllowed(
event RfqOrderOriginsAllowed(
address origin,
address addr,
address[] addrs,
bool allowed
);
@ -220,9 +220,9 @@ interface INativeOrdersFeature {
/// @dev Mark what tx.origin addresses are allowed to fill an order that
/// specifies the message sender as its txOrigin.
/// @param origin The origin to update.
/// @param origins An array of origin addresses to update.
/// @param allowed True to register, false to unregister.
function registerAllowedRfqOrigin(address origin, bool allowed)
function registerAllowedRfqOrigins(address[] memory origins, bool allowed)
external;
/// @dev Cancel multiple limit orders. The caller must be the maker.

View File

@ -146,7 +146,7 @@ contract NativeOrdersFeature is
_registerFeatureFunction(this.getLimitOrderHash.selector);
_registerFeatureFunction(this.getRfqOrderHash.selector);
_registerFeatureFunction(this.getProtocolFeeMultiplier.selector);
_registerFeatureFunction(this.registerAllowedRfqOrigin.selector);
_registerFeatureFunction(this.registerAllowedRfqOrigins.selector);
return LibMigrate.MIGRATE_SUCCESS;
}
@ -493,7 +493,7 @@ contract NativeOrdersFeature is
require(
makerTokens.length == takerTokens.length &&
makerTokens.length == minValidSalts.length,
"LimitOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS"
"NativeOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS"
);
for (uint256 i = 0; i < makerTokens.length; ++i) {
@ -551,21 +551,26 @@ contract NativeOrdersFeature is
/// @dev Mark what tx.origin addresses are allowed to fill an order that
/// specifies the message sender as its txOrigin.
/// @param origin The origin to update.
/// @param origins An array of origin addresses to update.
/// @param allowed True to register, false to unregister.
function registerAllowedRfqOrigin(
address origin,
function registerAllowedRfqOrigins(
address[] memory origins,
bool allowed
)
external
override
{
require(msg.sender == tx.origin,
"NativeOrdersFeature/NO_CONTRACT_ORIGINS");
LibNativeOrdersStorage.Storage storage stor =
LibNativeOrdersStorage.getStorage();
stor.originRegistry[msg.sender][origin] = allowed;
for (uint256 i = 0; i < origins.length; i++) {
stor.originRegistry[msg.sender][origins[i]] = allowed;
}
emit RfqOrderOriginAllowed(msg.sender, origin, allowed);
emit RfqOrderOriginsAllowed(msg.sender, origins, allowed);
}
/// @dev Cancel all RFQ orders for a given maker and pair with a salt less
@ -586,7 +591,7 @@ contract NativeOrdersFeature is
require(
makerTokens.length == takerTokens.length &&
makerTokens.length == minValidSalts.length,
"LimitOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS"
"NativeOrdersFeature/MISMATCHED_PAIR_ORDERS_ARRAY_LENGTHS"
);
for (uint256 i = 0; i < makerTokens.length; ++i) {

View File

@ -0,0 +1,34 @@
/*
Copyright 2020 ZeroEx Intl.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
pragma solidity ^0.6.5;
pragma experimental ABIEncoderV2;
import "../src/features/INativeOrdersFeature.sol";
contract TestRfqOriginRegistration {
function registerAllowedRfqOrigins(
INativeOrdersFeature feature,
address[] memory origins,
bool allowed
)
external
{
feature.registerAllowedRfqOrigins(origins, allowed);
}
}

View File

@ -42,7 +42,7 @@
"config": {
"publicInterfaceContracts": "IZeroEx,ZeroEx,FullMigration,InitialMigration,IFlashWallet,IAllowanceTarget,IERC20Transformer,IOwnableFeature,ISimpleFunctionRegistryFeature,ITokenSpenderFeature,ITransformERC20Feature,FillQuoteTransformer,PayTakerTransformer,WethTransformer,OwnableFeature,SimpleFunctionRegistryFeature,TransformERC20Feature,TokenSpenderFeature,AffiliateFeeTransformer,SignatureValidatorFeature,MetaTransactionsFeature,LogMetadataTransformer,BridgeAdapter,LiquidityProviderFeature,ILiquidityProviderFeature,NativeOrdersFeature,INativeOrdersFeature",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually.",
"abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|AllowanceTarget|BootstrapFeature|BridgeAdapter|FeeCollector|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinProtocolFees|FixinReentrancyGuard|FixinTokenSpender|FlashWallet|FullMigration|IAllowanceTarget|IBootstrapFeature|IBridgeAdapter|IERC20Bridge|IERC20Transformer|IExchange|IFeature|IFlashWallet|IGasToken|ILiquidityProvider|ILiquidityProviderFeature|ILiquidityProviderSandbox|IMetaTransactionsFeature|INativeOrdersFeature|IOwnableFeature|ISignatureValidatorFeature|ISimpleFunctionRegistryFeature|IStaking|ITestSimpleFunctionRegistryFeature|ITokenSpenderFeature|ITransformERC20Feature|IUniswapFeature|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibLiquidityProviderRichErrors|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibNativeOrder|LibNativeOrdersRichErrors|LibNativeOrdersStorage|LibOrderHash|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibReentrancyGuardStorage|LibSignature|LibSignatureRichErrors|LibSignedCallData|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibSpenderRichErrors|LibStorage|LibTokenSpenderStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|LiquidityProviderFeature|LiquidityProviderSandbox|LogMetadataTransformer|MetaTransactionsFeature|MixinAdapterAddresses|MixinBalancer|MixinCurve|MixinDodo|MixinKyber|MixinMStable|MixinMooniswap|MixinOasis|MixinShell|MixinSushiswap|MixinUniswap|MixinUniswapV2|MixinZeroExBridge|NativeOrdersFeature|OwnableFeature|PayTakerTransformer|SignatureValidatorFeature|SimpleFunctionRegistryFeature|TestBridge|TestCallTarget|TestDelegateCaller|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFixinProtocolFees|TestFixinTokenSpender|TestFullMigration|TestInitialMigration|TestLibNativeOrder|TestLibSignature|TestLiquidityProvider|TestMetaTransactionsNativeOrdersFeature|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestNativeOrdersFeature|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestStaking|TestTokenSpender|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TokenSpenderFeature|TransformERC20Feature|Transformer|TransformerDeployer|UniswapFeature|WethTransformer|ZeroEx|ZeroExOptimized).json"
"abis": "./test/generated-artifacts/@(AffiliateFeeTransformer|AllowanceTarget|BootstrapFeature|BridgeAdapter|FeeCollector|FillQuoteTransformer|FixinCommon|FixinEIP712|FixinProtocolFees|FixinReentrancyGuard|FixinTokenSpender|FlashWallet|FullMigration|IAllowanceTarget|IBootstrapFeature|IBridgeAdapter|IERC20Bridge|IERC20Transformer|IExchange|IFeature|IFlashWallet|IGasToken|ILiquidityProvider|ILiquidityProviderFeature|ILiquidityProviderSandbox|IMetaTransactionsFeature|INativeOrdersFeature|IOwnableFeature|ISignatureValidatorFeature|ISimpleFunctionRegistryFeature|IStaking|ITestSimpleFunctionRegistryFeature|ITokenSpenderFeature|ITransformERC20Feature|IUniswapFeature|IZeroEx|InitialMigration|LibBootstrap|LibCommonRichErrors|LibERC20Transformer|LibLiquidityProviderRichErrors|LibMetaTransactionsRichErrors|LibMetaTransactionsStorage|LibMigrate|LibNativeOrder|LibNativeOrdersRichErrors|LibNativeOrdersStorage|LibOrderHash|LibOwnableRichErrors|LibOwnableStorage|LibProxyRichErrors|LibProxyStorage|LibReentrancyGuardStorage|LibSignature|LibSignatureRichErrors|LibSignedCallData|LibSimpleFunctionRegistryRichErrors|LibSimpleFunctionRegistryStorage|LibSpenderRichErrors|LibStorage|LibTokenSpenderStorage|LibTransformERC20RichErrors|LibTransformERC20Storage|LibWalletRichErrors|LiquidityProviderFeature|LiquidityProviderSandbox|LogMetadataTransformer|MetaTransactionsFeature|MixinAdapterAddresses|MixinBalancer|MixinCurve|MixinDodo|MixinKyber|MixinMStable|MixinMooniswap|MixinOasis|MixinShell|MixinSushiswap|MixinUniswap|MixinUniswapV2|MixinZeroExBridge|NativeOrdersFeature|OwnableFeature|PayTakerTransformer|SignatureValidatorFeature|SimpleFunctionRegistryFeature|TestBridge|TestCallTarget|TestDelegateCaller|TestFillQuoteTransformerBridge|TestFillQuoteTransformerExchange|TestFillQuoteTransformerHost|TestFixinProtocolFees|TestFixinTokenSpender|TestFullMigration|TestInitialMigration|TestLibNativeOrder|TestLibSignature|TestLiquidityProvider|TestMetaTransactionsNativeOrdersFeature|TestMetaTransactionsTransformERC20Feature|TestMigrator|TestMintTokenERC20Transformer|TestMintableERC20Token|TestNativeOrdersFeature|TestRfqOriginRegistration|TestSimpleFunctionRegistryFeatureImpl1|TestSimpleFunctionRegistryFeatureImpl2|TestStaking|TestTokenSpender|TestTokenSpenderERC20Token|TestTransformERC20|TestTransformerBase|TestTransformerDeployerTransformer|TestTransformerHost|TestWeth|TestWethTransformerHost|TestZeroExFeature|TokenSpenderFeature|TransformERC20Feature|Transformer|TransformerDeployer|UniswapFeature|WethTransformer|ZeroEx|ZeroExOptimized).json"
},
"repository": {
"type": "git",

View File

@ -110,6 +110,7 @@ import * as TestMigrator from '../test/generated-artifacts/TestMigrator.json';
import * as TestMintableERC20Token from '../test/generated-artifacts/TestMintableERC20Token.json';
import * as TestMintTokenERC20Transformer from '../test/generated-artifacts/TestMintTokenERC20Transformer.json';
import * as TestNativeOrdersFeature from '../test/generated-artifacts/TestNativeOrdersFeature.json';
import * as TestRfqOriginRegistration from '../test/generated-artifacts/TestRfqOriginRegistration.json';
import * as TestSimpleFunctionRegistryFeatureImpl1 from '../test/generated-artifacts/TestSimpleFunctionRegistryFeatureImpl1.json';
import * as TestSimpleFunctionRegistryFeatureImpl2 from '../test/generated-artifacts/TestSimpleFunctionRegistryFeatureImpl2.json';
import * as TestStaking from '../test/generated-artifacts/TestStaking.json';
@ -244,6 +245,7 @@ export const artifacts = {
TestMintTokenERC20Transformer: TestMintTokenERC20Transformer as ContractArtifact,
TestMintableERC20Token: TestMintableERC20Token as ContractArtifact,
TestNativeOrdersFeature: TestNativeOrdersFeature as ContractArtifact,
TestRfqOriginRegistration: TestRfqOriginRegistration as ContractArtifact,
TestSimpleFunctionRegistryFeatureImpl1: TestSimpleFunctionRegistryFeatureImpl1 as ContractArtifact,
TestSimpleFunctionRegistryFeatureImpl2: TestSimpleFunctionRegistryFeatureImpl2 as ContractArtifact,
TestStaking: TestStaking as ContractArtifact,

View File

@ -8,7 +8,7 @@ import { IZeroExContract, IZeroExEvents } from '../../src/wrappers';
import { artifacts } from '../artifacts';
import { fullMigrateAsync } from '../utils/migration';
import { getRandomLimitOrder, getRandomRfqOrder } from '../utils/orders';
import { TestMintableERC20TokenContract } from '../wrappers';
import { TestMintableERC20TokenContract, TestRfqOriginRegistrationContract } from '../wrappers';
blockchainTests.resets('NativeOrdersFeature', env => {
const { NULL_ADDRESS, MAX_UINT256, ZERO_AMOUNT } = constants;
@ -24,6 +24,7 @@ blockchainTests.resets('NativeOrdersFeature', env => {
let makerToken: TestMintableERC20TokenContract;
let takerToken: TestMintableERC20TokenContract;
let wethToken: TestMintableERC20TokenContract;
let testRfqOriginRegistration: TestRfqOriginRegistrationContract;
before(async () => {
let owner;
@ -57,6 +58,12 @@ blockchainTests.resets('NativeOrdersFeature', env => {
takerToken.approve(zeroEx.address, MAX_UINT256).awaitTransactionSuccessAsync({ from: a }),
),
);
testRfqOriginRegistration = await TestRfqOriginRegistrationContract.deployFrom0xArtifactAsync(
artifacts.TestRfqOriginRegistration,
env.provider,
env.txDefaults,
artifacts,
);
});
function getTestLimitOrder(fields: Partial<LimitOrderFields> = {}): LimitOrder {
@ -1048,6 +1055,15 @@ blockchainTests.resets('NativeOrdersFeature', env => {
expect(takerBalance).to.bignumber.eq(makerTokenFilledAmount);
}
describe('registerAllowedRfqOrigins()', () => {
it('cannot register through a contract', async () => {
const tx = testRfqOriginRegistration
.registerAllowedRfqOrigins(zeroEx.address, [], true)
.awaitTransactionSuccessAsync();
expect(tx).to.revertWith('NativeOrdersFeature/NO_CONTRACT_ORIGINS');
});
});
describe('fillRfqOrder()', () => {
it('can fully fill an order', async () => {
const order = getTestRfqOrder();
@ -1155,18 +1171,18 @@ blockchainTests.resets('NativeOrdersFeature', env => {
const order = getTestRfqOrder();
const receipt = await zeroEx
.registerAllowedRfqOrigin(notTaker, true)
.registerAllowedRfqOrigins([notTaker], true)
.awaitTransactionSuccessAsync({ from: taker });
verifyEventsFromLogs(
receipt.logs,
[
{
origin: taker,
addr: notTaker,
addrs: [notTaker],
allowed: true,
},
],
IZeroExEvents.RfqOrderOriginAllowed,
IZeroExEvents.RfqOrderOriginsAllowed,
);
return fillRfqOrderAsync(order, order.takerAmount, notTaker);
});
@ -1174,20 +1190,20 @@ blockchainTests.resets('NativeOrdersFeature', env => {
it('cannot fill an order with registered then unregistered tx.origin', async () => {
const order = getTestRfqOrder();
await zeroEx.registerAllowedRfqOrigin(notTaker, true).awaitTransactionSuccessAsync({ from: taker });
await zeroEx.registerAllowedRfqOrigins([notTaker], true).awaitTransactionSuccessAsync({ from: taker });
const receipt = await zeroEx
.registerAllowedRfqOrigin(notTaker, false)
.registerAllowedRfqOrigins([notTaker], false)
.awaitTransactionSuccessAsync({ from: taker });
verifyEventsFromLogs(
receipt.logs,
[
{
origin: taker,
addr: notTaker,
addrs: [notTaker],
allowed: false,
},
],
IZeroExEvents.RfqOrderOriginAllowed,
IZeroExEvents.RfqOrderOriginsAllowed,
);
const tx = fillRfqOrderAsync(order, order.takerAmount, notTaker);

View File

@ -108,6 +108,7 @@ export * from '../test/generated-wrappers/test_migrator';
export * from '../test/generated-wrappers/test_mint_token_erc20_transformer';
export * from '../test/generated-wrappers/test_mintable_erc20_token';
export * from '../test/generated-wrappers/test_native_orders_feature';
export * from '../test/generated-wrappers/test_rfq_origin_registration';
export * from '../test/generated-wrappers/test_simple_function_registry_feature_impl1';
export * from '../test/generated-wrappers/test_simple_function_registry_feature_impl2';
export * from '../test/generated-wrappers/test_staking';

View File

@ -135,6 +135,7 @@
"test/generated-artifacts/TestMintTokenERC20Transformer.json",
"test/generated-artifacts/TestMintableERC20Token.json",
"test/generated-artifacts/TestNativeOrdersFeature.json",
"test/generated-artifacts/TestRfqOriginRegistration.json",
"test/generated-artifacts/TestSimpleFunctionRegistryFeatureImpl1.json",
"test/generated-artifacts/TestSimpleFunctionRegistryFeatureImpl2.json",
"test/generated-artifacts/TestStaking.json",