Address comments

This commit is contained in:
Michael Zhu 2020-03-04 16:36:06 -08:00
parent 9c5ac26170
commit 490094939f
3 changed files with 63 additions and 53 deletions

View File

@ -17,7 +17,6 @@
*/ */
pragma solidity ^0.5.16; pragma solidity ^0.5.16;
pragma experimental ABIEncoderV2;
contract MaximumGasPrice { contract MaximumGasPrice {
@ -38,12 +37,11 @@ contract MaximumGasPrice {
/// @dev Checks that the current transaction's gas price is less than /// @dev Checks that the current transaction's gas price is less than
/// the specified maximum value. /// the specified maximum value.
/// @param data Encodes the maximum gas price. /// @param maxGasPrice The maximum gas price allowed for the current transaction.
function checkGasPrice(bytes calldata data) function checkGasPrice(uint256 maxGasPrice)
external external
view view
{ {
(uint256 maxGasPrice) = abi.decode(data, (uint256));
require( require(
tx.gasprice <= maxGasPrice, tx.gasprice <= maxGasPrice,
"MaximumGasPrice/GAS_PRICE_EXCEEDS_MAXIMUM" "MaximumGasPrice/GAS_PRICE_EXCEEDS_MAXIMUM"

View File

@ -3,8 +3,7 @@ import { assetDataUtils } from '@0x/order-utils';
import { StaticCallAssetData } from '@0x/types'; import { StaticCallAssetData } from '@0x/types';
import { AbiEncoder, BigNumber } from '@0x/utils'; import { AbiEncoder, BigNumber } from '@0x/utils';
const maxGasPriceEncoder = AbiEncoder.create([{ name: 'maxGasPrice', type: 'uint256' }]); const customGasPriceEncoder = AbiEncoder.createMethod('checkGasPrice', [{ name: 'maxGasPrice', type: 'uint256' }]);
const customGasPriceEncoder = AbiEncoder.createMethod('checkGasPrice', [{ name: 'data', type: 'bytes' }]);
const defaultGasPriceEncoder = AbiEncoder.createMethod('checkGasPrice', []); const defaultGasPriceEncoder = AbiEncoder.createMethod('checkGasPrice', []);
const ONE_GWEI = new BigNumber(10 ** 9); const ONE_GWEI = new BigNumber(10 ** 9);
@ -16,11 +15,7 @@ export const TWENTY_GWEI = ONE_GWEI.times(20);
*/ */
export function encodeMaxGasPriceStaticCallData(maxGasPriceContractAddress: string, maxGasPrice?: BigNumber): string { export function encodeMaxGasPriceStaticCallData(maxGasPriceContractAddress: string, maxGasPrice?: BigNumber): string {
const staticCallData = const staticCallData =
maxGasPrice === undefined maxGasPrice === undefined ? defaultGasPriceEncoder.encode({}) : customGasPriceEncoder.encode({ maxGasPrice });
? defaultGasPriceEncoder.encode({})
: customGasPriceEncoder.encode({
data: maxGasPriceEncoder.encode({ maxGasPrice }),
});
return assetDataUtils.encodeStaticCallAssetData( return assetDataUtils.encodeStaticCallAssetData(
maxGasPriceContractAddress, maxGasPriceContractAddress,
staticCallData, staticCallData,
@ -35,8 +30,7 @@ export function decodeMaxGasPriceStaticCallData(assetData: string): BigNumber {
// tslint:disable-next-line:no-unnecessary-type-assertion // tslint:disable-next-line:no-unnecessary-type-assertion
const { staticCallData } = assetDataUtils.decodeAssetDataOrThrow(assetData) as StaticCallAssetData; const { staticCallData } = assetDataUtils.decodeAssetDataOrThrow(assetData) as StaticCallAssetData;
try { try {
const { maxGasPrice } = maxGasPriceEncoder.decode(customGasPriceEncoder.strictDecode<string>(staticCallData)); return customGasPriceEncoder.strictDecode<BigNumber>(staticCallData);
return maxGasPrice;
} catch (e) { } catch (e) {
defaultGasPriceEncoder.strictDecode(staticCallData); defaultGasPriceEncoder.strictDecode(staticCallData);
return TWENTY_GWEI; return TWENTY_GWEI;

View File

@ -1,5 +1,5 @@
import { blockchainTests, constants, expect, getRandomInteger } from '@0x/contracts-test-utils'; import { artifacts as assetProxyArtifacts, StaticCallProxyContract } from '@0x/contracts-asset-proxy';
import { AbiEncoder } from '@0x/utils'; import { blockchainTests, constants, expect, getRandomInteger, randomAddress } from '@0x/contracts-test-utils';
import { import {
decodeMaxGasPriceStaticCallData, decodeMaxGasPriceStaticCallData,
@ -12,7 +12,9 @@ import { MaximumGasPriceContract } from './wrappers';
blockchainTests.resets('MaximumGasPrice unit tests', env => { blockchainTests.resets('MaximumGasPrice unit tests', env => {
let maxGasPriceContract: MaximumGasPriceContract; let maxGasPriceContract: MaximumGasPriceContract;
const maxGasPriceEncoder = AbiEncoder.create([{ name: 'maxGasPrice', type: 'uint256' }]); let staticCallProxy: StaticCallProxyContract;
let defaultMaxAssetData: string;
before(async () => { before(async () => {
maxGasPriceContract = await MaximumGasPriceContract.deployFrom0xArtifactAsync( maxGasPriceContract = await MaximumGasPriceContract.deployFrom0xArtifactAsync(
@ -21,51 +23,67 @@ blockchainTests.resets('MaximumGasPrice unit tests', env => {
env.txDefaults, env.txDefaults,
artifacts, artifacts,
); );
}); staticCallProxy = await StaticCallProxyContract.deployFrom0xArtifactAsync(
assetProxyArtifacts.StaticCallProxy,
env.provider,
env.txDefaults,
assetProxyArtifacts,
);
describe('Contract functionality', () => { defaultMaxAssetData = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address);
it('does not revert if tx.gasprice < default maximum', async () => {
await maxGasPriceContract.checkGasPrice1().callAsync({ gasPrice: TWENTY_GWEI.minus(1) });
});
it('does not revert if tx.gasprice = default maximum', async () => {
await maxGasPriceContract.checkGasPrice1().callAsync({ gasPrice: TWENTY_GWEI });
});
it('reverts if tx.gasPrice > default maximum', async () => {
const tx = maxGasPriceContract.checkGasPrice1().callAsync({ gasPrice: TWENTY_GWEI.plus(1) });
return expect(tx).to.revertWith('MaximumGasPrice/GAS_PRICE_EXCEEDS_20_GWEI');
});
it('does not revert if tx.gasprice < custom maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
await maxGasPriceContract
.checkGasPrice2(maxGasPriceEncoder.encode({ maxGasPrice }))
.callAsync({ gasPrice: maxGasPrice.minus(1) });
});
it('does not revert if tx.gasprice = default maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
await maxGasPriceContract
.checkGasPrice2(maxGasPriceEncoder.encode({ maxGasPrice }))
.callAsync({ gasPrice: maxGasPrice });
});
it('reverts if tx.gasPrice > default maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
const tx = maxGasPriceContract
.checkGasPrice2(maxGasPriceEncoder.encode({ maxGasPrice }))
.callAsync({ gasPrice: maxGasPrice.plus(1) });
return expect(tx).to.revertWith('MaximumGasPrice/GAS_PRICE_EXCEEDS_MAXIMUM');
});
}); });
describe('Data encoding/decoding tools', () => { describe('Data encoding/decoding tools', () => {
it('correctly decodes default maximum gas price', async () => { it('correctly decodes default maximum gas price', async () => {
const encoded = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address); const decoded = decodeMaxGasPriceStaticCallData(defaultMaxAssetData);
const decoded = decodeMaxGasPriceStaticCallData(encoded);
expect(decoded).to.bignumber.equal(TWENTY_GWEI); expect(decoded).to.bignumber.equal(TWENTY_GWEI);
}); });
it('correctly decodes custom maximum gas price', async () => { it('correctly decodes custom maximum gas price', async () => {
const maxGasPrice = getRandomInteger(0, constants.MAX_UINT256); const customMaxGasPrice = getRandomInteger(0, constants.MAX_UINT256);
const encoded = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address, maxGasPrice); const customMaxAssetData = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address, customMaxGasPrice);
const decoded = decodeMaxGasPriceStaticCallData(encoded); const decoded = decodeMaxGasPriceStaticCallData(customMaxAssetData);
expect(decoded).to.bignumber.equal(maxGasPrice); expect(decoded).to.bignumber.equal(customMaxGasPrice);
});
});
describe('Contract functionality', () => {
it('does not revert if tx.gasprice < default maximum', async () => {
await staticCallProxy
.transferFrom(defaultMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: TWENTY_GWEI.minus(1) });
});
it('does not revert if tx.gasprice = default maximum', async () => {
await staticCallProxy
.transferFrom(defaultMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: TWENTY_GWEI });
});
it('reverts if tx.gasPrice > default maximum', async () => {
const tx = staticCallProxy
.transferFrom(defaultMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: TWENTY_GWEI.plus(1) });
return expect(tx).to.revertWith('MaximumGasPrice/GAS_PRICE_EXCEEDS_20_GWEI');
});
it('does not revert if tx.gasprice < custom maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
const customMaxAssetData = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address, maxGasPrice);
await staticCallProxy
.transferFrom(customMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: maxGasPrice.minus(1) });
});
it('does not revert if tx.gasprice = custom maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
const customMaxAssetData = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address, maxGasPrice);
await staticCallProxy
.transferFrom(customMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: maxGasPrice });
});
it('reverts if tx.gasPrice > custom maximum', async () => {
const maxGasPrice = getRandomInteger(0, TWENTY_GWEI.times(2));
const customMaxAssetData = encodeMaxGasPriceStaticCallData(maxGasPriceContract.address, maxGasPrice);
const tx = staticCallProxy
.transferFrom(customMaxAssetData, randomAddress(), randomAddress(), constants.ZERO_AMOUNT)
.callAsync({ gasPrice: maxGasPrice.plus(1) });
return expect(tx).to.revertWith('MaximumGasPrice/GAS_PRICE_EXCEEDS_MAXIMUM');
}); });
}); });
}); });