crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Style audit for proxies + libmem + libbytes

Browse Source
This commit is contained in:
Greg Hysen
2018-06-01 11:54:20 -07:00
parent f03e5c6bd1
commit 3ed13150e1
8 changed files with 72 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
View File

@@ -47,7 +47,7 @@ contract ERC20Proxy is
)
internal
{
// Decode proxy data.
// Decode asset data.
(
uint8 proxyId,
address token

4
packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
View File

@@ -48,7 +48,7 @@ contract ERC721Proxy is
)
internal
{
// Decode proxy data.
// Decode asset data.
(
uint8 proxyId,
address token,
@@ -118,7 +118,7 @@ contract ERC721Proxy is
INVALID_ASSET_DATA_LENGTH
);
// Decode asset data
// Decode asset data.
proxyId = uint8(assetData[0]);
token = readAddress(assetData, 1);
tokenId = readUint256(assetData, 21);

11
packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
View File

@@ -155,7 +155,6 @@ contract TestLibBytes is
return b;
}
=======
/// @dev Reads the first 4 bytes from a byte array of arbitrary length.
/// @param b Byte array to read first 4 bytes from.
/// @return First 4 bytes of data.
@@ -168,6 +167,10 @@ contract TestLibBytes is
return result;
}
/// @dev Reads nested bytes from a specific position.
/// @param b Byte array containing nested bytes.
/// @param index Index of nested bytes.
/// @return result Nested bytes.
function publicReadBytes(
bytes memory b,
uint256 index)
@@ -179,7 +182,11 @@ contract TestLibBytes is
return result;
}
/// @dev Inserts bytes at a specific position in a byte array.
/// @param b Byte array to insert <input> into.
/// @param index Index in byte array of <input>.
/// @param input bytes to insert.
/// @return b Updated input byte array
function publicWriteBytes(
bytes memory b,
uint256 index,

15
packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol
View File

@@ -23,8 +23,15 @@ import "../../utils/LibMem/LibMem.sol";
contract TestLibMem is
LibMem
{
/// @dev Copies a block of memory from one location to another.
/// @param mem Memory contents we want to apply memcpy to
/// @param dest Destination offset into <mem>.
/// @param source Source offset into <mem>.
/// @param length Length of bytes to copy from <source> to <dest>
/// @return mem Memory contents after calling memcpy.
function testMemcpy(
bytes mem, ///< Memory contents we want to apply memcpy to
bytes mem,
uint256 dest,
uint256 source,
uint256 length
@@ -36,13 +43,13 @@ contract TestLibMem is
// Sanity check. Overflows are not checked.
require(source + length <= mem.length);
require(dest + length <= mem.length);
// Get pointer to memory contents
uint256 offset = getMemAddress(mem) + 32;
// Execute memcpy adjusted for memory array location
memcpy(offset + dest, offset + source, length);
// Return modified memory contents
return mem;
}

13
packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
View File

@@ -268,7 +268,6 @@ contract LibBytes is
writeBytes32(b, index, bytes32(input));
}
=======
/// @dev Reads the first 4 bytes from a byte array of arbitrary length.
/// @param b Byte array to read first 4 bytes from.
/// @return First 4 bytes of data.
@@ -287,10 +286,10 @@ contract LibBytes is
return result;
}
/// @dev Reads a uint256 value from a position in a byte array.
/// @param b Byte array containing a uint256 value.
/// @param index Index in byte array of uint256 value.
/// @return uint256 value from byte array.
/// @dev Reads nested bytes from a specific position.
/// @param b Byte array containing nested bytes.
/// @param index Index of nested bytes.
/// @return result Nested bytes.
function readBytes(
bytes memory b,
uint256 index
@@ -321,10 +320,10 @@ contract LibBytes is
return result;
}
/// @dev Writes a uint256 into a specific position in a byte array.
/// @dev Inserts bytes at a specific position in a byte array.
/// @param b Byte array to insert <input> into.
/// @param index Index in byte array of <input>.
/// @param input uint256 to put into byte array.
/// @param input bytes to insert.
function writeBytes(
bytes memory b,
uint256 index,

18
packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol
View File

@@ -18,23 +18,27 @@
pragma solidity ^0.4.24;
contract LibMem {
contract LibMem
{
/// @dev Gets the memory address for a byte array.
/// @param input Byte array to lookup.
/// @return memoryAddress Memory address of byte array.
function getMemAddress(bytes memory input)
internal
pure
returns (uint256 address_)
returns (uint256 memoryAddress)
{
assembly {
address_ := input
memoryAddress := input
}
return address_;
return memoryAddress;
}
/// @dev Copies `length` bytes from memory location `source` to `dest`.
/// @param dest memory address to copy bytes to
/// @param source memory address to copy bytes from
/// @param length number of bytes to copy
/// @param dest memory address to copy bytes to.
/// @param source memory address to copy bytes from.
/// @param length number of bytes to copy.
function memcpy(
uint256 dest,
uint256 source,

21
packages/contracts/test/asset_proxy/decoder.ts
View File

@@ -19,7 +19,7 @@ chaiSetup.configure();
const expect = chai.expect;
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
describe('LibAssetProxyDecoder', () => {
describe('TestAssetDataDecoders', () => {
let owner: string;
let testAssetProxyDecoder: TestAssetDataDecodersContract;
let testAddress: string;
@@ -43,8 +43,8 @@ describe('LibAssetProxyDecoder', () => {
await blockchainLifecycle.revertAsync();
});
describe('LibAssetProxyDecoder', () => {
it('should correctly decode ERC20 proxy data)', async () => {
describe('Asset Data Decoders', () => {
it('should correctly decode ERC20 asset data)', async () => {
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(testAddress);
const expectedDecodedAssetData = assetProxyUtils.decodeERC20AssetData(encodedAssetData);
let decodedAssetProxyId: number;
@@ -56,7 +56,7 @@ describe('LibAssetProxyDecoder', () => {
expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
});
it('should correctly decode ERC721 proxy data', async () => {
it('should correctly decode ERC721 asset data', async () => {
const tokenId = ZeroEx.generatePseudoRandomSalt();
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId);
const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
@@ -76,25 +76,26 @@ describe('LibAssetProxyDecoder', () => {
expect(decodedData).to.be.equal(expectedDecodedAssetData.data);
});
it('should correctly decode ERC721 proxy data with receiver data', async () => {
it('should correctly decode ERC721 asset data with receiver data', async () => {
const tokenId = ZeroEx.generatePseudoRandomSalt();
const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())) + 'FFFF';
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, data);
const receiverData =
ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())) + 'FFFF';
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData);
const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
let decodedAssetProxyId: number;
let decodedTokenAddress: string;
let decodedTokenId: BigNumber;
let decodedData: string;
let decodedReceiverData: string;
[
decodedAssetProxyId,
decodedTokenAddress,
decodedTokenId,
decodedData,
decodedReceiverData,
] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData);
expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId);
expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId);
expect(decodedData).to.be.equal(expectedDecodedAssetData.data);
expect(decodedReceiverData).to.be.equal(expectedDecodedAssetData.data);
});
});
});

38
packages/contracts/test/asset_proxy/proxies.ts
View File

@@ -106,7 +106,7 @@ describe('Asset Transfer Proxies', () => {
describe('Transfer Proxy - ERC20', () => {
describe('transferFrom', () => {
it('should successfully transfer tokens', async () => {
// Construct metadata for ERC20 proxy
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
// Perform a transfer from makerAddress to takerAddress
const erc20Balances = await erc20Wrapper.getBalancesAsync();
@@ -132,7 +132,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should do nothing if transferring 0 amount of a token', async () => {
// Construct metadata for ERC20 proxy
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
// Perform a transfer from makerAddress to takerAddress
const erc20Balances = await erc20Wrapper.getBalancesAsync();
@@ -158,7 +158,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if allowances are too low', async () => {
// Construct metadata for ERC20 proxy
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
// Create allowance less than transfer amount. Set allowance on proxy.
const allowance = new BigNumber(0);
@@ -182,7 +182,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if requesting address is not authorized', async () => {
// Construct metadata for ERC20 proxy
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(10);
@@ -258,7 +258,7 @@ describe('Asset Transfer Proxies', () => {
describe('Transfer Proxy - ERC721', () => {
describe('transferFrom', () => {
it('should successfully transfer tokens', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -281,7 +281,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should not call onERC721Received when transferring to a smart contract without receiver data', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -310,12 +310,14 @@ describe('Asset Transfer Proxies', () => {
});
it('should call onERC721Received when transferring to a smart contract with receiver data', async () => {
// Construct metadata for ERC721 proxy
const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()));
// Construct ERC721 asset data
const receiverData = ethUtil.bufferToHex(
assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()),
);
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(
erc721Token.address,
erc721MakerTokenId,
data,
receiverData,
);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -340,19 +342,21 @@ describe('Asset Transfer Proxies', () => {
const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>;
expect(tokenReceivedLog.args.from).to.be.equal(makerAddress);
expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId);
expect(tokenReceivedLog.args.data).to.be.equal(data);
expect(tokenReceivedLog.args.data).to.be.equal(receiverData);
// Verify transfer was successful
const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address);
});
it('should throw if there is receiver data but contract does not have onERC721Received', async () => {
// Construct metadata for ERC721 proxy
const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()));
// Construct ERC721 asset data
const receiverData = ethUtil.bufferToHex(
assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()),
);
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(
erc721Token.address,
erc721MakerTokenId,
data,
receiverData,
);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -372,7 +376,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if transferring 0 amount of a token', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -391,7 +395,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if transferring > 1 amount of a token', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
@@ -410,7 +414,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if allowances are too low', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Remove transfer approval for makerAddress.
await web3Wrapper.awaitTransactionSuccessAsync(
@@ -429,7 +433,7 @@ describe('Asset Transfer Proxies', () => {
});
it('should throw if requesting address is not authorized', async () => {
// Construct metadata for ERC721 proxy
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);