@0x:contracts-staking Addressed review comments

This commit is contained in:
Alex Towle 2019-09-11 16:58:41 -07:00
parent 82afdda256
commit b7f25ee3b6
3 changed files with 98 additions and 141 deletions

View File

@ -1,17 +1,12 @@
import { blockchainTests, constants, expect, hexRandom } from '@0x/contracts-test-utils';
import { blockchainTests, constants, expect, hexConcat, hexRandom, hexSlice } from '@0x/contracts-test-utils';
import { StakingRevertErrors } from '@0x/order-utils';
import { artifacts, TestLibProxyContract, TestLibProxyReceiverContract } from '../../src';
blockchainTests.resets('LibProxy', env => {
blockchainTests.resets.only('LibProxy', env => {
let proxy: TestLibProxyContract;
let receiver: TestLibProxyReceiverContract;
// Generates a random bytes4 value.
function randomBytes4(): string {
return hexRandom(4);
}
before(async () => {
proxy = await TestLibProxyContract.deployFrom0xArtifactAsync(
artifacts.TestLibProxy,
@ -38,14 +33,14 @@ blockchainTests.resets('LibProxy', env => {
// that it does not call `externalProxyCall` by accident. This calldata will make the fallback
// in `TestLibProxyReceiver` fail because it is 4 bytes long.
function constructRandomFailureCalldata(): string {
return '0x00'.concat(randomBytes4().slice(4, 10));
return hexConcat('0x00', hexRandom(3));
}
// Choose a random 24 byte string of calldata to send and prepend with `0x00` to ensure
// that it does not call `externalProxyCall` by accident. This calldata will make the fallback
// in `TestLibProxyReceiver` succeed because it isn't 4 bytes long.
function constructRandomSuccessCalldata(): string {
return '0x00'.concat(hexRandom(36).slice(2, 74));
return hexConcat('0x00', hexRandom(35));
}
interface PublicProxyCallArgs {
@ -71,7 +66,7 @@ blockchainTests.resets('LibProxy', env => {
describe('proxyCall', () => {
// Verifies that the result of a given call to `proxyCall()` results in specified outcome
function checkEndingConditions(result: [boolean, string], success: boolean, calldata: string): void {
function verifyPostConditions(result: [boolean, string], success: boolean, calldata: string): void {
expect(result[0]).to.be.eq(success);
expect(result[1]).to.be.eq(calldata);
}
@ -107,70 +102,12 @@ blockchainTests.resets('LibProxy', env => {
});
});
describe('Calldata Checks', () => {
it('should simply forward the calldata and succeed when customEngressSelector == bytes4(0), ignoreIngressSelector == false, and revertRule = RevertOnError', async () => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(await publicProxyCallAsync({ calldata }), true, calldata);
});
it('should send the customEgressSelector followed by the calldata when customEgressSelector != bytes4(0), ignoreIngressSelector == false, and revertRule == RevertOnError', async () => {
const calldata = constructRandomSuccessCalldata();
// Choose a random customEgressSelector selector.
const customEgressSelector = randomBytes4();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
customEgressSelector,
}),
true,
customEgressSelector.concat(calldata.slice(2, calldata.length)),
);
});
it('should send the the calldata without the selector when customEgressSelector == bytes4(0), ignoreIngressSelector == true, and revertRule == RevertOnError', async () => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
ignoreIngressSelector: true,
}),
true,
'0x'.concat(calldata.slice(10, calldata.length)),
);
});
it('should send the calldata with the customEgressSelector replacing its selctor when customEngressSelector != bytes4(0), ignoreIngressSelector == true, and revertRule == RevertOnError', async () => {
const calldata = constructRandomSuccessCalldata();
// Choose a random customEgressSelector selector.
const customEgressSelector = randomBytes4();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
customEgressSelector,
ignoreIngressSelector: true,
}),
true,
customEgressSelector.concat(calldata.slice(10, calldata.length)),
);
});
});
describe('RevertRule Checks', () => {
it('should revert with the correct data when the call succeeds and revertRule = AlwaysRevert', async () => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
revertRule: RevertRule.AlwaysRevert,
@ -184,7 +121,7 @@ blockchainTests.resets('LibProxy', env => {
const calldata = constructRandomFailureCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
revertRule: RevertRule.AlwaysRevert,
@ -198,7 +135,7 @@ blockchainTests.resets('LibProxy', env => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
revertRule: RevertRule.NeverRevert,
@ -212,7 +149,7 @@ blockchainTests.resets('LibProxy', env => {
const calldata = constructRandomFailureCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
revertRule: RevertRule.NeverRevert,
@ -226,7 +163,7 @@ blockchainTests.resets('LibProxy', env => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
}),
@ -236,12 +173,10 @@ blockchainTests.resets('LibProxy', env => {
});
it('should revert with the correct data when the call falls and revertRule = RevertOnError', async () => {
// Choose a random 4 byte string of calldata to send and replace the first byte with `0x00` to ensure
// that it does not call `publicProxyCall` by accident.
const calldata = '0x00'.concat(randomBytes4().slice(4, 10));
const calldata = constructRandomFailureCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
verifyPostConditions(
await publicProxyCallAsync({
calldata,
}),
@ -251,74 +186,88 @@ blockchainTests.resets('LibProxy', env => {
});
});
// For brevity, only `RevertOnError` was tested by the `customEgressSelector` and `ignoreIngressSelector` tests. These
// cases are intended to prevent regressions from occuring with the other two revert rules.
describe('Mixed Checks', () => {
it('should function correctly when customEgressSelector != bytes4(0) and revertRule == AlwaysRevert', async () => {
const calldata = constructRandomSuccessCalldata();
describe('Combinatorial Tests', () => {
const revertRuleScenarios: RevertRule[] = [
RevertRule.RevertOnError,
RevertRule.AlwaysRevert,
RevertRule.NeverRevert,
];
const ignoreIngressScenarios: boolean[] = [false, true];
const customEgressScenarios: string[] = [
constants.NULL_BYTES4,
constructRandomFailureCalldata(), // Random failure calldata is used because it is nonzero and won't collide.
];
const calldataScenarios: string[] = [constructRandomFailureCalldata(), constructRandomSuccessCalldata()];
// Choose a random customEgressSelector selector.
const customEgressSelector = randomBytes4();
function createTestDescription(
revertRule: RevertRule,
customEgressSelector: string,
ignoreIngressSelector: boolean,
calldata: string,
): string {
return `should work correctly when revertRule == ${revertRule}, customEgressSelector == ${customEgressSelector},
ignoreIngressSelector == ${ignoreIngressSelector}, calldata == ${calldata}`;
}
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
customEgressSelector,
revertRule: RevertRule.AlwaysRevert,
}),
false,
customEgressSelector.concat(calldata.slice(2, calldata.length)),
);
});
// Combinatorially test `proxyCall()` with all input types.
for (const revertRule of revertRuleScenarios) {
for (const customEgressSelector of customEgressScenarios) {
for (const shouldIgnoreIngressSelector of ignoreIngressScenarios) {
for (const calldata of calldataScenarios) {
it(
createTestDescription(
revertRule,
customEgressSelector,
shouldIgnoreIngressSelector,
calldata,
),
async () => {
// Determine whether or not the call should succeed.
let shouldSucceed = true;
if (
((shouldIgnoreIngressSelector &&
customEgressSelector !== constants.NULL_BYTES4) ||
(!shouldIgnoreIngressSelector &&
customEgressSelector === constants.NULL_BYTES4)) &&
calldata.length === 10 // This corresponds to a hex length of 4
) {
shouldSucceed = false;
}
it('should function correctly when customEgressSelector != bytes4(0) and revertRule == NeverRevert', async () => {
const calldata = constructRandomSuccessCalldata();
// Override the above success value if the RevertRule defines the success.
if (revertRule === RevertRule.AlwaysRevert) {
shouldSucceed = false;
}
if (revertRule === RevertRule.NeverRevert) {
shouldSucceed = true;
}
// Choose a random customEgressSelector selector.
const customEgressSelector = randomBytes4();
// Construct the data that should be returned.
let returnData: string = calldata;
if (shouldIgnoreIngressSelector) {
returnData = hexSlice(returnData, 4);
}
if (customEgressSelector !== constants.NULL_BYTES4) {
returnData = hexConcat(customEgressSelector, returnData);
}
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
customEgressSelector,
revertRule: RevertRule.NeverRevert,
}),
true,
customEgressSelector.concat(calldata.slice(2, calldata.length)),
);
});
it('should function correctly when ignoreIngressSelector == true and revertRule == AlwaysRevert', async () => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
ignoreIngressSelector: true,
revertRule: RevertRule.AlwaysRevert,
}),
false,
'0x'.concat(calldata.slice(10, calldata.length)),
);
});
it('should function correctly when ignoreIngressSelector == true and revertRule == NeverRevert', async () => {
const calldata = constructRandomSuccessCalldata();
// Ensure that the returndata (the provided calldata) is correct.
checkEndingConditions(
await publicProxyCallAsync({
calldata,
ignoreIngressSelector: true,
revertRule: RevertRule.NeverRevert,
}),
true,
'0x'.concat(calldata.slice(10, calldata.length)),
);
});
// Ensure that the test passes as expected.
verifyPostConditions(
await publicProxyCallAsync({
calldata,
customEgressSelector,
ignoreIngressSelector: shouldIgnoreIngressSelector,
revertRule,
}),
shouldSucceed,
returnData,
);
},
);
}
}
}
}
});
});
});

View File

@ -44,6 +44,14 @@ export function hexInvert(n: string | BigNumber | number, size: number = WORD_LE
return ethUtil.bufferToHex(Buffer.from(buf.map(b => ~b)));
}
/**
* Slices off the desired number of bytes from a hex number.
*/
export function hexSlice(n: string | BigNumber | number, size: number): string {
const hex = toHex(n);
return '0x'.concat(toHex(n).slice(size * 2 + 2, hex.length));
}
/**
* Convert a string, a number, or a BigNumber into a hex string.
* Works with negative numbers, as well.

View File

@ -28,7 +28,7 @@ export { bytes32Values, testCombinatoriallyWithReferenceFunc, uint256Values } fr
export { TransactionFactory } from './transaction_factory';
export { MutatorContractFunction, TransactionHelper } from './transaction_helper';
export { testWithReferenceFuncAsync } from './test_with_reference';
export { hexConcat, hexLeftPad, hexInvert, hexRandom, hexRightPad, toHex } from './hex_utils';
export { hexConcat, hexLeftPad, hexInvert, hexSlice, hexRandom, hexRightPad, toHex } from './hex_utils';
export {
BatchMatchOrder,
ContractName,