From feafd5702747eaff76675f57a5886d4ba3f8d3d1 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 19 Jun 2018 19:50:09 -0400 Subject: make metacoin tsconfig independent --- packages/metacoin/tsconfig.json | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/metacoin/tsconfig.json b/packages/metacoin/tsconfig.json index 8b4cd47a2..bc3bd1e94 100644 --- a/packages/metacoin/tsconfig.json +++ b/packages/metacoin/tsconfig.json @@ -1,7 +1,16 @@ { - "extends": "../../tsconfig", "compilerOptions": { - "outDir": "lib" + "outDir": "lib", + "lib": ["es2017"], + "skipLibCheck": true, + "typeRoots": [ + "comment: for building within 0x-monorepo:", + "../../node_modules/@0xproject/typescript-typings/types", + "../../node_modules/@types", + "comment: for building in an isolated environment:", + "node_modules/@0xproject/typescript-typings/types", + "node_modules/@types" + ] }, "include": ["src/**/*", "test/**/*"] } -- cgit v1.2.3 From 457f951aaf589670b3edd11be7849ac17ef7e6f4 Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 19 Jun 2018 19:50:32 -0400 Subject: add missing metacoin dependencies --- packages/metacoin/package.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/metacoin/package.json b/packages/metacoin/package.json index 6db14334e..1a7b75f73 100644 --- a/packages/metacoin/package.json +++ b/packages/metacoin/package.json @@ -36,11 +36,15 @@ "@0xproject/subproviders": "^0.10.4", "@0xproject/tslint-config": "^0.4.20", "@0xproject/types": "^0.8.1", + "@0xproject/typescript-typings": "^0.4.1", "@0xproject/utils": "^0.7.1", "@0xproject/web3-wrapper": "^0.7.1", + "@types/mocha": "^5.2.2", + "copyfiles": "^2.0.0", "ethereum-types": "^0.0.1", "ethers": "3.0.22", "lodash": "^4.17.4", + "run-s": "^0.0.0", "web3-provider-engine": "^14.0.4" }, "devDependencies": { -- cgit v1.2.3 From 663f9657ea208060abd23485a1b607fbc3cde67e Mon Sep 17 00:00:00 2001 From: "F. Eugene Aumson" Date: Tue, 19 Jun 2018 19:54:35 -0400 Subject: ethereum-types now only has v0.0.2, no 0.0.1 --- packages/metacoin/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metacoin/package.json b/packages/metacoin/package.json index 1a7b75f73..6156571a4 100644 --- a/packages/metacoin/package.json +++ b/packages/metacoin/package.json @@ -41,7 +41,7 @@ "@0xproject/web3-wrapper": "^0.7.1", "@types/mocha": "^5.2.2", "copyfiles": "^2.0.0", - "ethereum-types": "^0.0.1", + "ethereum-types": "^0.0.2", "ethers": "3.0.22", "lodash": "^4.17.4", "run-s": "^0.0.0", -- cgit v1.2.3 From bd03151c2a10c680bb8a12e0bdd73590381aa4ca Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Wed, 20 Jun 2018 14:03:56 -0700 Subject: Various relayer grid ui polish items --- packages/website/ts/components/portal/portal.tsx | 2 +- .../website/ts/components/portal/text_header.tsx | 15 ++---- .../components/relayer_index/relayer_grid_tile.tsx | 54 +++++++++++----------- .../relayer_index/relayer_top_tokens.tsx | 7 ++- packages/website/ts/pages/jobs/open_positions.tsx | 3 +- packages/website/ts/utils/utils.ts | 3 ++ 6 files changed, 42 insertions(+), 42 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index 11b3b43f4..cd88a3f95 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -551,7 +551,7 @@ export class Portal extends React.Component { private _renderRelayerIndexSection(): React.ReactNode { return (
} + header={} body={} /> ); diff --git a/packages/website/ts/components/portal/text_header.tsx b/packages/website/ts/components/portal/text_header.tsx index 4aabd47d0..853da3a29 100644 --- a/packages/website/ts/components/portal/text_header.tsx +++ b/packages/website/ts/components/portal/text_header.tsx @@ -1,21 +1,16 @@ -import { Styles } from '@0xproject/react-shared'; +import { colors } from '@0xproject/react-shared'; import * as React from 'react'; +import { Text } from 'ts/components/ui/text'; + export interface TextHeaderProps { labelText: string; } -const styles: Styles = { - title: { - fontWeight: 'bold', - fontSize: 20, - }, -}; - export const TextHeader = (props: TextHeaderProps) => { return ( -
+ {props.labelText} -
+ ); }; diff --git a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx index 23860856b..7f2df325e 100644 --- a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx +++ b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx @@ -1,6 +1,6 @@ import { constants as sharedConstants, Styles } from '@0xproject/react-shared'; import * as _ from 'lodash'; -import { GridTile } from 'material-ui/GridList'; +import { GridTile as PlainGridTile } from 'material-ui/GridList'; import * as React from 'react'; import { analytics } from 'ts/utils/analytics'; @@ -9,7 +9,9 @@ import { Container } from 'ts/components/ui/container'; import { Image } from 'ts/components/ui/image'; import { Island } from 'ts/components/ui/island'; import { colors } from 'ts/style/colors'; +import { styled } from 'ts/style/theme'; import { WebsiteBackendRelayerInfo } from 'ts/types'; +import { utils } from 'ts/utils/utils'; export interface RelayerGridTileProps { relayerInfo: WebsiteBackendRelayerInfo; @@ -21,24 +23,14 @@ const styles: Styles = { boxSizing: 'border-box', }, innerDiv: { - padding: 6, height: '100%', boxSizing: 'border-box', }, header: { height: '50%', width: '100%', - borderBottomRightRadius: 4, - borderBottomLeftRadius: 4, - borderTopRightRadius: 4, - borderTopLeftRadius: 4, - borderWidth: 1, - borderStyle: 'solid', - borderColor: colors.walletBorder, }, body: { - paddingLeft: 6, - paddingRight: 6, height: '50%', width: '100%', boxSizing: 'border-box', @@ -69,7 +61,10 @@ export const RelayerGridTile: React.StatelessComponent = ( const weeklyTxnVolume = props.relayerInfo.weeklyTxnVolume; const networkName = sharedConstants.NETWORK_NAME_BY_ID[props.networkId]; const eventLabel = `${props.relayerInfo.name}-${networkName}`; - const trackRelayerClick = () => analytics.logEvent('Portal', 'Relayer Click', eventLabel); + const onClick = () => { + analytics.logEvent('Portal', 'Relayer Click', eventLabel); + utils.openUrl(link); + }; const headerImageUrl = props.relayerInfo.logoImgUrl; const headerBackgroundColor = !_.isUndefined(headerImageUrl) && !_.isUndefined(props.relayerInfo.primaryColor) @@ -77,22 +72,17 @@ export const RelayerGridTile: React.StatelessComponent = ( : FALLBACK_PRIMARY_COLOR; return ( -
- -
- -
- -
-
+
+
+ +
+
+
{props.relayerInfo.name}
@@ -111,6 +101,14 @@ export const RelayerGridTile: React.StatelessComponent = ( ); }; +export const GridTile = styled(PlainGridTile)` + cursor: pointer; + transition: transform 0.2s ease; + &:hover { + transform: translate(0px, -3px); + } +`; + interface SectionProps { titleText: string; children?: React.ReactNode; diff --git a/packages/website/ts/components/relayer_index/relayer_top_tokens.tsx b/packages/website/ts/components/relayer_index/relayer_top_tokens.tsx index b599e7123..f544fc924 100644 --- a/packages/website/ts/components/relayer_index/relayer_top_tokens.tsx +++ b/packages/website/ts/components/relayer_index/relayer_top_tokens.tsx @@ -70,7 +70,10 @@ class TokenLink extends React.Component { }; const networkName = sharedConstants.NETWORK_NAME_BY_ID[this.props.networkId]; const eventLabel = `${this.props.tokenInfo.symbol}-${networkName}`; - const trackTokenClick = () => analytics.logEvent('Portal', 'Token Click', eventLabel); + const onClick = (event: React.MouseEvent) => { + event.stopPropagation(); + analytics.logEvent('Portal', 'Token Click', eventLabel); + }; return ( { style={style} onMouseEnter={this._onToggleHover.bind(this, true)} onMouseLeave={this._onToggleHover.bind(this, false)} - onClick={trackTokenClick} + onClick={onClick} > {this.props.tokenInfo.symbol} diff --git a/packages/website/ts/pages/jobs/open_positions.tsx b/packages/website/ts/pages/jobs/open_positions.tsx index f3d980315..e789795c1 100644 --- a/packages/website/ts/pages/jobs/open_positions.tsx +++ b/packages/website/ts/pages/jobs/open_positions.tsx @@ -11,6 +11,7 @@ import { colors } from 'ts/style/colors'; import { styled } from 'ts/style/theme'; import { ScreenWidths, WebsiteBackendJobInfo } from 'ts/types'; import { backendClient } from 'ts/utils/backend_client'; +import { utils } from 'ts/utils/utils'; const labelStyle = { fontFamily: 'Roboto Mono', fontSize: 18 }; const HEADER_TEXT = 'Open Positions'; @@ -161,7 +162,7 @@ export class OpenPositions extends React.Component Date: Thu, 21 Jun 2018 10:34:45 -0700 Subject: Add Background component --- packages/website/ts/components/portal/portal.tsx | 24 ++++++++++-------------- packages/website/ts/components/ui/background.tsx | 24 ++++++++++++++++++++++++ packages/website/ts/style/z_index.ts | 1 + 3 files changed, 35 insertions(+), 14 deletions(-) create mode 100644 packages/website/ts/components/ui/background.tsx diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index cd88a3f95..4f1dfea6f 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -22,6 +22,7 @@ import { RelayerIndex } from 'ts/components/relayer_index/relayer_index'; import { TokenBalances } from 'ts/components/token_balances'; import { TopBar, TopBarDisplayType } from 'ts/components/top_bar/top_bar'; import { TradeHistory } from 'ts/components/trade_history/trade_history'; +import { Background } from 'ts/components/ui/background'; import { Container } from 'ts/components/ui/container'; import { FlashMessage } from 'ts/components/ui/flash_message'; import { Island } from 'ts/components/ui/island'; @@ -109,22 +110,16 @@ const MENU_PADDING_LEFT = 185; const LARGE_LAYOUT_MAX_WIDTH = 1200; const styles: Styles = { - root: { - width: '100%', - height: '100%', - backgroundColor: colors.lightestGrey, - }, body: { - height: `calc(100vh - ${TOP_BAR_HEIGHT}px)`, + marginTop: TOP_BAR_HEIGHT, }, leftColumn: { width: LEFT_COLUMN_WIDTH, - height: '100%', + position: 'fixed', }, scrollContainer: { - height: `calc(100vh - ${TOP_BAR_HEIGHT}px)`, - WebkitOverflowScrolling: 'touch', - overflow: 'auto', + marginLeft: LEFT_COLUMN_WIDTH + 30, + marginRight: 30, }, }; @@ -245,7 +240,8 @@ export class Portal extends React.Component { ? TokenVisibility.UNTRACKED : TokenVisibility.TRACKED; return ( -
+
+ { blockchain={this._blockchain} translate={this.props.translate} displayType={TopBarDisplayType.Expanded} - style={{ backgroundColor: colors.lightestGrey }} + style={{ backgroundColor: colors.lightestGrey, position: 'fixed' }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} />
@@ -709,10 +705,10 @@ interface LargeLayoutProps { const LargeLayout = (props: LargeLayoutProps) => { return (
-
+
{props.left}
-
+
{props.right}
diff --git a/packages/website/ts/components/ui/background.tsx b/packages/website/ts/components/ui/background.tsx new file mode 100644 index 000000000..808792a41 --- /dev/null +++ b/packages/website/ts/components/ui/background.tsx @@ -0,0 +1,24 @@ +import { colors } from '@0xproject/react-shared'; +import * as React from 'react'; +import { styled } from 'ts/style/theme'; +import { zIndex } from 'ts/style/z_index'; + +export interface BackgroundProps { + color?: string; +} + +const PlainBackground: React.StatelessComponent = props =>
; + +export const Background = styled(PlainBackground)` + background-color: ${props => props.color}; + height: 100vh; + width: 100vw; + position: fixed; + z-index: ${zIndex.background}; +`; + +Background.defaultProps = { + color: colors.lightestGrey, +}; + +Background.displayName = 'Background'; diff --git a/packages/website/ts/style/z_index.ts b/packages/website/ts/style/z_index.ts index 0411cdd91..6e3aebaa1 100644 --- a/packages/website/ts/style/z_index.ts +++ b/packages/website/ts/style/z_index.ts @@ -1,4 +1,5 @@ export const zIndex = { + background: -1, topBar: 1100, overlay: 1105, aboveOverlay: 1106, -- cgit v1.2.3 From df94e5f164a2ce92dceb680d6835cbc700632da7 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 21 Jun 2018 12:55:00 -0700 Subject: Remove styles from Portal component --- packages/website/ts/components/portal/portal.tsx | 37 +++++++++--------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index 4f1dfea6f..c82bfbe6a 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -1,4 +1,4 @@ -import { colors, constants as sharedConstants, Styles } from '@0xproject/react-shared'; +import { colors, constants as sharedConstants } from '@0xproject/react-shared'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; import ActionAccountBalanceWallet from 'material-ui/svg-icons/action/account-balance-wallet'; @@ -108,20 +108,7 @@ const TOP_BAR_HEIGHT = TopBar.heightForDisplayType(TopBarDisplayType.Expanded); const LEFT_COLUMN_WIDTH = 346; const MENU_PADDING_LEFT = 185; const LARGE_LAYOUT_MAX_WIDTH = 1200; - -const styles: Styles = { - body: { - marginTop: TOP_BAR_HEIGHT, - }, - leftColumn: { - width: LEFT_COLUMN_WIDTH, - position: 'fixed', - }, - scrollContainer: { - marginLeft: LEFT_COLUMN_WIDTH + 30, - marginRight: 30, - }, -}; +const LARGE_LAYOUT_MARGIN = 30; export class Portal extends React.Component { private _blockchain: Blockchain; @@ -258,7 +245,7 @@ export class Portal extends React.Component { style={{ backgroundColor: colors.lightestGrey, position: 'fixed' }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} /> -
+ { tokenByAddress={this.props.tokenByAddress} tokenVisibility={tokenVisibility} /> -
+ { return (
-
{props.left}
-
-
- {props.right} + + {props.left} +
+ + + {props.right} + +
); }; @@ -721,9 +712,7 @@ interface SmallLayoutProps { const SmallLayout = (props: SmallLayoutProps) => { return (
-
- {props.content} -
+
{props.content}
); }; // tslint:disable:max-file-line-count -- cgit v1.2.3 From b6765b849cec691418bd0ccb38c584a146d97b19 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 21 Jun 2018 12:59:11 -0700 Subject: Padding adjustment --- packages/website/ts/components/relayer_index/relayer_grid_tile.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx index 7f2df325e..ba3579926 100644 --- a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx +++ b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx @@ -34,6 +34,7 @@ const styles: Styles = { height: '50%', width: '100%', boxSizing: 'border-box', + padding: 12, }, weeklyTradeVolumeLabel: { fontSize: 14, @@ -81,7 +82,7 @@ export const RelayerGridTile: React.StatelessComponent = ( height={RELAYER_ICON_HEIGHT} />
-
+
{props.relayerInfo.name}
-- cgit v1.2.3 From 3df58827119b2dd70a1a41ceccd304b6055500b8 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 21 Jun 2018 13:50:58 -0700 Subject: Remove Background component --- packages/website/ts/components/portal/portal.tsx | 4 +-- .../ts/components/relayer_index/relayer_index.tsx | 39 ++++++---------------- packages/website/ts/components/ui/background.tsx | 24 ------------- packages/website/ts/components/ui/container.tsx | 1 + packages/website/ts/style/z_index.ts | 1 - 5 files changed, 12 insertions(+), 57 deletions(-) delete mode 100644 packages/website/ts/components/ui/background.tsx diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index c82bfbe6a..ec46fd4cd 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -22,7 +22,6 @@ import { RelayerIndex } from 'ts/components/relayer_index/relayer_index'; import { TokenBalances } from 'ts/components/token_balances'; import { TopBar, TopBarDisplayType } from 'ts/components/top_bar/top_bar'; import { TradeHistory } from 'ts/components/trade_history/trade_history'; -import { Background } from 'ts/components/ui/background'; import { Container } from 'ts/components/ui/container'; import { FlashMessage } from 'ts/components/ui/flash_message'; import { Island } from 'ts/components/ui/island'; @@ -228,7 +227,6 @@ export class Portal extends React.Component { : TokenVisibility.TRACKED; return (
- { style={{ backgroundColor: colors.lightestGrey, position: 'fixed' }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} /> - + - - {this.state.relayerInfos.map((relayerInfo: WebsiteBackendRelayerInfo, index) => ( - - ))} - -
+ + {this.state.relayerInfos.map((relayerInfo: WebsiteBackendRelayerInfo, index) => ( + + ))} + ); } } diff --git a/packages/website/ts/components/ui/background.tsx b/packages/website/ts/components/ui/background.tsx deleted file mode 100644 index 808792a41..000000000 --- a/packages/website/ts/components/ui/background.tsx +++ /dev/null @@ -1,24 +0,0 @@ -import { colors } from '@0xproject/react-shared'; -import * as React from 'react'; -import { styled } from 'ts/style/theme'; -import { zIndex } from 'ts/style/z_index'; - -export interface BackgroundProps { - color?: string; -} - -const PlainBackground: React.StatelessComponent = props =>
; - -export const Background = styled(PlainBackground)` - background-color: ${props => props.color}; - height: 100vh; - width: 100vw; - position: fixed; - z-index: ${zIndex.background}; -`; - -Background.defaultProps = { - color: colors.lightestGrey, -}; - -Background.displayName = 'Background'; diff --git a/packages/website/ts/components/ui/container.tsx b/packages/website/ts/components/ui/container.tsx index 90aec0e7c..a747ef01f 100644 --- a/packages/website/ts/components/ui/container.tsx +++ b/packages/website/ts/components/ui/container.tsx @@ -15,6 +15,7 @@ export interface ContainerProps { borderRadius?: StringOrNum; maxWidth?: StringOrNum; width?: StringOrNum; + minHeight?: StringOrNum; isHidden?: boolean; className?: string; position?: 'absolute' | 'fixed' | 'relative' | 'unset'; diff --git a/packages/website/ts/style/z_index.ts b/packages/website/ts/style/z_index.ts index 6e3aebaa1..0411cdd91 100644 --- a/packages/website/ts/style/z_index.ts +++ b/packages/website/ts/style/z_index.ts @@ -1,5 +1,4 @@ export const zIndex = { - background: -1, topBar: 1100, overlay: 1105, aboveOverlay: 1106, -- cgit v1.2.3 From 82789d53155772e339c1c500bb538638dfc20376 Mon Sep 17 00:00:00 2001 From: fragosti Date: Thu, 21 Jun 2018 15:39:00 -0700 Subject: Fix issues that arose from merge --- packages/website/ts/components/onboarding/onboarding_flow.tsx | 6 +++++- packages/website/ts/components/portal/portal.tsx | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/website/ts/components/onboarding/onboarding_flow.tsx b/packages/website/ts/components/onboarding/onboarding_flow.tsx index 331899469..ec8d96191 100644 --- a/packages/website/ts/components/onboarding/onboarding_flow.tsx +++ b/packages/website/ts/components/onboarding/onboarding_flow.tsx @@ -42,7 +42,11 @@ export class OnboardingFlow extends React.Component { onboardingElement = {this._renderOnboardignCard()}; } else { onboardingElement = ( - + {this._renderPopperChildren.bind(this)} ); diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index ec46fd4cd..8606eed1b 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -226,7 +226,7 @@ export class Portal extends React.Component { ? TokenVisibility.UNTRACKED : TokenVisibility.TRACKED; return ( -
+ { style={{ backgroundColor: colors.lightestGrey, position: 'fixed' }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} /> - + { trackedTokenStateByAddress={this.state.trackedTokenStateByAddress} refetchTokenStateAsync={this._refetchTokenStateAsync.bind(this)} /> -
+ ); } private _renderMainRoute(): React.ReactNode { -- cgit v1.2.3 From b0000bb276df7609bea8a48252063693f0553222 Mon Sep 17 00:00:00 2001 From: fragosti Date: Thu, 21 Jun 2018 17:40:17 -0700 Subject: Fix z-index issues --- packages/website/ts/components/portal/portal.tsx | 79 ++++++++++++---------- .../components/relayer_index/relayer_grid_tile.tsx | 3 +- .../ts/components/relayer_index/relayer_index.tsx | 1 - packages/website/ts/components/ui/animation.tsx | 6 +- packages/website/ts/components/wallet/wallet.tsx | 20 +++--- packages/website/ts/utils/utils.ts | 3 + 6 files changed, 63 insertions(+), 49 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index 8606eed1b..e31404559 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -240,7 +240,12 @@ export class Portal extends React.Component { blockchain={this._blockchain} translate={this.props.translate} displayType={TopBarDisplayType.Expanded} - style={{ backgroundColor: colors.lightestGrey, position: 'fixed' }} + style={{ + backgroundColor: colors.lightestGrey, + position: 'fixed', + // Hack: used to make onboarding z-index logi work for both mobile and desktop + zIndex: utils.isMobile(this.props.screenWidth) ? zIndex.topBar : undefined, + }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} /> @@ -283,11 +288,6 @@ export class Portal extends React.Component { tokenVisibility={tokenVisibility} /> - ); } @@ -322,41 +322,48 @@ export class Portal extends React.Component { } private _renderWallet(): React.ReactNode { const startOnboarding = this._renderStartOnboarding(); - const isMobile = this.props.screenWidth === ScreenWidths.Sm; + const isMobile = utils.isMobile(this.props.screenWidth); // We need room to scroll down for mobile onboarding const marginBottom = isMobile ? '200px' : '15px'; return (
- {isMobile && {startOnboarding}} - - + + {isMobile && {startOnboarding}} + + + + {!isMobile && {startOnboarding}} - {!isMobile && {startOnboarding}} +
); } diff --git a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx index ba3579926..5f4c6998c 100644 --- a/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx +++ b/packages/website/ts/components/relayer_index/relayer_grid_tile.tsx @@ -21,6 +21,7 @@ export interface RelayerGridTileProps { const styles: Styles = { root: { boxSizing: 'border-box', + position: 'static', }, innerDiv: { height: '100%', @@ -102,7 +103,7 @@ export const RelayerGridTile: React.StatelessComponent = ( ); }; -export const GridTile = styled(PlainGridTile)` +const GridTile = styled(PlainGridTile)` cursor: pointer; transition: transform 0.2s ease; &:hover { diff --git a/packages/website/ts/components/relayer_index/relayer_index.tsx b/packages/website/ts/components/relayer_index/relayer_index.tsx index 5a6dc1ae9..4aea1bbbb 100644 --- a/packages/website/ts/components/relayer_index/relayer_index.tsx +++ b/packages/website/ts/components/relayer_index/relayer_index.tsx @@ -5,7 +5,6 @@ import * as React from 'react'; import { RelayerGridTile } from 'ts/components/relayer_index/relayer_grid_tile'; import { Retry } from 'ts/components/ui/retry'; -import { colors } from 'ts/style/colors'; import { ScreenWidths, WebsiteBackendRelayerInfo } from 'ts/types'; import { backendClient } from 'ts/utils/backend_client'; diff --git a/packages/website/ts/components/ui/animation.tsx b/packages/website/ts/components/ui/animation.tsx index cbda2993d..136f3d005 100644 --- a/packages/website/ts/components/ui/animation.tsx +++ b/packages/website/ts/components/ui/animation.tsx @@ -11,13 +11,15 @@ const PlainAnimation: React.StatelessComponent = props =>
{ public static defaultProps = { style: {}, @@ -244,17 +251,12 @@ export class Wallet extends React.Component { - + - - + + - +
Date: Thu, 21 Jun 2018 17:42:37 -0700 Subject: Lint and fix typo --- packages/website/ts/components/portal/portal.tsx | 2 +- packages/website/ts/components/wallet/wallet.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index e31404559..9a725c742 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -243,7 +243,7 @@ export class Portal extends React.Component { style={{ backgroundColor: colors.lightestGrey, position: 'fixed', - // Hack: used to make onboarding z-index logi work for both mobile and desktop + // Hack: used to make onboarding z-index logic work for both mobile and desktop zIndex: utils.isMobile(this.props.screenWidth) ? zIndex.topBar : undefined, }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} diff --git a/packages/website/ts/components/wallet/wallet.tsx b/packages/website/ts/components/wallet/wallet.tsx index 0f9e011b4..4cc70badd 100644 --- a/packages/website/ts/components/wallet/wallet.tsx +++ b/packages/website/ts/components/wallet/wallet.tsx @@ -29,6 +29,7 @@ import { WrapEtherItem } from 'ts/components/wallet/wrap_ether_item'; import { AllowanceToggle } from 'ts/containers/inputs/allowance_toggle'; import { Dispatcher } from 'ts/redux/dispatcher'; import { colors } from 'ts/style/colors'; +import { styled } from 'ts/style/theme'; import { BlockchainErrs, ProviderType, @@ -43,7 +44,6 @@ import { import { analytics } from 'ts/utils/analytics'; import { constants } from 'ts/utils/constants'; import { utils } from 'ts/utils/utils'; -import { styled } from 'ts/style/theme'; import { styles as walletItemStyles } from 'ts/utils/wallet_item_styles'; export interface WalletProps { -- cgit v1.2.3 From a07cfb7abc3649f29ab5020ff5d2bea37c26cefd Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Thu, 21 Jun 2018 16:53:57 -0700 Subject: Do not add empty token to local storage --- packages/website/ts/components/generate_order/asset_picker.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/website/ts/components/generate_order/asset_picker.tsx b/packages/website/ts/components/generate_order/asset_picker.tsx index b43ac1f2e..738692264 100644 --- a/packages/website/ts/components/generate_order/asset_picker.tsx +++ b/packages/website/ts/components/generate_order/asset_picker.tsx @@ -246,6 +246,14 @@ export class AssetPicker extends React.Component Date: Thu, 21 Jun 2018 17:57:22 -0700 Subject: Fix state mutation in reducer --- packages/website/ts/redux/reducer.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/website/ts/redux/reducer.ts b/packages/website/ts/redux/reducer.ts index 9d3d8f7d9..ed6a4868e 100644 --- a/packages/website/ts/redux/reducer.ts +++ b/packages/website/ts/redux/reducer.ts @@ -156,7 +156,7 @@ export function reducer(state: State = INITIAL_STATE, action: Action): State { } case ActionTypes.AddTokenToTokenByAddress: { - const newTokenByAddress = state.tokenByAddress; + const newTokenByAddress = { ...state.tokenByAddress }; newTokenByAddress[action.data.address] = action.data; return { ...state, @@ -165,7 +165,7 @@ export function reducer(state: State = INITIAL_STATE, action: Action): State { } case ActionTypes.RemoveTokenFromTokenByAddress: { - const newTokenByAddress = state.tokenByAddress; + const newTokenByAddress = { ...state.tokenByAddress }; delete newTokenByAddress[action.data.address]; return { ...state, @@ -174,7 +174,7 @@ export function reducer(state: State = INITIAL_STATE, action: Action): State { } case ActionTypes.UpdateTokenByAddress: { - const tokenByAddress = state.tokenByAddress; + const tokenByAddress = { ...state.tokenByAddress }; const tokens = action.data; _.each(tokens, token => { const updatedToken = { @@ -253,7 +253,7 @@ export function reducer(state: State = INITIAL_STATE, action: Action): State { } case ActionTypes.UpdateChosenAssetTokenAddress: { - const newAssetToken = state.sideToAssetToken[action.data.side]; + const newAssetToken = { ...state.sideToAssetToken[action.data.side] }; newAssetToken.address = action.data.address; const newSideToAssetToken = { ...state.sideToAssetToken, -- cgit v1.2.3 From 2d8e9eda56f1455d5d5de61924c9c061e64756f5 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 21 Jun 2018 15:16:26 -0700 Subject: Converted `hashZeroExTransaction` to assembly. Saves 1k gas --- .../protocol/Exchange/MixinTransactions.sol | 26 +++++++++++++++------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index d3d22d48f..86f7c7b72 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -51,17 +51,27 @@ contract MixinTransactions is /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @return EIP712 hash of the Transaction. - function hashZeroExTransaction(uint256 salt, address signerAddress, bytes data) + function hashZeroExTransaction( + uint256 salt, + address signerAddress, + bytes memory data + ) internal pure - returns (bytes32) + returns (bytes32 result) { - return keccak256(abi.encode( - EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, - salt, - signerAddress, - keccak256(data) - )); + bytes32 schemaHash = EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH; + bytes32 dataHash = keccak256(data); + assembly { + let memPtr := mload(64) + mstore(memPtr, schemaHash) + mstore(add(memPtr, 32), salt) + mstore(add(memPtr, 64), signerAddress) + mstore(add(memPtr, 96), dataHash) + result := keccak256(memPtr, 128) + } + + return result; } /// @dev Executes an exchange method call in the context of signer. -- cgit v1.2.3 From b2a4b7eb888131608d2526d806bc0461c31e8aef Mon Sep 17 00:00:00 2001 From: Jacob Evans Date: Fri, 22 Jun 2018 23:00:22 +1000 Subject: Fix known order hash value test --- packages/order-utils/test/order_hash_test.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/order-utils/test/order_hash_test.ts b/packages/order-utils/test/order_hash_test.ts index 28bfcbafc..0a6be83d0 100644 --- a/packages/order-utils/test/order_hash_test.ts +++ b/packages/order-utils/test/order_hash_test.ts @@ -12,8 +12,8 @@ const expect = chai.expect; describe('Order hashing', () => { describe('#getOrderHashHex', () => { - const expectedOrderHash = '0x367ad7730eb8b5feab8a9c9f47c6fcba77a2d4df125ee6a59cc26ac955710f7e'; - const fakeExchangeContractAddress = '0xb69e673309512a9d726f87304c6984054f87a93b'; + const expectedOrderHash = '0x434c6b41e2fb6dfcfe1b45c4492fb03700798e9c1afc6f801ba6203f948c1fa7'; + const fakeExchangeContractAddress = '0x1dc4c1cefef38a777b15aa20260a54e584b16c48'; const order: Order = { makerAddress: constants.NULL_ADDRESS, takerAddress: constants.NULL_ADDRESS, @@ -29,15 +29,11 @@ describe('Order hashing', () => { takerAssetAmount: new BigNumber(0), expirationTimeSeconds: new BigNumber(0), }; - // HACK: Temporarily disable these tests until @dekz has time to fix. - // This allows us to get all tests running on CI immediately - it.skip('calculates the order hash', async () => { + it('calculates the order hash', async () => { const orderHash = orderHashUtils.getOrderHashHex(order); expect(orderHash).to.be.equal(expectedOrderHash); }); - // HACK: Temporarily disable these tests until @dekz has time to fix. - // This allows us to get all tests running on CI immediately - it.skip('throws a readable error message if taker format is invalid', async () => { + it('throws a readable error message if taker format is invalid', async () => { const orderWithInvalidtakerFormat = { ...order, takerAddress: (null as any) as string, -- cgit v1.2.3 From 05c008844532a9386341f60e51270756341d00db Mon Sep 17 00:00:00 2001 From: fragosti Date: Fri, 22 Jun 2018 11:22:33 -0700 Subject: Comment out Chris, and move Mel up in about page --- packages/website/ts/pages/about/about.tsx | 23 +++++++++++------------ packages/website/ts/pages/about/profile.tsx | 1 + 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/website/ts/pages/about/about.tsx b/packages/website/ts/pages/about/about.tsx index 6830b64ab..62a2519a4 100644 --- a/packages/website/ts/pages/about/about.tsx +++ b/packages/website/ts/pages/about/about.tsx @@ -156,17 +156,6 @@ const teamRow5: ProfileInfo[] = [ linkedIn: 'https://www.linkedin.com/in/fragosti/', github: 'http://github.com/fragosti', }, - { - name: 'Chris Kalani', - title: 'Director of Design', - description: `Previously founded Wake (acquired by InVision). Early Facebook product designer.`, - image: 'images/team/chris.png', - linkedIn: 'https://www.linkedin.com/in/chriskalani/', - github: 'https://github.com/chriskalani', - }, -]; - -const teamRow6: ProfileInfo[] = [ { name: 'Mel Oberto', title: 'Office Operations / Executive Assistant', @@ -176,6 +165,17 @@ const teamRow6: ProfileInfo[] = [ }, ]; +const teamRow6: ProfileInfo[] = [ + // { + // name: 'Chris Kalani', + // title: 'Director of Design', + // description: `Previously founded Wake (acquired by InVision). Early Facebook product designer.`, + // image: 'images/team/chris.png', + // linkedIn: 'https://www.linkedin.com/in/chriskalani/', + // github: 'https://github.com/chriskalani', + // }, +]; + const advisors: ProfileInfo[] = [ { name: 'Fred Ehrsam', @@ -270,7 +270,6 @@ export class About extends React.Component {
{this._renderProfiles(teamRow3)}
{this._renderProfiles(teamRow4)}
{this._renderProfiles(teamRow5)}
-
{this._renderProfiles(teamRow6)}
{ fontSize: 14, fontFamily: 'Roboto Mono', color: colors.darkGrey, + whiteSpace: 'nowrap', }} > {props.profileInfo.title.toUpperCase()} -- cgit v1.2.3 From 543f2c91fe3d00be33b2193eb88529a53622506a Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Fri, 22 Jun 2018 11:37:57 -0700 Subject: Pulled out reset AssetPicker state into a common variable --- .../ts/components/generate_order/asset_picker.tsx | 24 ++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/website/ts/components/generate_order/asset_picker.tsx b/packages/website/ts/components/generate_order/asset_picker.tsx index 738692264..87618f1e0 100644 --- a/packages/website/ts/components/generate_order/asset_picker.tsx +++ b/packages/website/ts/components/generate_order/asset_picker.tsx @@ -232,12 +232,14 @@ export class AssetPicker extends React.Component { + const resetState: AssetPickerState = { + ...this.state, + isAddingTokenToTracked: false, + assetView: AssetViews.ASSET_PICKER, + chosenTrackTokenAddress: undefined, + }; if (!didUserAcceptTracking) { - this.setState({ - isAddingTokenToTracked: false, - assetView: AssetViews.ASSET_PICKER, - chosenTrackTokenAddress: undefined, - }); + this.setState(resetState); this._onCloseDialog(); return; } @@ -247,11 +249,7 @@ export class AssetPicker extends React.Component Date: Fri, 22 Jun 2018 11:47:57 -0700 Subject: Use Ops instead of Operations in Mels description --- packages/website/ts/pages/about/about.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/website/ts/pages/about/about.tsx b/packages/website/ts/pages/about/about.tsx index 62a2519a4..dc7ce265e 100644 --- a/packages/website/ts/pages/about/about.tsx +++ b/packages/website/ts/pages/about/about.tsx @@ -158,7 +158,7 @@ const teamRow5: ProfileInfo[] = [ }, { name: 'Mel Oberto', - title: 'Office Operations / Executive Assistant', + title: 'Office Ops / Executive Assistant', description: `Daily Operations. Previously People Operations Associate at Heap. Marketing and MBA at Sacred Heart University.`, image: 'images/team/mel.png', linkedIn: 'https://www.linkedin.com/in/melanieoberto', -- cgit v1.2.3 From 06705f9e88c5b8901e4edea08ffed92c8c6808f6 Mon Sep 17 00:00:00 2001 From: fragosti Date: Fri, 22 Jun 2018 11:54:24 -0700 Subject: Remove diverse from about page --- packages/website/ts/pages/about/about.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/website/ts/pages/about/about.tsx b/packages/website/ts/pages/about/about.tsx index dc7ce265e..e427104aa 100644 --- a/packages/website/ts/pages/about/about.tsx +++ b/packages/website/ts/pages/about/about.tsx @@ -259,9 +259,9 @@ export class About extends React.Component { lineHeight: 1.5, }} > - Our team is a diverse and globally distributed group with backgrounds in engineering, - research, business and design. We are passionate about decentralized technology and its - potential to act as an equalizing force in the world. + Our team is a globally distributed group with backgrounds in engineering, research, business + and design. We are passionate about decentralized technology and its potential to act as an + equalizing force in the world.
-- cgit v1.2.3 From 80fe1938b8cc7ad2b72972133cf9594fa4ee04d9 Mon Sep 17 00:00:00 2001 From: fragosti Date: Fri, 22 Jun 2018 12:04:45 -0700 Subject: Fix tslint error --- packages/website/ts/pages/about/about.tsx | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/website/ts/pages/about/about.tsx b/packages/website/ts/pages/about/about.tsx index e427104aa..0259af36f 100644 --- a/packages/website/ts/pages/about/about.tsx +++ b/packages/website/ts/pages/about/about.tsx @@ -165,16 +165,16 @@ const teamRow5: ProfileInfo[] = [ }, ]; -const teamRow6: ProfileInfo[] = [ - // { - // name: 'Chris Kalani', - // title: 'Director of Design', - // description: `Previously founded Wake (acquired by InVision). Early Facebook product designer.`, - // image: 'images/team/chris.png', - // linkedIn: 'https://www.linkedin.com/in/chriskalani/', - // github: 'https://github.com/chriskalani', - // }, -]; +// const teamRow6: ProfileInfo[] = [ +// { +// name: 'Chris Kalani', +// title: 'Director of Design', +// description: `Previously founded Wake (acquired by InVision). Early Facebook product designer.`, +// image: 'images/team/chris.png', +// linkedIn: 'https://www.linkedin.com/in/chriskalani/', +// github: 'https://github.com/chriskalani', +// }, +// ]; const advisors: ProfileInfo[] = [ { -- cgit v1.2.3 From 7fb8b546f9b20719fcc9b10d77b9e07c0cef6025 Mon Sep 17 00:00:00 2001 From: fragosti Date: Fri, 22 Jun 2018 12:22:31 -0700 Subject: Remove unecessary position relative rules --- packages/website/ts/components/portal/portal.tsx | 4 ++-- packages/website/ts/components/relayer_index/relayer_grid_tile.tsx | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index 9a725c742..c9f2b2633 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -226,7 +226,7 @@ export class Portal extends React.Component { ? TokenVisibility.UNTRACKED : TokenVisibility.TRACKED; return ( - + { const marginBottom = isMobile ? '200px' : '15px'; return (
- + {isMobile && {startOnboarding}} Date: Fri, 22 Jun 2018 14:32:21 -0700 Subject: Fix issue where hovered tiles appear over top bar --- packages/website/ts/components/portal/portal.tsx | 14 +++++++++----- packages/website/ts/style/z_index.ts | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/website/ts/components/portal/portal.tsx b/packages/website/ts/components/portal/portal.tsx index c9f2b2633..4166fde53 100644 --- a/packages/website/ts/components/portal/portal.tsx +++ b/packages/website/ts/components/portal/portal.tsx @@ -243,8 +243,7 @@ export class Portal extends React.Component { style={{ backgroundColor: colors.lightestGrey, position: 'fixed', - // Hack: used to make onboarding z-index logic work for both mobile and desktop - zIndex: utils.isMobile(this.props.screenWidth) ? zIndex.topBar : undefined, + zIndex: zIndex.topBar, }} maxWidth={LARGE_LAYOUT_MAX_WIDTH} /> @@ -696,9 +695,14 @@ interface LargeLayoutProps { } const LargeLayout = (props: LargeLayoutProps) => { return ( -
+
- + {props.left}
@@ -707,7 +711,7 @@ const LargeLayout = (props: LargeLayoutProps) => { {props.right} -
+ ); }; diff --git a/packages/website/ts/style/z_index.ts b/packages/website/ts/style/z_index.ts index 0411cdd91..a3998f59b 100644 --- a/packages/website/ts/style/z_index.ts +++ b/packages/website/ts/style/z_index.ts @@ -1,5 +1,6 @@ export const zIndex = { topBar: 1100, + aboveTopBar: 1101, overlay: 1105, aboveOverlay: 1106, }; -- cgit v1.2.3 From 66be42d1f839f99bc4d953fe4be401776573f23f Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Fri, 22 Jun 2018 15:33:58 -0700 Subject: Center items in portal drawer header --- packages/website/ts/components/portal/drawer_menu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/website/ts/components/portal/drawer_menu.tsx b/packages/website/ts/components/portal/drawer_menu.tsx index 4bd07769f..205a60afc 100644 --- a/packages/website/ts/components/portal/drawer_menu.tsx +++ b/packages/website/ts/components/portal/drawer_menu.tsx @@ -65,7 +65,7 @@ interface HeaderProps { const Header = (props: HeaderProps) => { return (
-
+
{props.displayMessage} -- cgit v1.2.3 From a1737a28d08798e4b58ff49cd116609ddafa5295 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Fri, 22 Jun 2018 15:56:24 -0700 Subject: Do not render ZRX or WETH as removable tracked tokens --- packages/website/ts/components/generate_order/asset_picker.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/website/ts/components/generate_order/asset_picker.tsx b/packages/website/ts/components/generate_order/asset_picker.tsx index 87618f1e0..b0dcf5678 100644 --- a/packages/website/ts/components/generate_order/asset_picker.tsx +++ b/packages/website/ts/components/generate_order/asset_picker.tsx @@ -9,6 +9,7 @@ import { TokenIcon } from 'ts/components/ui/token_icon'; import { trackedTokenStorage } from 'ts/local_storage/tracked_token_storage'; import { Dispatcher } from 'ts/redux/dispatcher'; import { DialogConfigs, Token, TokenByAddress, TokenVisibility } from 'ts/types'; +import { constants } from 'ts/utils/constants'; const TOKEN_ICON_DIMENSION = 100; const TILE_DIMENSION = 146; @@ -134,7 +135,9 @@ export class AssetPicker extends React.Component { if ( (this.props.tokenVisibility === TokenVisibility.TRACKED && !token.isTracked) || - (this.props.tokenVisibility === TokenVisibility.UNTRACKED && token.isTracked) + (this.props.tokenVisibility === TokenVisibility.UNTRACKED && token.isTracked) || + token.symbol === constants.ZRX_TOKEN_SYMBOL || + token.symbol === constants.ETHER_TOKEN_SYMBOL ) { return null; // Skip } -- cgit v1.2.3 From 1f5848ba8265885ef4644a8103029552782965c8 Mon Sep 17 00:00:00 2001 From: Brandon Millman Date: Fri, 22 Jun 2018 16:07:07 -0700 Subject: Remove extra border from the bottom of the wallet --- packages/website/ts/components/wallet/wallet.tsx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/website/ts/components/wallet/wallet.tsx b/packages/website/ts/components/wallet/wallet.tsx index 4cc70badd..dc48d6619 100644 --- a/packages/website/ts/components/wallet/wallet.tsx +++ b/packages/website/ts/components/wallet/wallet.tsx @@ -311,7 +311,7 @@ export class Wallet extends React.Component { wrappedEtherDirection: Side.Deposit, }; const key = ETHER_ITEM_KEY; - return this._renderBalanceRow(key, icon, primaryText, secondaryText, accessoryItemConfig, 'eth-row'); + return this._renderBalanceRow(key, icon, primaryText, secondaryText, accessoryItemConfig, false, 'eth-row'); } private _renderTokenRows(): React.ReactNode { const trackedTokens = this.props.trackedTokens; @@ -322,7 +322,7 @@ export class Wallet extends React.Component { ); return _.map(trackedTokensStartingWithEtherToken, this._renderTokenRow.bind(this)); } - private _renderTokenRow(token: Token, _index: number): React.ReactNode { + private _renderTokenRow(token: Token, index: number): React.ReactNode { const tokenState = this.props.trackedTokenStateByAddress[token.address]; if (_.isUndefined(tokenState)) { return null; @@ -350,12 +350,14 @@ export class Wallet extends React.Component { }, }; const key = token.address; + const isLastRow = index === this.props.trackedTokens.length - 1; return this._renderBalanceRow( key, icon, primaryText, secondaryText, accessoryItemConfig, + isLastRow, isWeth ? 'weth-row' : undefined, ); } @@ -365,13 +367,19 @@ export class Wallet extends React.Component { primaryText: React.ReactNode, secondaryText: React.ReactNode, accessoryItemConfig: AccessoryItemConfig, + isLastRow: boolean, className?: string, ): React.ReactNode { const shouldShowWrapEtherItem = !_.isUndefined(this.state.wrappedEtherDirection) && this.state.wrappedEtherDirection === accessoryItemConfig.wrappedEtherDirection && !_.isUndefined(this.props.userEtherBalanceInWei); - const additionalStyle = shouldShowWrapEtherItem ? walletItemStyles.focusedItem : styles.borderedItem; + let additionalStyle; + if (shouldShowWrapEtherItem) { + additionalStyle = walletItemStyles.focusedItem; + } else if (!isLastRow) { + additionalStyle = styles.borderedItem; + } const style = { ...styles.tokenItem, ...additionalStyle }; const etherToken = this._getEthToken(); return ( -- cgit v1.2.3 From 7fcd34eb3656a49eaac684ee3cb1b7d55d9e15e6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 22 Jun 2018 16:35:50 -0700 Subject: Apply mask to address to zero-out unused bytes. --- .../src/contracts/current/protocol/Exchange/MixinTransactions.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index 86f7c7b72..20a4a12df 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -66,7 +66,7 @@ contract MixinTransactions is let memPtr := mload(64) mstore(memPtr, schemaHash) mstore(add(memPtr, 32), salt) - mstore(add(memPtr, 64), signerAddress) + mstore(add(memPtr, 64), and(signerAddress, 0xffffffffffffffffffffffffffffffffffffffff)) mstore(add(memPtr, 96), dataHash) result := keccak256(memPtr, 128) } -- cgit v1.2.3 From 98840c9c5f078d42ea54585dda1645029772db11 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 14 Jun 2018 11:49:17 +0200 Subject: Use provided mem in refernce memcpy --- packages/contracts/test/libraries/lib_mem.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/test/libraries/lib_mem.ts b/packages/contracts/test/libraries/lib_mem.ts index 00f7c4d8b..8cf5cf3a7 100644 --- a/packages/contracts/test/libraries/lib_mem.ts +++ b/packages/contracts/test/libraries/lib_mem.ts @@ -30,8 +30,8 @@ describe('LibMem', () => { const memHex = toHex(memory); // Reference implementation to test against - const refMemcpy = (_mem: Uint8Array, dest: number, source: number, length: number): Uint8Array => - Uint8Array.from(memory).copyWithin(dest, source, source + length); + const refMemcpy = (mem: Uint8Array, dest: number, source: number, length: number): Uint8Array => + Uint8Array.from(mem).copyWithin(dest, source, source + length); // Test vectors: destination, source, length, job description type Tests = Array<[number, number, number, string]>; -- cgit v1.2.3 From 7f84049538af09e7596623158e1364a68bb17a35 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:36:35 +0200 Subject: Merge LibMem and LibBytes --- packages/contracts/compiler.json | 1 - packages/contracts/package.json | 2 +- .../current/test/TestLibBytes/TestLibBytes.sol | 30 ++++ .../current/test/TestLibMem/TestLibMem.sol | 56 ------ .../contracts/current/utils/LibBytes/LibBytes.sol | 125 +++++++++++++- .../src/contracts/current/utils/LibMem/LibMem.sol | 142 --------------- packages/contracts/src/utils/artifacts.ts | 2 - packages/contracts/src/utils/types.ts | 1 - packages/contracts/test/asset_proxy/decoder.ts | 1 - packages/contracts/test/libraries/lib_bytes.ts | 171 +++++++++++++++++++ packages/contracts/test/libraries/lib_mem.ts | 190 --------------------- 11 files changed, 322 insertions(+), 399 deletions(-) delete mode 100644 packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol delete mode 100644 packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol delete mode 100644 packages/contracts/test/libraries/lib_mem.ts diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index 38580f4dc..f1646e79e 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -33,7 +33,6 @@ "TestAssetDataDecoders", "TestAssetProxyDispatcher", "TestLibBytes", - "TestLibMem", "TestLibs", "TestSignatureValidator", "TestValidator", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 3bb66067d..87f88a8eb 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -34,7 +34,7 @@ }, "config": { "abis": - "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 6f1898acd..d9b246b29 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -221,4 +221,34 @@ contract TestLibBytes is writeBytes(b, index, input); return b; } + + /// @dev Copies a block of memory from one location to another. + /// @param mem Memory contents we want to apply memCopy to + /// @param dest Destination offset into . + /// @param source Source offset into . + /// @param length Length of bytes to copy from to + /// @return mem Memory contents after calling memCopy. + function testMemcpy( + bytes mem, + uint256 dest, + uint256 source, + uint256 length + ) + public // not external, we need input in memory + pure + returns (bytes) + { + // 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 memCopy adjusted for memory array location + memCopy(offset + dest, offset + source, length); + + // Return modified memory contents + return mem; + } } diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol deleted file mode 100644 index b7e2e06b8..000000000 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ /dev/null @@ -1,56 +0,0 @@ -/* - - Copyright 2018 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.4.24; - -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 memCopy to - /// @param dest Destination offset into . - /// @param source Source offset into . - /// @param length Length of bytes to copy from to - /// @return mem Memory contents after calling memCopy. - function testMemcpy( - bytes mem, - uint256 dest, - uint256 source, - uint256 length - ) - public // not external, we need input in memory - pure - returns (bytes) - { - // 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 memCopy adjusted for memory array location - memCopy(offset + dest, offset + source, length); - - // Return modified memory contents - return mem; - } -} diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 10d7ce41a..cb63e38e6 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,11 +18,7 @@ pragma solidity ^0.4.24; -import "../LibMem/LibMem.sol"; - -contract LibBytes is - LibMem -{ +contract LibBytes { // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; @@ -32,6 +28,125 @@ contract LibBytes is string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; + /// @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 memoryAddress) + { + assembly { + memoryAddress := input + } + 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. + function memCopy( + uint256 dest, + uint256 source, + uint256 length + ) + internal + pure + { + if (length < 32) { + // Handle a partial word by reading destination and masking + // off the bits we are interested in. + // This correctly handles overlap, zero lengths and source == dest + assembly { + let mask := sub(exp(256, sub(32, length)), 1) + let s := and(mload(source), not(mask)) + let d := and(mload(dest), mask) + mstore(dest, or(s, d)) + } + } else { + // Skip the O(length) loop when source == dest. + if (source == dest) { + return; + } + + // For large copies we copy whole words at a time. The final + // word is aligned to the end of the range (instead of after the + // previous) to handle partial words. So a copy will look like this: + // + // #### + // #### + // #### + // #### + // + // We handle overlap in the source and destination range by + // changing the copying direction. This prevents us from + // overwriting parts of source that we still need to copy. + // + // This correctly handles source == dest + // + if (source > dest) { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because it + // is easier to compare with in the loop, and these + // are also the addresses we need for copying the + // last bytes. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the last 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the last bytes in + // source already due to overlap. + let last := mload(sEnd) + + // Copy whole words front to back + // Note: the first check is always true, + // this could have been a do-while loop. + for {} lt(source, sEnd) {} { + mstore(dest, mload(source)) + source := add(source, 32) + dest := add(dest, 32) + } + + // Write the last 32 bytes + mstore(dEnd, last) + } + } else { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because those + // are the starting points when copying a word at the end. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the first 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the first bytes in + // source already due to overlap. + let first := mload(source) + + // Copy whole words back to front + // We use a signed comparisson here to allow dEnd to become + // negative (happens when source and dest < 32). Valid + // addresses in local memory will never be larger than + // 2**255, so they can be safely re-interpreted as signed. + // Note: the first check is always true, + // this could have been a do-while loop. + for {} slt(dest, dEnd) {} { + mstore(dEnd, mload(sEnd)) + sEnd := sub(sEnd, 32) + dEnd := sub(dEnd, 32) + } + + // Write the first 32 bytes + mstore(dest, first) + } + } + } + } + /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol deleted file mode 100644 index 97fb5fb0f..000000000 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ /dev/null @@ -1,142 +0,0 @@ -/* - - Copyright 2018 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.4.24; - -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 memoryAddress) - { - assembly { - memoryAddress := input - } - 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. - function memCopy( - uint256 dest, - uint256 source, - uint256 length - ) - internal - pure - { - if (length < 32) { - // Handle a partial word by reading destination and masking - // off the bits we are interested in. - // This correctly handles overlap, zero lengths and source == dest - assembly { - let mask := sub(exp(256, sub(32, length)), 1) - let s := and(mload(source), not(mask)) - let d := and(mload(dest), mask) - mstore(dest, or(s, d)) - } - } else { - // Skip the O(length) loop when source == dest. - if (source == dest) { - return; - } - - // For large copies we copy whole words at a time. The final - // word is aligned to the end of the range (instead of after the - // previous) to handle partial words. So a copy will look like this: - // - // #### - // #### - // #### - // #### - // - // We handle overlap in the source and destination range by - // changing the copying direction. This prevents us from - // overwriting parts of source that we still need to copy. - // - // This correctly handles source == dest - // - if (source > dest) { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because it - // is easier to compare with in the loop, and these - // are also the addresses we need for copying the - // last bytes. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the last 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the last bytes in - // source already due to overlap. - let last := mload(sEnd) - - // Copy whole words front to back - // Note: the first check is always true, - // this could have been a do-while loop. - for {} lt(source, sEnd) {} { - mstore(dest, mload(source)) - source := add(source, 32) - dest := add(dest, 32) - } - - // Write the last 32 bytes - mstore(dEnd, last) - } - } else { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because those - // are the starting points when copying a word at the end. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the first 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the first bytes in - // source already due to overlap. - let first := mload(source) - - // Copy whole words back to front - // We use a signed comparisson here to allow dEnd to become - // negative (happens when source and dest < 32). Valid - // addresses in local memory will never be larger than - // 2**255, so they can be safely re-interpreted as signed. - // Note: the first check is always true, - // this could have been a do-while loop. - for {} slt(dest, dEnd) {} { - mstore(dEnd, mload(sEnd)) - sEnd := sub(sEnd, 32) - dEnd := sub(dEnd, 32) - } - - // Write the first 32 bytes - mstore(dest, first) - } - } - } - } -} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 4375d87c6..fa18cc9d2 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -14,7 +14,6 @@ import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTime import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; -import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; import * as TestValidator from '../artifacts/TestValidator.json'; @@ -40,7 +39,6 @@ export const artifacts = { TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, - TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, TestValidator: (TestValidator as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 7a1f92afd..5dfac64fc 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -92,7 +92,6 @@ export enum ContractName { Arbitrage = 'Arbitrage', TestAssetDataDecoders = 'TestAssetDataDecoders', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', - TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts index 875d55daa..98d18aa38 100644 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ b/packages/contracts/test/asset_proxy/decoder.ts @@ -27,7 +27,6 @@ describe('TestAssetDataDecoders', () => { // Setup accounts & addresses const accounts = await web3Wrapper.getAvailableAddressesAsync(); testAddress = accounts[0]; - // Deploy TestLibMem testAssetProxyDecoder = await TestAssetDataDecodersContract.deployFrom0xArtifactAsync( artifacts.TestAssetDataDecoders, provider, diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index a31a4789c..b378bb036 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -17,6 +17,12 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +// BUG: Ideally we would use Buffer.from(memory).toString('hex') +// https://github.com/Microsoft/TypeScript/issues/23155 +const toHex = (buf: Uint8Array): string => buf.reduce((a, v) => a + ('00' + v.toString(16)).slice(-2), '0x'); + +const fromHex = (str: string): Uint8Array => Uint8Array.from(Buffer.from(str.slice(2), 'hex')); + describe('LibBytes', () => { let libBytes: TestLibBytesContract; const byteArrayShorterThan32Bytes = '0x012345'; @@ -617,5 +623,170 @@ describe('LibBytes', () => { ); }); }); + + describe('memCopy', () => { + // Create memory 0x000102...FF + const memSize = 256; + const memory = new Uint8Array(memSize).map((_, i) => i); + const memHex = toHex(memory); + + // Reference implementation to test against + const refMemcpy = (mem: Uint8Array, dest: number, source: number, length: number): Uint8Array => + Uint8Array.from(mem).copyWithin(dest, source, source + length); + + // Test vectors: destination, source, length, job description + type Tests = Array<[number, number, number, string]>; + + const test = (tests: Tests) => + tests.forEach(([dest, source, length, job]) => + it(job, async () => { + const expected = refMemcpy(memory, dest, source, length); + const resultStr = await libBytes.testMemcpy.callAsync( + memHex, + new BigNumber(dest), + new BigNumber(source), + new BigNumber(length), + ); + const result = fromHex(resultStr); + expect(result).to.deep.equal(expected); + }), + ); + + test([[0, 0, 0, 'copies zero bytes with overlap']]); + + describe('copies forward', () => + test([ + [128, 0, 0, 'zero bytes'], + [128, 0, 1, 'one byte'], + [128, 0, 11, 'eleven bytes'], + [128, 0, 31, 'thirty-one bytes'], + [128, 0, 32, 'one word'], + [128, 0, 64, 'two words'], + [128, 0, 96, 'three words'], + [128, 0, 33, 'one word and one byte'], + [128, 0, 72, 'two words and eight bytes'], + [128, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward within one word', () => + test([ + [16, 0, 0, 'zero bytes'], + [16, 0, 1, 'one byte'], + [16, 0, 11, 'eleven bytes'], + [16, 0, 16, 'sixteen bytes'], + ])); + + describe('copies forward with one byte overlap', () => + test([ + [0, 0, 1, 'one byte'], + [10, 0, 11, 'eleven bytes'], + [30, 0, 31, 'thirty-one bytes'], + [31, 0, 32, 'one word'], + [32, 0, 33, 'one word and one byte'], + [71, 0, 72, 'two words and eight bytes'], + [99, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward with thirty-one bytes overlap', () => + test([ + [0, 0, 31, 'thirty-one bytes'], + [1, 0, 32, 'one word'], + [2, 0, 33, 'one word and one byte'], + [41, 0, 72, 'two words and eight bytes'], + [69, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward with one word overlap', () => + test([ + [0, 0, 32, 'one word'], + [1, 0, 33, 'one word and one byte'], + [41, 0, 72, 'two words and eight bytes'], + [69, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward with one word and one byte overlap', () => + test([ + [0, 0, 33, 'one word and one byte'], + [40, 0, 72, 'two words and eight bytes'], + [68, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward with two words overlap', () => + test([ + [0, 0, 64, 'two words'], + [8, 0, 72, 'two words and eight bytes'], + [36, 0, 100, 'three words and four bytes'], + ])); + + describe('copies forward within one word and one byte overlap', () => + test([[0, 0, 1, 'one byte'], [10, 0, 11, 'eleven bytes'], [15, 0, 16, 'sixteen bytes']])); + + describe('copies backward', () => + test([ + [0, 128, 0, 'zero bytes'], + [0, 128, 1, 'one byte'], + [0, 128, 11, 'eleven bytes'], + [0, 128, 31, 'thirty-one bytes'], + [0, 128, 32, 'one word'], + [0, 128, 64, 'two words'], + [0, 128, 96, 'three words'], + [0, 128, 33, 'one word and one byte'], + [0, 128, 72, 'two words and eight bytes'], + [0, 128, 100, 'three words and four bytes'], + ])); + + describe('copies backward within one word', () => + test([ + [0, 16, 0, 'zero bytes'], + [0, 16, 1, 'one byte'], + [0, 16, 11, 'eleven bytes'], + [0, 16, 16, 'sixteen bytes'], + ])); + + describe('copies backward with one byte overlap', () => + test([ + [0, 0, 1, 'one byte'], + [0, 10, 11, 'eleven bytes'], + [0, 30, 31, 'thirty-one bytes'], + [0, 31, 32, 'one word'], + [0, 32, 33, 'one word and one byte'], + [0, 71, 72, 'two words and eight bytes'], + [0, 99, 100, 'three words and four bytes'], + ])); + + describe('copies backward with thirty-one bytes overlap', () => + test([ + [0, 0, 31, 'thirty-one bytes'], + [0, 1, 32, 'one word'], + [0, 2, 33, 'one word and one byte'], + [0, 41, 72, 'two words and eight bytes'], + [0, 69, 100, 'three words and four bytes'], + ])); + + describe('copies backward with one word overlap', () => + test([ + [0, 0, 32, 'one word'], + [0, 1, 33, 'one word and one byte'], + [0, 41, 72, 'two words and eight bytes'], + [0, 69, 100, 'three words and four bytes'], + ])); + + describe('copies backward with one word and one byte overlap', () => + test([ + [0, 0, 33, 'one word and one byte'], + [0, 40, 72, 'two words and eight bytes'], + [0, 68, 100, 'three words and four bytes'], + ])); + + describe('copies backward with two words overlap', () => + test([ + [0, 0, 64, 'two words'], + [0, 8, 72, 'two words and eight bytes'], + [0, 36, 100, 'three words and four bytes'], + ])); + + describe('copies forward within one word and one byte overlap', () => + test([[0, 0, 1, 'one byte'], [0, 10, 11, 'eleven bytes'], [0, 15, 16, 'sixteen bytes']])); + }); }); // tslint:disable:max-file-line-count diff --git a/packages/contracts/test/libraries/lib_mem.ts b/packages/contracts/test/libraries/lib_mem.ts deleted file mode 100644 index 8cf5cf3a7..000000000 --- a/packages/contracts/test/libraries/lib_mem.ts +++ /dev/null @@ -1,190 +0,0 @@ -import { BigNumber } from '@0xproject/utils'; -import * as chai from 'chai'; - -import { TestLibMemContract } from '../../src/generated_contract_wrappers/test_lib_mem'; -import { artifacts } from '../../src/utils/artifacts'; -import { chaiSetup } from '../../src/utils/chai_setup'; -import { provider, txDefaults } from '../../src/utils/web3_wrapper'; - -chaiSetup.configure(); -const expect = chai.expect; - -// BUG: Ideally we would use Buffer.from(memory).toString('hex') -// https://github.com/Microsoft/TypeScript/issues/23155 -const toHex = (buf: Uint8Array): string => buf.reduce((a, v) => a + ('00' + v.toString(16)).slice(-2), '0x'); - -const fromHex = (str: string): Uint8Array => Uint8Array.from(Buffer.from(str.slice(2), 'hex')); - -describe('LibMem', () => { - let testLibMem: TestLibMemContract; - - before(async () => { - // Deploy TestLibMem - testLibMem = await TestLibMemContract.deployFrom0xArtifactAsync(artifacts.TestLibMem, provider, txDefaults); - }); - - describe('memCopy', () => { - // Create memory 0x000102...FF - const memSize = 256; - const memory = new Uint8Array(memSize).map((_, i) => i); - const memHex = toHex(memory); - - // Reference implementation to test against - const refMemcpy = (mem: Uint8Array, dest: number, source: number, length: number): Uint8Array => - Uint8Array.from(mem).copyWithin(dest, source, source + length); - - // Test vectors: destination, source, length, job description - type Tests = Array<[number, number, number, string]>; - - const test = (tests: Tests) => - tests.forEach(([dest, source, length, job]) => - it(job, async () => { - const expected = refMemcpy(memory, dest, source, length); - const resultStr = await testLibMem.testMemcpy.callAsync( - memHex, - new BigNumber(dest), - new BigNumber(source), - new BigNumber(length), - ); - const result = fromHex(resultStr); - expect(result).to.deep.equal(expected); - }), - ); - - test([[0, 0, 0, 'copies zero bytes with overlap']]); - - describe('copies forward', () => - test([ - [128, 0, 0, 'zero bytes'], - [128, 0, 1, 'one byte'], - [128, 0, 11, 'eleven bytes'], - [128, 0, 31, 'thirty-one bytes'], - [128, 0, 32, 'one word'], - [128, 0, 64, 'two words'], - [128, 0, 96, 'three words'], - [128, 0, 33, 'one word and one byte'], - [128, 0, 72, 'two words and eight bytes'], - [128, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward within one word', () => - test([ - [16, 0, 0, 'zero bytes'], - [16, 0, 1, 'one byte'], - [16, 0, 11, 'eleven bytes'], - [16, 0, 16, 'sixteen bytes'], - ])); - - describe('copies forward with one byte overlap', () => - test([ - [0, 0, 1, 'one byte'], - [10, 0, 11, 'eleven bytes'], - [30, 0, 31, 'thirty-one bytes'], - [31, 0, 32, 'one word'], - [32, 0, 33, 'one word and one byte'], - [71, 0, 72, 'two words and eight bytes'], - [99, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward with thirty-one bytes overlap', () => - test([ - [0, 0, 31, 'thirty-one bytes'], - [1, 0, 32, 'one word'], - [2, 0, 33, 'one word and one byte'], - [41, 0, 72, 'two words and eight bytes'], - [69, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward with one word overlap', () => - test([ - [0, 0, 32, 'one word'], - [1, 0, 33, 'one word and one byte'], - [41, 0, 72, 'two words and eight bytes'], - [69, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward with one word and one byte overlap', () => - test([ - [0, 0, 33, 'one word and one byte'], - [40, 0, 72, 'two words and eight bytes'], - [68, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward with two words overlap', () => - test([ - [0, 0, 64, 'two words'], - [8, 0, 72, 'two words and eight bytes'], - [36, 0, 100, 'three words and four bytes'], - ])); - - describe('copies forward within one word and one byte overlap', () => - test([[0, 0, 1, 'one byte'], [10, 0, 11, 'eleven bytes'], [15, 0, 16, 'sixteen bytes']])); - - describe('copies backward', () => - test([ - [0, 128, 0, 'zero bytes'], - [0, 128, 1, 'one byte'], - [0, 128, 11, 'eleven bytes'], - [0, 128, 31, 'thirty-one bytes'], - [0, 128, 32, 'one word'], - [0, 128, 64, 'two words'], - [0, 128, 96, 'three words'], - [0, 128, 33, 'one word and one byte'], - [0, 128, 72, 'two words and eight bytes'], - [0, 128, 100, 'three words and four bytes'], - ])); - - describe('copies backward within one word', () => - test([ - [0, 16, 0, 'zero bytes'], - [0, 16, 1, 'one byte'], - [0, 16, 11, 'eleven bytes'], - [0, 16, 16, 'sixteen bytes'], - ])); - - describe('copies backward with one byte overlap', () => - test([ - [0, 0, 1, 'one byte'], - [0, 10, 11, 'eleven bytes'], - [0, 30, 31, 'thirty-one bytes'], - [0, 31, 32, 'one word'], - [0, 32, 33, 'one word and one byte'], - [0, 71, 72, 'two words and eight bytes'], - [0, 99, 100, 'three words and four bytes'], - ])); - - describe('copies backward with thirty-one bytes overlap', () => - test([ - [0, 0, 31, 'thirty-one bytes'], - [0, 1, 32, 'one word'], - [0, 2, 33, 'one word and one byte'], - [0, 41, 72, 'two words and eight bytes'], - [0, 69, 100, 'three words and four bytes'], - ])); - - describe('copies backward with one word overlap', () => - test([ - [0, 0, 32, 'one word'], - [0, 1, 33, 'one word and one byte'], - [0, 41, 72, 'two words and eight bytes'], - [0, 69, 100, 'three words and four bytes'], - ])); - - describe('copies backward with one word and one byte overlap', () => - test([ - [0, 0, 33, 'one word and one byte'], - [0, 40, 72, 'two words and eight bytes'], - [0, 68, 100, 'three words and four bytes'], - ])); - - describe('copies backward with two words overlap', () => - test([ - [0, 0, 64, 'two words'], - [0, 8, 72, 'two words and eight bytes'], - [0, 36, 100, 'three words and four bytes'], - ])); - - describe('copies forward within one word and one byte overlap', () => - test([[0, 0, 1, 'one byte'], [0, 10, 11, 'eleven bytes'], [0, 15, 16, 'sixteen bytes']])); - }); -}); -- cgit v1.2.3 From afd83e59b8bf2b755e589562ab12b300aaf12601 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:53:22 +0200 Subject: Make LibBytes a library --- .../protocol/AssetProxy/MixinERC20Transfer.sol | 3 +- .../protocol/AssetProxy/MixinERC721Transfer.sol | 7 ++--- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 3 +- .../protocol/Exchange/MixinSignatureValidator.sol | 18 ++++++------ .../current/test/TestLibBytes/TestLibBytes.sol | 32 ++++++++++------------ .../contracts/current/utils/LibBytes/LibBytes.sol | 2 +- 6 files changed, 30 insertions(+), 35 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol index 4af39a00b..34da13fcc 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol @@ -24,7 +24,6 @@ import "../../tokens/ERC20Token/IERC20Token.sol"; import "./libs/LibTransferErrors.sol"; contract MixinERC20Transfer is - LibBytes, LibTransferErrors { /// @dev Internal version of `transferFrom`. @@ -41,7 +40,7 @@ contract MixinERC20Transfer is internal { // Decode asset data. - address token = readAddress(assetData, 0); + address token = LibBytes.readAddress(assetData, 0); // Transfer tokens. // We do a raw call so we can check the success separate diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index 6e3156e8a..028f4ed8a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -24,7 +24,6 @@ import "../../tokens/ERC721Token/ERC721Token.sol"; import "./libs/LibTransferErrors.sol"; contract MixinERC721Transfer is - LibBytes, LibTransferErrors { /// @dev Internal version of `transferFrom`. @@ -78,10 +77,10 @@ contract MixinERC721Transfer is ) { // Decode asset data. - token = readAddress(assetData, 0); - tokenId = readUint256(assetData, 20); + token = LibBytes.readAddress(assetData, 0); + tokenId = LibBytes.readUint256(assetData, 20); if (assetData.length > 52) { - receiverData = readBytes(assetData, 52); + receiverData = LibBytes.readBytes(assetData, 52); } return ( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 7f5f056b5..4d5e5f786 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -22,7 +22,6 @@ import "../../multisig/MultiSigWalletWithTimeLock.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is - LibBytes, MultiSigWalletWithTimeLock { @@ -104,7 +103,7 @@ contract AssetProxyOwner is pure returns (bool) { - bytes4 first4Bytes = readFirst4(data); + bytes4 first4Bytes = LibBytes.readFirst4(data); require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); return true; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 881d6e7b3..4a6bb5de5 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -26,7 +26,6 @@ import "./interfaces/IWallet.sol"; import "./interfaces/IValidator.sol"; contract MixinSignatureValidator is - LibBytes, LibExchangeErrors, MSignatureValidator, MTransactions @@ -102,7 +101,7 @@ contract MixinSignatureValidator is ); // Ensure signature is supported - uint8 signatureTypeRaw = uint8(popLastByte(signature)); + uint8 signatureTypeRaw = uint8(LibBytes.popLastByte(signature)); require( signatureTypeRaw < uint8(SignatureType.NSignatureTypes), SIGNATURE_UNSUPPORTED @@ -144,8 +143,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = LibBytes.readBytes32(signature, 1); + s = LibBytes.readBytes32(signature, 33); recovered = ecrecover(hash, v, r, s); isValid = signerAddress == recovered; return isValid; @@ -157,8 +156,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = LibBytes.readBytes32(signature, 1); + s = LibBytes.readBytes32(signature, 33); recovered = ecrecover( keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)), v, @@ -199,7 +198,8 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validatorAddress = popLast20Bytes(signature); + address validatorAddress = LibBytes.popLast20Bytes(signature); + // Ensure signer has approved validator. if (!allowedValidators[signerAddress][validatorAddress]) { return false; @@ -230,8 +230,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = LibBytes.readBytes32(signature, 1); + s = LibBytes.readBytes32(signature, 33); recovered = ecrecover( keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)), v, diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index d9b246b29..fc9d1e5c4 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -21,9 +21,7 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -contract TestLibBytes is - LibBytes -{ +contract TestLibBytes { /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -33,7 +31,7 @@ contract TestLibBytes is pure returns (bytes memory, bytes1 result) { - result = popLastByte(b); + result = LibBytes.popLastByte(b); return (b, result); } @@ -45,7 +43,7 @@ contract TestLibBytes is pure returns (bytes memory, address result) { - result = popLast20Bytes(b); + result = LibBytes.popLast20Bytes(b); return (b, result); } @@ -58,7 +56,7 @@ contract TestLibBytes is pure returns (bool equal) { - equal = areBytesEqual(lhs, rhs); + equal = LibBytes.areBytesEqual(lhs, rhs); return equal; } @@ -89,7 +87,7 @@ contract TestLibBytes is pure returns (address result) { - result = readAddress(b, index); + result = LibBytes.readAddress(b, index); return result; } @@ -106,7 +104,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeAddress(b, index, input); + LibBytes.writeAddress(b, index, input); return b; } @@ -122,7 +120,7 @@ contract TestLibBytes is pure returns (bytes32 result) { - result = readBytes32(b, index); + result = LibBytes.readBytes32(b, index); return result; } @@ -139,7 +137,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes32(b, index, input); + LibBytes.writeBytes32(b, index, input); return b; } @@ -155,7 +153,7 @@ contract TestLibBytes is pure returns (uint256 result) { - result = readUint256(b, index); + result = LibBytes.readUint256(b, index); return result; } @@ -172,7 +170,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeUint256(b, index, input); + LibBytes.writeUint256(b, index, input); return b; } @@ -184,7 +182,7 @@ contract TestLibBytes is pure returns (bytes4 result) { - result = readFirst4(b); + result = LibBytes.readFirst4(b); return result; } @@ -200,7 +198,7 @@ contract TestLibBytes is pure returns (bytes memory result) { - result = readBytes(b, index); + result = LibBytes.readBytes(b, index); return result; } @@ -218,7 +216,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes(b, index, input); + LibBytes.writeBytes(b, index, input); return b; } @@ -243,10 +241,10 @@ contract TestLibBytes is require(dest + length <= mem.length); // Get pointer to memory contents - uint256 offset = getMemAddress(mem) + 32; + uint256 offset = LibBytes.getMemAddress(mem) + 32; // Execute memCopy adjusted for memory array location - memCopy(offset + dest, offset + source, length); + LibBytes.memCopy(offset + dest, offset + source, length); // Return modified memory contents return mem; diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index cb63e38e6..05cf53df5 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,7 +18,7 @@ pragma solidity ^0.4.24; -contract LibBytes { +library LibBytes { // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; -- cgit v1.2.3 From 2ea0b839d3f704ea85c992912b86db0cb49f80ea Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 12:02:32 +0200 Subject: Using LibBytes for bytes --- .../protocol/AssetProxy/MixinERC20Transfer.sol | 4 +++- .../protocol/AssetProxy/MixinERC721Transfer.sol | 8 ++++--- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 3 ++- .../protocol/Exchange/MixinSignatureValidator.sol | 19 ++++++++------- .../protocol/Exchange/MixinWrapperFunctions.sol | 2 ++ .../current/test/TestLibBytes/TestLibBytes.sol | 28 ++++++++++++---------- 6 files changed, 38 insertions(+), 26 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol index 34da13fcc..0e7f3fc89 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol @@ -26,6 +26,8 @@ import "./libs/LibTransferErrors.sol"; contract MixinERC20Transfer is LibTransferErrors { + using LibBytes for bytes; + /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -40,7 +42,7 @@ contract MixinERC20Transfer is internal { // Decode asset data. - address token = LibBytes.readAddress(assetData, 0); + address token = assetData.readAddress(0); // Transfer tokens. // We do a raw call so we can check the success separate diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index 028f4ed8a..2df9725b4 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -26,6 +26,8 @@ import "./libs/LibTransferErrors.sol"; contract MixinERC721Transfer is LibTransferErrors { + using LibBytes for bytes; + /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -77,10 +79,10 @@ contract MixinERC721Transfer is ) { // Decode asset data. - token = LibBytes.readAddress(assetData, 0); - tokenId = LibBytes.readUint256(assetData, 20); + token = assetData.readAddress(0); + tokenId = assetData.readUint256(20); if (assetData.length > 52) { - receiverData = LibBytes.readBytes(assetData, 52); + receiverData = assetData.readBytes(52); } return ( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 4d5e5f786..261ce8f34 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -24,6 +24,7 @@ import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is MultiSigWalletWithTimeLock { + using LibBytes for bytes; event AssetProxyRegistration(address assetProxyContract, bool isRegistered); @@ -103,7 +104,7 @@ contract AssetProxyOwner is pure returns (bool) { - bytes4 first4Bytes = LibBytes.readFirst4(data); + bytes4 first4Bytes = data.readFirst4(); require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); return true; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 4a6bb5de5..cbb55bfce 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -30,6 +30,8 @@ contract MixinSignatureValidator is MSignatureValidator, MTransactions { + using LibBytes for bytes; + // Personal message headers string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32"; string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20"; @@ -101,7 +103,7 @@ contract MixinSignatureValidator is ); // Ensure signature is supported - uint8 signatureTypeRaw = uint8(LibBytes.popLastByte(signature)); + uint8 signatureTypeRaw = uint8(signature.popLastByte()); require( signatureTypeRaw < uint8(SignatureType.NSignatureTypes), SIGNATURE_UNSUPPORTED @@ -143,8 +145,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = LibBytes.readBytes32(signature, 1); - s = LibBytes.readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover(hash, v, r, s); isValid = signerAddress == recovered; return isValid; @@ -156,8 +158,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = LibBytes.readBytes32(signature, 1); - s = LibBytes.readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)), v, @@ -198,7 +200,8 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validatorAddress = LibBytes.popLast20Bytes(signature); + + address validatorAddress = signature.popLast20Bytes(); // Ensure signer has approved validator. if (!allowedValidators[signerAddress][validatorAddress]) { @@ -230,8 +233,8 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = LibBytes.readBytes32(signature, 1); - s = LibBytes.readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)), v, diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 724f95518..87375d4bf 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -31,6 +31,8 @@ contract MixinWrapperFunctions is LibExchangeErrors, MExchangeCore { + using LibBytes for bytes; + /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @param order Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index fc9d1e5c4..96020906c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -22,6 +22,8 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; contract TestLibBytes { + + using LibBytes for bytes; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -31,7 +33,7 @@ contract TestLibBytes { pure returns (bytes memory, bytes1 result) { - result = LibBytes.popLastByte(b); + result = b.popLastByte(); return (b, result); } @@ -43,7 +45,7 @@ contract TestLibBytes { pure returns (bytes memory, address result) { - result = LibBytes.popLast20Bytes(b); + result = b.popLast20Bytes(); return (b, result); } @@ -56,7 +58,7 @@ contract TestLibBytes { pure returns (bool equal) { - equal = LibBytes.areBytesEqual(lhs, rhs); + equal = lhs.areBytesEqual(rhs); return equal; } @@ -87,7 +89,7 @@ contract TestLibBytes { pure returns (address result) { - result = LibBytes.readAddress(b, index); + result = b.readAddress(index); return result; } @@ -104,7 +106,7 @@ contract TestLibBytes { pure returns (bytes memory) { - LibBytes.writeAddress(b, index, input); + b.writeAddress(index, input); return b; } @@ -120,7 +122,7 @@ contract TestLibBytes { pure returns (bytes32 result) { - result = LibBytes.readBytes32(b, index); + result = b.readBytes32(index); return result; } @@ -137,7 +139,7 @@ contract TestLibBytes { pure returns (bytes memory) { - LibBytes.writeBytes32(b, index, input); + b.writeBytes32(index, input); return b; } @@ -153,7 +155,7 @@ contract TestLibBytes { pure returns (uint256 result) { - result = LibBytes.readUint256(b, index); + result = b.readUint256(index); return result; } @@ -170,7 +172,7 @@ contract TestLibBytes { pure returns (bytes memory) { - LibBytes.writeUint256(b, index, input); + b.writeUint256(index, input); return b; } @@ -182,7 +184,7 @@ contract TestLibBytes { pure returns (bytes4 result) { - result = LibBytes.readFirst4(b); + result = b.readFirst4(); return result; } @@ -198,7 +200,7 @@ contract TestLibBytes { pure returns (bytes memory result) { - result = LibBytes.readBytes(b, index); + result = b.readBytes(index); return result; } @@ -216,7 +218,7 @@ contract TestLibBytes { pure returns (bytes memory) { - LibBytes.writeBytes(b, index, input); + b.writeBytes(index, input); return b; } @@ -241,7 +243,7 @@ contract TestLibBytes { require(dest + length <= mem.length); // Get pointer to memory contents - uint256 offset = LibBytes.getMemAddress(mem) + 32; + uint256 offset = mem.getMemAddress() + 32; // Execute memCopy adjusted for memory array location LibBytes.memCopy(offset + dest, offset + source, length); -- cgit v1.2.3 From 88982f98ff2ece93831b1442d2c4e1ef0878d919 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 17:13:59 +0200 Subject: Rename read/writeBytesWithLength --- .../protocol/AssetProxy/MixinERC721Transfer.sol | 2 +- .../current/test/TestLibBytes/TestLibBytes.sol | 8 +-- .../contracts/current/utils/LibBytes/LibBytes.sol | 4 +- packages/contracts/test/libraries/lib_bytes.ts | 58 +++++++++++----------- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index 2df9725b4..944068bbb 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -82,7 +82,7 @@ contract MixinERC721Transfer is token = assetData.readAddress(0); tokenId = assetData.readUint256(20); if (assetData.length > 52) { - receiverData = assetData.readBytes(52); + receiverData = assetData.readBytesWithLength(52); } return ( diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 96020906c..84b9e4673 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -192,7 +192,7 @@ contract TestLibBytes { /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function publicReadBytes( + function publicReadBytesWithLength( bytes memory b, uint256 index ) @@ -200,7 +200,7 @@ contract TestLibBytes { pure returns (bytes memory result) { - result = b.readBytes(index); + result = b.readBytesWithLength(index); return result; } @@ -209,7 +209,7 @@ contract TestLibBytes { /// @param index Index in byte array of . /// @param input bytes to insert. /// @return b Updated input byte array - function publicWriteBytes( + function publicWriteBytesWithLength( bytes memory b, uint256 index, bytes memory input @@ -218,7 +218,7 @@ contract TestLibBytes { pure returns (bytes memory) { - b.writeBytes(index, input); + b.writeBytesWithLength(index, input); return b; } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 05cf53df5..ad3a35aef 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -371,7 +371,7 @@ library LibBytes { /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function readBytes( + function readBytesWithLength( bytes memory b, uint256 index ) @@ -405,7 +405,7 @@ library LibBytes { /// @param b Byte array to insert into. /// @param index Index in byte array of . /// @param input bytes to insert. - function writeBytes( + function writeBytesWithLength( bytes memory b, uint256 index, bytes memory input diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index b378bb036..3afa2bab4 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -465,10 +465,10 @@ describe('LibBytes', () => { }); }); - describe('readBytes', () => { + describe('readBytesWithLength', () => { it('should successfully read short, nested array of bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const bytes = await libBytes.publicReadBytes.callAsync(shortTestBytes, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(shortTestBytes, testBytesOffset); return expect(bytes).to.be.equal(shortData); }); it('should successfully read short, nested array of bytes when it is offset in the array', async () => { @@ -476,12 +476,12 @@ describe('LibBytes', () => { const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, shortTestBytesAsBuffer]); const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); - const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(shortData); }); it('should successfully read a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const bytes = await libBytes.publicReadBytes.callAsync(wordOfTestBytes, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(wordOfTestBytes, testBytesOffset); return expect(bytes).to.be.equal(wordOfData); }); it('should successfully read a nested array of bytes - one word in length - when it is offset in the array', async () => { @@ -489,12 +489,12 @@ describe('LibBytes', () => { const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, wordOfTestBytesAsBuffer]); const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); - const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(wordOfData); }); it('should successfully read long, nested array of bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); - const bytes = await libBytes.publicReadBytes.callAsync(longTestBytes, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(longTestBytes, testBytesOffset); return expect(bytes).to.be.equal(longData); }); it('should successfully read long, nested array of bytes when it is offset in the array', async () => { @@ -502,46 +502,46 @@ describe('LibBytes', () => { const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, longTestBytesAsBuffer]); const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); - const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(longData); }); it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { // The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read. const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( - libBytes.publicReadBytes.callAsync(byteArrayShorterThan32Bytes, offset), + libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); it('should fail if we store a nested byte array length, without a nested byte array', async () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( - libBytes.publicReadBytes.callAsync(testBytes32, offset), + libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength); return expectRevertOrOtherErrorAsync( - libBytes.publicReadBytes.callAsync(byteArrayShorterThan32Bytes, badOffset), + libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); return expectRevertOrOtherErrorAsync( - libBytes.publicReadBytes.callAsync(testBytes32, badOffset), + libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); }); - describe('writeBytes', () => { + describe('writeBytesWithLength', () => { it('should successfully write short, nested array of bytes when it takes up the whole array)', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, testBytesOffset, shortData); - const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, shortData); + const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(shortData); }); it('should successfully write short, nested array of bytes when it is offset in the array', async () => { @@ -552,19 +552,19 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + shortTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); - bytesWritten = await libBytes.publicWriteBytes.callAsync(bytesWritten, testBytesOffset, shortData); + bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, shortData); // Read data after prefix and validate - const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(shortData); }); it('should successfully write a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(wordOfTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, testBytesOffset, wordOfData); - const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, wordOfData); + const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(wordOfData); }); it('should successfully write a nested array of bytes - one word in length - when it is offset in the array', async () => { @@ -575,19 +575,19 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + wordOfTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); - bytesWritten = await libBytes.publicWriteBytes.callAsync(bytesWritten, testBytesOffset, wordOfData); + bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, wordOfData); // Read data after prefix and validate - const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(wordOfData); }); it('should successfully write a long, nested bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(longTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, testBytesOffset, longData); - const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, longData); + const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(longData); }); it('should successfully write long, nested array of bytes when it is offset in the array', async () => { @@ -598,19 +598,19 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + longTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytes.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); - bytesWritten = await libBytes.publicWriteBytes.callAsync(bytesWritten, testBytesOffset, longData); + bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, longData); // Read data after prefix and validate - const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); + const bytes = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(longData); }); it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { const offset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(1)); return expectRevertOrOtherErrorAsync( - libBytes.publicWriteBytes.callAsync(emptyByteArray, offset, longData), + libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, ); }); @@ -618,7 +618,7 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); return expectRevertOrOtherErrorAsync( - libBytes.publicWriteBytes.callAsync(emptyByteArray, badOffset, shortData), + libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, ); }); -- cgit v1.2.3 From 2054cd78da9c39e5572cd76bb9fff7a4cd581903 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 17:20:18 +0200 Subject: Rename bytes.rawAddress and add bytes.contentAddress --- .../current/test/TestLibBytes/TestLibBytes.sol | 2 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 31 +++++++++++++++++----- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 84b9e4673..299452c96 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -243,7 +243,7 @@ contract TestLibBytes { require(dest + length <= mem.length); // Get pointer to memory contents - uint256 offset = mem.getMemAddress() + 32; + uint256 offset = mem.contentAddress(); // Execute memCopy adjusted for memory array location LibBytes.memCopy(offset + dest, offset + source, length); diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index ad3a35aef..d824449a2 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -19,6 +19,7 @@ pragma solidity ^0.4.24; library LibBytes { + using LibBytes for bytes; // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; @@ -30,8 +31,10 @@ library LibBytes { /// @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) + /// @return memoryAddress Memory address of byte array. This + /// points to the header of the byte array which contains + /// the length. + function rawAddress(bytes memory input) internal pure returns (uint256 memoryAddress) @@ -41,6 +44,20 @@ library LibBytes { } return memoryAddress; } + + /// @dev Gets the memory address for the contents of a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of the contents of the byte array. + function contentAddress(bytes memory input) + internal + pure + returns (uint256 memoryAddress) + { + assembly { + memoryAddress := add(input, 32) + } + return memoryAddress; + } /// @dev Copies `length` bytes from memory location `source` to `dest`. /// @param dest memory address to copy bytes to. @@ -393,8 +410,8 @@ library LibBytes { // Allocate memory and copy value to result result = new bytes(nestedBytesLength); memCopy( - getMemAddress(result) + 32, // +32 skips array length - getMemAddress(b) + index + 32, + result.contentAddress(), + b.contentAddress() + index, nestedBytesLength ); @@ -422,9 +439,9 @@ library LibBytes { // Copy into memCopy( - getMemAddress(b) + 32 + index, // +32 to skip length of - getMemAddress(input), // includes length of - input.length + 32 // +32 bytes to store length + b.contentAddress() + index, + input.rawAddress(), // includes length of + input.length + 32 // +32 bytes to store length ); } -- cgit v1.2.3 From c83ee04662177407f1d484175d8a76dddf3c4e85 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 17:34:35 +0200 Subject: Add slice and sliceDestructive --- .../contracts/current/utils/LibBytes/LibBytes.sol | 46 ++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index d824449a2..58ce31c70 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -28,6 +28,8 @@ library LibBytes { string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; + string constant FROM_LESS_THAN_TO_REQUIRED = "FROM_LESS_THAN_TO_REQUIRED"; + string constant TO_LESS_THAN_LENGTH_REQUIRED = "TO_LESS_THAN_LENGTH_REQUIRED"; /// @dev Gets the memory address for a byte array. /// @param input Byte array to lookup. @@ -163,6 +165,50 @@ library LibBytes { } } } + + /// @dev Returns a slices from a byte array. + /// @param b The byte array to take a slice from. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + function slice(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require(from <= to, FROM_LESS_THAN_TO_REQUIRED); + require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED); + + // Create a new bytes structure and copy contents + result = new bytes(to - from); + memCopy( + result.contentAddress(), + b.contentAddress() + from, + result.length); + return result; + } + + /// @dev Returns a slices from a byte array without preserving the input. + /// @param b The byte array to take a slice from. Will be destroyed in the process. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + /// @dev When `from == 0`, the original array will match the slice. In other cases it's state will be corrupted. + function sliceDestructive(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require(from <= to, FROM_LESS_THAN_TO_REQUIRED); + require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED); + + // Create a new bytes structure around [from, to) in-place. + assembly { + result := add(b, from) + mstore(result, sub(to, from)) + } + return result; + } /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. -- cgit v1.2.3 From 425af46f98c83e2796aaf0b946d250193393754c Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 17:46:46 +0200 Subject: Rename bytes.equals --- .../current/test/TestLibBytes/TestLibBytes.sol | 4 +-- .../contracts/current/utils/LibBytes/LibBytes.sol | 35 ++++++++++++++++++++++ packages/contracts/test/libraries/lib_bytes.ts | 26 ++++++++-------- 3 files changed, 50 insertions(+), 15 deletions(-) diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 299452c96..60ee3db7f 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -53,12 +53,12 @@ contract TestLibBytes { /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. /// @return True if arrays are the same. False otherwise. - function publicAreBytesEqual(bytes memory lhs, bytes memory rhs) + function publicEquals(bytes memory lhs, bytes memory rhs) public pure returns (bool equal) { - equal = lhs.areBytesEqual(rhs); + equal = lhs.equals(rhs); return equal; } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 58ce31c70..2862ee3dd 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -258,6 +258,41 @@ library LibBytes { return result; } + /// @dev Tests equality of two byte arrays. + /// @param lhs First byte array to compare. + /// @param rhs Second byte array to compare. + /// @return True if arrays are the same. False otherwise. + function equals( + bytes memory lhs, + bytes memory rhs + ) + internal + pure + returns (bool equal) + { + assembly { + // Get the number of words occupied by + let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) + + // Add 1 to the number of words, to account for the length field + lenFullWords := add(lenFullWords, 0x1) + + // Test equality word-by-word. + // Terminates early if there is a mismatch. + for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { + let lhsWord := mload(add(lhs, mul(i, 0x20))) + let rhsWord := mload(add(rhs, mul(i, 0x20))) + equal := eq(lhsWord, rhsWord) + if eq(equal, 0) { + // Break + i := lenFullWords + } + } + } + + return equal; + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 3afa2bab4..0c8431a6a 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -129,48 +129,48 @@ describe('LibBytes', () => { }); }); - describe('areBytesEqual', () => { + describe('equals', () => { it('should return true if byte arrays are equal (both arrays < 32 bytes)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayShorterThan32Bytes, byteArrayShorterThan32Bytes, ); - return expect(areBytesEqual).to.be.true(); + return expect(equals).to.be.true(); }); it('should return true if byte arrays are equal (both arrays > 32 bytes)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayLongerThan32Bytes, byteArrayLongerThan32Bytes, ); - return expect(areBytesEqual).to.be.true(); + return expect(equals).to.be.true(); }); it('should return false if byte arrays are not equal (first array < 32 bytes, second array > 32 bytes)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes, ); - return expect(areBytesEqual).to.be.false(); + return expect(equals).to.be.false(); }); it('should return false if byte arrays are not equal (first array > 32 bytes, second array < 32 bytes)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayLongerThan32Bytes, byteArrayShorterThan32Bytes, ); - return expect(areBytesEqual).to.be.false(); + return expect(equals).to.be.false(); }); it('should return false if byte arrays are not equal (same length, but a byte in first word differs)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayLongerThan32BytesFirstBytesSwapped, byteArrayLongerThan32Bytes, ); - return expect(areBytesEqual).to.be.false(); + return expect(equals).to.be.false(); }); it('should return false if byte arrays are not equal (same length, but a byte in last word differs)', async () => { - const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( + const equals = await libBytes.publicEquals.callAsync( byteArrayLongerThan32BytesLastBytesSwapped, byteArrayLongerThan32Bytes, ); - return expect(areBytesEqual).to.be.false(); + return expect(equals).to.be.false(); }); }); -- cgit v1.2.3 From 384cd2f6059bba994a6af2992a7df4909e24897c Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 18:12:45 +0200 Subject: Add trailing garbage testcase for LibBytes.equals --- .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol | 11 +++++++++++ packages/contracts/test/libraries/lib_bytes.ts | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 60ee3db7f..d14022a64 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -61,6 +61,17 @@ contract TestLibBytes { equal = lhs.equals(rhs); return equal; } + + function publicEqualsPop1(bytes memory lhs, bytes memory rhs) + public + pure + returns (bool equal) + { + lhs.popByte(); + rhs.popByte(); + equal = lhs.equals(rhs); + return equal; + } /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. /// @param dest Byte array that will be overwritten with source bytes. diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 0c8431a6a..5edb5835a 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -172,6 +172,16 @@ describe('LibBytes', () => { ); return expect(equals).to.be.false(); }); + + describe('should ignore trailing data', () => { + it('should return true when both < 32 bytes', async () => { + const equals = await libBytes.publicEqualsPop1.callAsync( + '0x0102', + '0x0103', + ); + return expect(equals).to.be.true(); + }); + }); }); describe('deepCopyBytes', () => { -- cgit v1.2.3 From 4bf4f96f478ba8e08bf2d44817294ddd5fe895c5 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 18:13:17 +0200 Subject: Fix LibBytes.equals --- .../contracts/current/utils/LibBytes/LibBytes.sol | 25 ++++------------------ 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 2862ee3dd..3149a47e5 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -270,27 +270,10 @@ library LibBytes { pure returns (bool equal) { - assembly { - // Get the number of words occupied by - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; + // Keccak gas cost is 30 + numWords * 6. This is a cheap way to compare. + // We early exit on unequal lengths, but keccak would also correctly + // handle this. + return lhs.length == rhs.length && keccak256(lhs) == keccak256(rhs); } /// @dev Reads an address from a position in a byte array. -- cgit v1.2.3 From 943e556f43a328dbb606567b06392851ba4ec3de Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 18:28:11 +0200 Subject: Refactor LibBytes.readBytes4 for consistency --- .../current/protocol/AssetProxyOwner/AssetProxyOwner.sol | 2 +- .../contracts/current/test/TestLibBytes/TestLibBytes.sol | 14 +++++++++----- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 13 ++++++++----- packages/contracts/test/libraries/lib_bytes.ts | 10 +++++++--- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 261ce8f34..50c538cea 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -104,7 +104,7 @@ contract AssetProxyOwner is pure returns (bool) { - bytes4 first4Bytes = data.readFirst4(); + bytes4 first4Bytes = data.readBytes4(0); require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); return true; } diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index d14022a64..5421045a0 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -187,15 +187,19 @@ contract TestLibBytes { 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. - function publicReadFirst4(bytes memory b) + /// @dev Reads an unpadded bytes4 value from a position in a byte array. + /// @param b Byte array containing a bytes4 value. + /// @param index Index in byte array of bytes4 value. + /// @return bytes4 value from byte array. + function publicReadBytes4( + bytes memory b, + uint256 index + ) public pure returns (bytes4 result) { - result = b.readFirst4(); + result = b.readBytes4(index); return result; } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 3149a47e5..cbe4f3be1 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -430,16 +430,19 @@ library LibBytes { 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. - function readFirst4(bytes memory b) + /// @dev Reads an unpadded bytes4 value from a position in a byte array. + /// @param b Byte array containing a bytes4 value. + /// @param index Index in byte array of bytes4 value. + /// @return bytes4 value from byte array. + function readBytes4( + bytes memory b, + uint256 index) internal pure returns (bytes4 result) { require( - b.length >= 4, + b.length >= index + 4, GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED ); assembly { diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 5edb5835a..8dc968ed7 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -459,17 +459,21 @@ describe('LibBytes', () => { }); }); - describe('readFirst4', () => { + describe('readBytes4', () => { // AssertionError: expected promise to be rejected with an error including 'revert' but it was fulfilled with '0x08c379a0' it('should revert if byte array has a length < 4', async () => { const byteArrayLessThan4Bytes = '0x010101'; return expectRevertOrOtherErrorAsync( - libBytes.publicReadFirst4.callAsync(byteArrayLessThan4Bytes), + libBytes.publicReadBytes4.callAsync( + byteArrayLessThan4Bytes, + new BigNumber(0)), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED, ); }); it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const first4Bytes = await libBytes.publicReadFirst4.callAsync(byteArrayLongerThan32Bytes); + const first4Bytes = await libBytes.publicReadBytes4.callAsync( + byteArrayLongerThan32Bytes, + new BigNumber(0)); const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); expect(first4Bytes).to.equal(expectedFirst4Bytes); }); -- cgit v1.2.3 From 5c612a186f6b6282323bc7172de22f43c511b8e4 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 18:36:56 +0200 Subject: Clean high bits in address --- packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index cbe4f3be1..c443dda8f 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -341,6 +341,10 @@ library LibBytes { // 2. Load 32-byte word from memory // 3. Apply 12-byte mask to obtain extra bytes occupying word of memory where we'll store the address let neighbors := and(mload(add(b, index)), 0xffffffffffffffffffffffff0000000000000000000000000000000000000000) + + // Make sure input address is clean. + // (Solidity does not guarantee this) + input := and(input, 0xffffffffffffffffffffffffffffffffffffffff) // Store the neighbors and address into memory mstore(add(b, index), xor(input, neighbors)) -- cgit v1.2.3 From c66477c69097b15b90d9474222e308baeaa929a6 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 18:37:12 +0200 Subject: Clean low bits in bytes4 --- packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index c443dda8f..4a1397470 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -451,6 +451,9 @@ library LibBytes { ); assembly { result := mload(add(b, 32)) + // Solidity does not require us to clean the trailing bytes. + // We do it anyway + result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000) } return result; } -- cgit v1.2.3 From 20a07494f669e4d161db4906f1a104afb89fbbbf Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:36:35 +0200 Subject: Fix usage of `contentAddress()` --- packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 4a1397470..a5ef3b5aa 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -571,8 +571,8 @@ library LibBytes { GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED ); memCopy( - getMemAddress(dest) + 32, // +32 to skip length of - getMemAddress(source) + 32, // +32 to skip length of + dest.contentAddress(), + source.contentAddress(), sourceLen ); } -- cgit v1.2.3 From ba1baafca591fca8c1f6d3677021cf452fc5efc7 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:36:35 +0200 Subject: Remove `areBytesEqual` --- .../contracts/current/utils/LibBytes/LibBytes.sol | 35 ---------------------- 1 file changed, 35 deletions(-) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index a5ef3b5aa..81cd99cff 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -519,41 +519,6 @@ library LibBytes { ); } - /// @dev Tests equality of two byte arrays. - /// @param lhs First byte array to compare. - /// @param rhs Second byte array to compare. - /// @return True if arrays are the same. False otherwise. - function areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; - } - /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. /// @param dest Byte array that will be overwritten with source bytes. /// @param source Byte array to copy onto dest bytes. -- cgit v1.2.3 From 2f8ceca2ef40a53579336850e728012d18f346a6 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:36:35 +0200 Subject: Fix LibBytes is a library --- .../current/protocol/Exchange/MixinAssetProxyDispatcher.sol | 2 -- .../src/contracts/current/protocol/Exchange/MixinExchangeCore.sol | 7 ++++--- .../src/contracts/current/protocol/Exchange/MixinMatchOrders.sol | 6 +++--- .../contracts/current/protocol/Exchange/MixinWrapperFunctions.sol | 2 -- .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol | 2 +- .../src/contracts/current/test/TestWallet/TestWallet.sol | 8 ++++---- 6 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index f85019012..b8d6c0722 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,14 +19,12 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, - LibBytes, LibExchangeErrors, MAssetProxyDispatcher { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c227d210f..b207b3e57 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -32,7 +32,6 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is LibConstants, - LibBytes, LibMath, LibOrder, LibFillResults, @@ -42,6 +41,8 @@ contract MixinExchangeCore is MSignatureValidator, MTransactions { + using LibBytes for bytes; + // Mapping of orderHash => amount of takerAsset already bought by maker mapping (bytes32 => uint256) public filled; @@ -411,8 +412,8 @@ contract MixinExchangeCore is ) private { - uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); + uint8 makerAssetProxyId = uint8(order.makerAssetData.popLastByte()); + uint8 takerAssetProxyId = uint8(order.takerAssetData.popLastByte()); bytes memory zrxAssetData = ZRX_ASSET_DATA; dispatchTransferFrom( order.makerAssetData, diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 82325a29f..e36fcc2c5 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -27,7 +27,6 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is LibConstants, - LibBytes, LibMath, LibExchangeErrors, MAssetProxyDispatcher, @@ -35,6 +34,7 @@ contract MixinMatchOrders is MMatchOrders, MTransactions { + using LibBytes for bytes; /// @dev Match two complementary orders that have a profitable spread. /// Each order is filled at their respective price point. However, the calculations are @@ -242,8 +242,8 @@ contract MixinMatchOrders is ) private { - uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); + uint8 leftMakerAssetProxyId = uint8(leftOrder.makerAssetData.popLastByte()); + uint8 rightMakerAssetProxyId = uint8(rightOrder.makerAssetData.popLastByte()); bytes memory zrxAssetData = ZRX_ASSET_DATA; // Order makers and taker dispatchTransferFrom( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 87375d4bf..724f95518 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -31,8 +31,6 @@ contract MixinWrapperFunctions is LibExchangeErrors, MExchangeCore { - using LibBytes for bytes; - /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. /// @param order Order struct containing order specifications. /// @param takerAssetFillAmount Desired amount of takerAsset to sell. diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 5421045a0..b2823aa88 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -84,7 +84,7 @@ contract TestLibBytes { pure returns (bytes memory) { - deepCopyBytes(dest, source); + LibBytes.deepCopyBytes(dest, source); return dest; } diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol index aca74b409..17dee9e9c 100644 --- a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -22,9 +22,9 @@ import "../../protocol/Exchange/interfaces/IWallet.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract TestWallet is - IWallet, - LibBytes + IWallet { + using LibBytes for bytes; string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; @@ -56,8 +56,8 @@ contract TestWallet is ); uint8 v = uint8(eip712Signature[0]); - bytes32 r = readBytes32(eip712Signature, 1); - bytes32 s = readBytes32(eip712Signature, 33); + bytes32 r = eip712Signature.readBytes32(1); + bytes32 s = eip712Signature.readBytes32(33); address recoveredAddress = ecrecover(hash, v, r, s); isValid = WALLET_OWNER == recoveredAddress; return isValid; -- cgit v1.2.3 From 19ba272d62a3d838bb024e5152a2eb88f33645cd Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 11:36:35 +0200 Subject: Fix usage of `popLastByte` --- .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index b2823aa88..f45faaf36 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -67,8 +67,8 @@ contract TestLibBytes { pure returns (bool equal) { - lhs.popByte(); - rhs.popByte(); + lhs.popLastByte(); + rhs.popLastByte(); equal = lhs.equals(rhs); return equal; } -- cgit v1.2.3 From 6a6f98299de5fd67f9162cc57c73583702924a2f Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Wed, 13 Jun 2018 20:21:01 +0200 Subject: Move isFunctionRemoveAuthorizedAddress to test --- packages/contracts/compiler.json | 1 + packages/contracts/package.json | 2 +- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 15 +------ .../TestAssetProxyOwner/TestAssetProxyOwner.sol | 47 ++++++++++++++++++++++ packages/contracts/src/utils/artifacts.ts | 2 + packages/contracts/test/asset_proxy_owner.ts | 25 ++++++++---- 6 files changed, 70 insertions(+), 22 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json index f1646e79e..6b7b69caa 100644 --- a/packages/contracts/compiler.json +++ b/packages/contracts/compiler.json @@ -30,6 +30,7 @@ "MixinAuthorizable", "MultiSigWallet", "MultiSigWalletWithTimeLock", + "TestAssetProxyOwner", "TestAssetDataDecoders", "TestAssetProxyDispatcher", "TestLibBytes", diff --git a/packages/contracts/package.json b/packages/contracts/package.json index 87f88a8eb..8249b9e0c 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -34,7 +34,7 @@ }, "config": { "abis": - "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 50c538cea..79f64312f 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -39,7 +39,7 @@ contract AssetProxyOwner is modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); - require(isFunctionRemoveAuthorizedAddress(tx.data)); + require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_SELECTOR); _; } @@ -95,17 +95,4 @@ contract AssetProxyOwner is tx.executed = false; } } - - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. - /// @param data Transaction data. - /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes memory data) - public - pure - returns (bool) - { - bytes4 first4Bytes = data.readBytes4(0); - require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); - return true; - } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol new file mode 100644 index 000000000..35746d2ad --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -0,0 +1,47 @@ +/* + + Copyright 2018 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.4.24; + +import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol"; + +contract TestAssetProxyOwner is + AssetProxyOwner +{ + constructor( + address[] memory _owners, + address[] memory _assetProxyContracts, + uint256 _required, + uint256 _secondsTimeLocked + ) + public + AssetProxyOwner(_owners, _assetProxyContracts, _required, _secondsTimeLocked) + { + } + + /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. + /// @param data Transaction data. + /// @return Successful if data is a call to removeAuthorizedAddress. + function isFunctionRemoveAuthorizedAddress(bytes memory data) + public + pure + returns (bool) + { + return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_SELECTOR; + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index fa18cc9d2..a46bab9ae 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -13,6 +13,7 @@ import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; +import * as TestAssetProxyOwner from '../artifacts/TestAssetProxyOwner.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; @@ -36,6 +37,7 @@ export const artifacts = { MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact, MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, + TestAssetProxyOwner: (TestAssetProxyOwner as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index aa4999e95..a46882828 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -10,11 +10,11 @@ import { ExecutionFailureContractEventArgs, SubmissionContractEventArgs, } from '../src/generated_contract_wrappers/asset_proxy_owner'; -import { MixinAuthorizableContract } from '../src/generated_contract_wrappers/mixin_authorizable'; +import { MixinAuthorizableContract } from '../src/generated_contract_wrappers/mixin_authorizable'; +import { TestAssetProxyOwnerContract} from '../src/generated_contract_wrappers/test_asset_proxy_owner'; import { artifacts } from '../src/utils/artifacts'; import { expectRevertOrAlwaysFailingTransactionAsync, - expectRevertOrContractCallFailedAsync, } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; @@ -35,6 +35,7 @@ describe('AssetProxyOwner', () => { let erc20Proxy: MixinAuthorizableContract; let erc721Proxy: MixinAuthorizableContract; let multiSig: AssetProxyOwnerContract; + let testAssetProxyOwner: TestAssetProxyOwnerContract; let multiSigWrapper: MultiSigWrapper; before(async () => { @@ -68,6 +69,15 @@ describe('AssetProxyOwner', () => { SECONDS_TIME_LOCKED, ); multiSigWrapper = new MultiSigWrapper(multiSig, provider); + testAssetProxyOwner = await TestAssetProxyOwnerContract.deployFrom0xArtifactAsync( + artifacts.TestAssetProxyOwner, + provider, + txDefaults, + owners, + defaultAssetProxyContractAddresses, + REQUIRED_APPROVALS, + SECONDS_TIME_LOCKED, + ); await web3Wrapper.awaitTransactionSuccessAsync( await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }), constants.AWAIT_TRANSACTION_MINED_MS, @@ -118,23 +128,24 @@ describe('AssetProxyOwner', () => { }); describe('isFunctionRemoveAuthorizedAddress', () => { - it('should throw if data is not for removeAuthorizedAddress', async () => { + it('should return false if data is not for removeAuthorizedAddress', async () => { const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( owners[0], ); - return expectRevertOrContractCallFailedAsync( - multiSig.isFunctionRemoveAuthorizedAddress.callAsync(notRemoveAuthorizedAddressData), + const result = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddress.callAsync( + notRemoveAuthorizedAddressData, ); + expect(result).to.be.false(); }); it('should return true if data is for removeAuthorizedAddress', async () => { const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( owners[0], ); - const isFunctionRemoveAuthorizedAddress = await multiSig.isFunctionRemoveAuthorizedAddress.callAsync( + const result = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddress.callAsync( removeAuthorizedAddressData, ); - expect(isFunctionRemoveAuthorizedAddress).to.be.true(); + expect(result).to.be.true(); }); }); -- cgit v1.2.3 From 1681361aedb2c7c4d97f8b74890ee2f86d96df45 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 21 Jun 2018 14:49:16 -0700 Subject: Change removeAuthorizedAddress => removeAuthorizedAddressAtIndex --- .../protocol/AssetProxyOwner/AssetProxyOwner.sol | 18 +++++++++--------- .../test/TestAssetProxyOwner/TestAssetProxyOwner.sol | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 79f64312f..eb58b3374 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -29,17 +29,17 @@ contract AssetProxyOwner is event AssetProxyRegistration(address assetProxyContract, bool isRegistered); // Mapping of AssetProxy contract address => - // if this contract is allowed to call the AssetProxy's removeAuthorizedAddress method without a time lock. + // if this contract is allowed to call the AssetProxy's `removeAuthorizedAddressAtIndex` method without a time lock. mapping (address => bool) public isAssetProxyRegistered; - bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); + bytes4 constant REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR = bytes4(keccak256("removeAuthorizedAddressAtIndex(address,uint256)")); - /// @dev Function will revert if the transaction does not call `removeAuthorizedAddress` + /// @dev Function will revert if the transaction does not call `removeAuthorizedAddressAtIndex` /// on an approved AssetProxy contract. - modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { + modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); - require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_SELECTOR); + require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); _; } @@ -66,7 +66,7 @@ contract AssetProxyOwner is } /// @dev Registers or deregisters an AssetProxy to be able to execute - /// removeAuthorizedAddress without a timelock. + /// `removeAuthorizedAddressAtIndex` without a timelock. /// @param assetProxyContract Address of AssetProxy contract. /// @param isRegistered Status of approval for AssetProxy contract. function registerAssetProxy(address assetProxyContract, bool isRegistered) @@ -78,13 +78,13 @@ contract AssetProxyOwner is AssetProxyRegistration(assetProxyContract, isRegistered); } - /// @dev Allows execution of removeAuthorizedAddress without time lock. + /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock. /// @param transactionId Transaction ID. - function executeRemoveAuthorizedAddress(uint256 transactionId) + function executeRemoveAuthorizedAddressAtIndex(uint256 transactionId) public notExecuted(transactionId) fullyConfirmed(transactionId) - validRemoveAuthorizedAddressTx(transactionId) + validRemoveAuthorizedAddressAtIndexTx(transactionId) { Transaction storage tx = transactions[transactionId]; tx.executed = true; diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol index 35746d2ad..40a6255cb 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -34,14 +34,23 @@ contract TestAssetProxyOwner is { } - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. + function testValidRemoveAuthorizedAddressTx(uint256 id) + public + validRemoveAuthorizedAddressAtIndexTx(id) + returns (bool) + { + // Do nothing. We expect reverts through the modifier + return true; + } + + /// @dev Compares first 4 bytes of byte array to `removeAuthorizedAddressAtIndex` function selector. /// @param data Transaction data. - /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes memory data) + /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`. + function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data) public pure returns (bool) { - return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_SELECTOR; + return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; } } -- cgit v1.2.3 From 8ddcb6c841fd97bca2cddffd49a5b773cf53635f Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 21 Jun 2018 16:25:42 -0700 Subject: Update and add tests --- .../TestAssetProxyOwner/TestAssetProxyOwner.sol | 2 +- packages/contracts/src/utils/multi_sig_wrapper.ts | 6 +- packages/contracts/test/asset_proxy_owner.ts | 351 +++++++++++++-------- packages/contracts/test/libraries/lib_bytes.ts | 88 ++++-- 4 files changed, 274 insertions(+), 173 deletions(-) diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol index 40a6255cb..2abcd17a0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -34,7 +34,7 @@ contract TestAssetProxyOwner is { } - function testValidRemoveAuthorizedAddressTx(uint256 id) + function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id) public validRemoveAuthorizedAddressAtIndexTx(id) returns (bool) diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index f0098bd5e..b0d4fa8ab 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -40,13 +40,15 @@ export class MultiSigWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } - public async executeRemoveAuthorizedAddressAsync( + public async executeRemoveAuthorizedAddressAtIndexAsync( txId: BigNumber, from: string, ): Promise { // tslint:disable-next-line:no-unnecessary-type-assertion const txHash = await (this - ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); + ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from, + }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } diff --git a/packages/contracts/test/asset_proxy_owner.ts b/packages/contracts/test/asset_proxy_owner.ts index a46882828..188cba5a3 100644 --- a/packages/contracts/test/asset_proxy_owner.ts +++ b/packages/contracts/test/asset_proxy_owner.ts @@ -10,11 +10,12 @@ import { ExecutionFailureContractEventArgs, SubmissionContractEventArgs, } from '../src/generated_contract_wrappers/asset_proxy_owner'; -import { MixinAuthorizableContract } from '../src/generated_contract_wrappers/mixin_authorizable'; -import { TestAssetProxyOwnerContract} from '../src/generated_contract_wrappers/test_asset_proxy_owner'; +import { MixinAuthorizableContract } from '../src/generated_contract_wrappers/mixin_authorizable'; +import { TestAssetProxyOwnerContract } from '../src/generated_contract_wrappers/test_asset_proxy_owner'; import { artifacts } from '../src/utils/artifacts'; import { expectRevertOrAlwaysFailingTransactionAsync, + expectRevertOrContractCallFailedAsync, } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; @@ -34,7 +35,6 @@ describe('AssetProxyOwner', () => { let erc20Proxy: MixinAuthorizableContract; let erc721Proxy: MixinAuthorizableContract; - let multiSig: AssetProxyOwnerContract; let testAssetProxyOwner: TestAssetProxyOwnerContract; let multiSigWrapper: MultiSigWrapper; @@ -59,16 +59,6 @@ describe('AssetProxyOwner', () => { txDefaults, ); const defaultAssetProxyContractAddresses: string[] = []; - multiSig = await AssetProxyOwnerContract.deployFrom0xArtifactAsync( - artifacts.AssetProxyOwner, - provider, - txDefaults, - owners, - defaultAssetProxyContractAddresses, - REQUIRED_APPROVALS, - SECONDS_TIME_LOCKED, - ); - multiSigWrapper = new MultiSigWrapper(multiSig, provider); testAssetProxyOwner = await TestAssetProxyOwnerContract.deployFrom0xArtifactAsync( artifacts.TestAssetProxyOwner, provider, @@ -78,12 +68,17 @@ describe('AssetProxyOwner', () => { REQUIRED_APPROVALS, SECONDS_TIME_LOCKED, ); + multiSigWrapper = new MultiSigWrapper(testAssetProxyOwner, provider); await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }), + await erc20Proxy.transferOwnership.sendTransactionAsync(testAssetProxyOwner.address, { + from: initialOwner, + }), constants.AWAIT_TRANSACTION_MINED_MS, ); await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Proxy.transferOwnership.sendTransactionAsync(multiSig.address, { from: initialOwner }), + await erc721Proxy.transferOwnership.sendTransactionAsync(testAssetProxyOwner.address, { + from: initialOwner, + }), constants.AWAIT_TRANSACTION_MINED_MS, ); }); @@ -127,25 +122,28 @@ describe('AssetProxyOwner', () => { }); }); - describe('isFunctionRemoveAuthorizedAddress', () => { - it('should return false if data is not for removeAuthorizedAddress', async () => { + describe('isFunctionRemoveAuthorizedAddressAtIndex', () => { + it('should return false if data is not for removeAuthorizedAddressAtIndex', async () => { const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( owners[0], ); - const result = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddress.callAsync( + + const isFunctionRemoveAuthorizedAddressAtIndex = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddressAtIndex.callAsync( notRemoveAuthorizedAddressData, ); - expect(result).to.be.false(); + expect(isFunctionRemoveAuthorizedAddressAtIndex).to.be.false(); }); - it('should return true if data is for removeAuthorizedAddress', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( + it('should return true if data is for removeAuthorizedAddressAtIndex', async () => { + const index = new BigNumber(0); + const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( owners[0], + index, ); - const result = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddress.callAsync( - removeAuthorizedAddressData, + const isFunctionRemoveAuthorizedAddressAtIndex = await testAssetProxyOwner.isFunctionRemoveAuthorizedAddressAtIndex.callAsync( + removeAuthorizedAddressAtIndexData, ); - expect(result).to.be.true(); + expect(isFunctionRemoveAuthorizedAddressAtIndex).to.be.true(); }); }); @@ -153,19 +151,21 @@ describe('AssetProxyOwner', () => { it('should throw if not called by multisig', async () => { const isRegistered = true; return expectRevertOrAlwaysFailingTransactionAsync( - multiSig.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { from: owners[0] }), + testAssetProxyOwner.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, isRegistered, { + from: owners[0], + }), ); }); it('should register an address if called by multisig after timelock', async () => { const addressToRegister = erc20Proxy.address; const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + const registerAssetProxyData = testAssetProxyOwner.registerAssetProxy.getABIEncodedTransactionData( addressToRegister, isRegistered, ); const submitTxRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, + testAssetProxyOwner.address, registerAssetProxyData, owners[0], ); @@ -181,19 +181,21 @@ describe('AssetProxyOwner', () => { expect(registerLog.args.assetProxyContract).to.equal(addressToRegister); expect(registerLog.args.isRegistered).to.equal(isRegistered); - const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + const isAssetProxyRegistered = await testAssetProxyOwner.isAssetProxyRegistered.callAsync( + addressToRegister, + ); expect(isAssetProxyRegistered).to.equal(isRegistered); }); it('should fail if registering a null address', async () => { const addressToRegister = constants.NULL_ADDRESS; const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + const registerAssetProxyData = testAssetProxyOwner.registerAssetProxy.getABIEncodedTransactionData( addressToRegister, isRegistered, ); const submitTxRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, + testAssetProxyOwner.address, registerAssetProxyData, owners[0], ); @@ -207,22 +209,26 @@ describe('AssetProxyOwner', () => { const failureLog = executeTxRes.logs[0] as LogWithDecodedArgs; expect(failureLog.args.transactionId).to.be.bignumber.equal(txId); - const isAssetProxyRegistered = await multiSig.isAssetProxyRegistered.callAsync(addressToRegister); + const isAssetProxyRegistered = await testAssetProxyOwner.isAssetProxyRegistered.callAsync( + addressToRegister, + ); expect(isAssetProxyRegistered).to.equal(false); }); }); - describe('executeRemoveAuthorizedAddress', () => { + describe('Calling removeAuthorizedAddressAtIndex', () => { + const erc20Index = new BigNumber(0); + const erc721Index = new BigNumber(1); before('authorize both proxies and register erc20 proxy', async () => { // Only register ERC20 proxy const addressToRegister = erc20Proxy.address; const isRegistered = true; - const registerAssetProxyData = multiSig.registerAssetProxy.getABIEncodedTransactionData( + const registerAssetProxyData = testAssetProxyOwner.registerAssetProxy.getABIEncodedTransactionData( addressToRegister, isRegistered, ); const registerAssetProxySubmitRes = await multiSigWrapper.submitTransactionAsync( - multiSig.address, + testAssetProxyOwner.address, registerAssetProxyData, owners[0], ); @@ -259,113 +265,180 @@ describe('AssetProxyOwner', () => { await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]); }); - it('should throw without the required confirmations', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - return expectRevertOrAlwaysFailingTransactionAsync( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ); - }); - - it('should throw if tx destination is not registered', async () => { - const removeAuthorizedAddressData = erc721Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc721Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - return expectRevertOrAlwaysFailingTransactionAsync( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ); + describe('validRemoveAuthorizedAddressAtIndexTx', () => { + it('should revert if data is not for removeAuthorizedAddressAtIndex and proxy is registered', async () => { + const notRemoveAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + authorized, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + notRemoveAuthorizedAddressData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + return expectRevertOrContractCallFailedAsync( + testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + ); + }); + + it('should return true if data is for removeAuthorizedAddressAtIndex and proxy is registered', async () => { + const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc20Index, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + const isValidRemoveAuthorizedAddressAtIndexTx = await testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync( + txId, + ); + expect(isValidRemoveAuthorizedAddressAtIndexTx).to.be.true(); + }); + + it('should revert if data is for removeAuthorizedAddressAtIndex and proxy is not registered', async () => { + const removeAuthorizedAddressAtIndexData = erc721Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc721Index, + ); + const submitTxRes = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const log = submitTxRes.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + return expectRevertOrContractCallFailedAsync( + testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId), + ); + }); }); - it('should throw if tx data is not for removeAuthorizedAddress', async () => { - const newAuthorized = owners[1]; - const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( - newAuthorized, - ); - const res = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - addAuthorizedAddressData, - owners[0], - ); - const log = res.logs[0] as LogWithDecodedArgs; - const txId = log.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - return expectRevertOrAlwaysFailingTransactionAsync( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ); - }); - - it('should execute removeAuthorizedAddress for registered address if fully confirmed', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const submitRes = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const submitLog = submitRes.logs[0] as LogWithDecodedArgs; - const txId = submitLog.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); - const execLog = execRes.logs[0] as LogWithDecodedArgs; - expect(execLog.args.transactionId).to.be.bignumber.equal(txId); - - const tx = await multiSig.transactions.callAsync(txId); - const isExecuted = tx[3]; - expect(isExecuted).to.equal(true); - - const isAuthorized = await erc20Proxy.authorized.callAsync(authorized); - expect(isAuthorized).to.equal(false); - }); - - it('should throw if already executed', async () => { - const removeAuthorizedAddressData = erc20Proxy.removeAuthorizedAddress.getABIEncodedTransactionData( - authorized, - ); - const submitRes = await multiSigWrapper.submitTransactionAsync( - erc20Proxy.address, - removeAuthorizedAddressData, - owners[0], - ); - const submitLog = submitRes.logs[0] as LogWithDecodedArgs; - const txId = submitLog.args.transactionId; - - await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); - - const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAsync(txId, owners[0]); - const execLog = execRes.logs[0] as LogWithDecodedArgs; - expect(execLog.args.transactionId).to.be.bignumber.equal(txId); - - const tx = await multiSig.transactions.callAsync(txId); - const isExecuted = tx[3]; - expect(isExecuted).to.equal(true); - - return expectRevertOrAlwaysFailingTransactionAsync( - multiSig.executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from: owners[1] }), - ); + describe('executeRemoveAuthorizedAddressAtIndex', () => { + it('should throw without the required confirmations', async () => { + const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc20Index, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + return expectRevertOrAlwaysFailingTransactionAsync( + testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from: owners[1], + }), + ); + }); + + it('should throw if tx destination is not registered', async () => { + const removeAuthorizedAddressAtIndexData = erc721Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc721Index, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc721Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + return expectRevertOrAlwaysFailingTransactionAsync( + testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from: owners[1], + }), + ); + }); + + it('should throw if tx data is not for removeAuthorizedAddressAtIndex', async () => { + const newAuthorized = owners[1]; + const addAuthorizedAddressData = erc20Proxy.addAuthorizedAddress.getABIEncodedTransactionData( + newAuthorized, + ); + const res = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + addAuthorizedAddressData, + owners[0], + ); + const log = res.logs[0] as LogWithDecodedArgs; + const txId = log.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + return expectRevertOrAlwaysFailingTransactionAsync( + testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from: owners[1], + }), + ); + }); + + it('should execute removeAuthorizedAddressAtIndex for registered address if fully confirmed', async () => { + const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc20Index, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAtIndexAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + + const tx = await testAssetProxyOwner.transactions.callAsync(txId); + const isExecuted = tx[3]; + expect(isExecuted).to.equal(true); + + const isAuthorized = await erc20Proxy.authorized.callAsync(authorized); + expect(isAuthorized).to.equal(false); + }); + + it('should throw if already executed', async () => { + const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData( + authorized, + erc20Index, + ); + const submitRes = await multiSigWrapper.submitTransactionAsync( + erc20Proxy.address, + removeAuthorizedAddressAtIndexData, + owners[0], + ); + const submitLog = submitRes.logs[0] as LogWithDecodedArgs; + const txId = submitLog.args.transactionId; + + await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); + + const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAtIndexAsync(txId, owners[0]); + const execLog = execRes.logs[0] as LogWithDecodedArgs; + expect(execLog.args.transactionId).to.be.bignumber.equal(txId); + + const tx = await testAssetProxyOwner.transactions.callAsync(txId); + const isExecuted = tx[3]; + expect(isExecuted).to.equal(true); + + return expectRevertOrAlwaysFailingTransactionAsync( + testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from: owners[1], + }), + ); + }); }); }); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 8dc968ed7..56f1dc2bc 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -131,55 +131,52 @@ describe('LibBytes', () => { describe('equals', () => { it('should return true if byte arrays are equal (both arrays < 32 bytes)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayShorterThan32Bytes, byteArrayShorterThan32Bytes, ); - return expect(equals).to.be.true(); + return expect(isEqual).to.be.true(); }); it('should return true if byte arrays are equal (both arrays > 32 bytes)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayLongerThan32Bytes, byteArrayLongerThan32Bytes, ); - return expect(equals).to.be.true(); + return expect(isEqual).to.be.true(); }); it('should return false if byte arrays are not equal (first array < 32 bytes, second array > 32 bytes)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes, ); - return expect(equals).to.be.false(); + return expect(isEqual).to.be.false(); }); it('should return false if byte arrays are not equal (first array > 32 bytes, second array < 32 bytes)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayLongerThan32Bytes, byteArrayShorterThan32Bytes, ); - return expect(equals).to.be.false(); + return expect(isEqual).to.be.false(); }); it('should return false if byte arrays are not equal (same length, but a byte in first word differs)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayLongerThan32BytesFirstBytesSwapped, byteArrayLongerThan32Bytes, ); - return expect(equals).to.be.false(); + return expect(isEqual).to.be.false(); }); it('should return false if byte arrays are not equal (same length, but a byte in last word differs)', async () => { - const equals = await libBytes.publicEquals.callAsync( + const isEqual = await libBytes.publicEquals.callAsync( byteArrayLongerThan32BytesLastBytesSwapped, byteArrayLongerThan32Bytes, ); - return expect(equals).to.be.false(); + return expect(isEqual).to.be.false(); }); describe('should ignore trailing data', () => { it('should return true when both < 32 bytes', async () => { - const equals = await libBytes.publicEqualsPop1.callAsync( - '0x0102', - '0x0103', - ); - return expect(equals).to.be.true(); + const isEqual = await libBytes.publicEqualsPop1.callAsync('0x0102', '0x0103'); + return expect(isEqual).to.be.true(); }); }); }); @@ -464,16 +461,12 @@ describe('LibBytes', () => { it('should revert if byte array has a length < 4', async () => { const byteArrayLessThan4Bytes = '0x010101'; return expectRevertOrOtherErrorAsync( - libBytes.publicReadBytes4.callAsync( - byteArrayLessThan4Bytes, - new BigNumber(0)), + libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED, ); }); it('should return the first 4 bytes of a byte array of arbitrary length', async () => { - const first4Bytes = await libBytes.publicReadBytes4.callAsync( - byteArrayLongerThan32Bytes, - new BigNumber(0)); + const first4Bytes = await libBytes.publicReadBytes4.callAsync(byteArrayLongerThan32Bytes, new BigNumber(0)); const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10); expect(first4Bytes).to.equal(expectedFirst4Bytes); }); @@ -554,7 +547,11 @@ describe('LibBytes', () => { it('should successfully write short, nested array of bytes when it takes up the whole array)', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, shortData); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + testBytesOffset, + shortData, + ); const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(shortData); }); @@ -566,10 +563,18 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + shortTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + prefixOffset, + prefixData, + ); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); - bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, shortData); + bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + bytesWritten, + testBytesOffset, + shortData, + ); // Read data after prefix and validate const bytes = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(shortData); @@ -577,7 +582,11 @@ describe('LibBytes', () => { it('should successfully write a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(wordOfTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, wordOfData); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + testBytesOffset, + wordOfData, + ); const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(wordOfData); }); @@ -589,10 +598,18 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + wordOfTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + prefixOffset, + prefixData, + ); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); - bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, wordOfData); + bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + bytesWritten, + testBytesOffset, + wordOfData, + ); // Read data after prefix and validate const bytes = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(wordOfData); @@ -600,7 +617,11 @@ describe('LibBytes', () => { it('should successfully write a long, nested bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(longTestBytesAsBuffer.byteLength)); - const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, testBytesOffset, longData); + const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + testBytesOffset, + longData, + ); const bytesRead = await libBytes.publicReadBytesWithLength.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(longData); }); @@ -612,7 +633,11 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex( new Buffer(prefixDataAsBuffer.byteLength + longTestBytesAsBuffer.byteLength), ); - let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, prefixOffset, prefixData); + let bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync( + emptyByteArray, + prefixOffset, + prefixData, + ); // Write data after prefix const testBytesOffset = new BigNumber(prefixDataAsBuffer.byteLength); bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(bytesWritten, testBytesOffset, longData); @@ -641,6 +666,7 @@ describe('LibBytes', () => { describe('memCopy', () => { // Create memory 0x000102...FF const memSize = 256; + // tslint:disable:no-shadowed-variable const memory = new Uint8Array(memSize).map((_, i) => i); const memHex = toHex(memory); -- cgit v1.2.3 From ea8c2b8d6923f37412cefcb8dc8482e009dce104 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 22 Jun 2018 17:17:55 -0700 Subject: Add modifier and tests for removeAuthorizedAddressAtIndex --- .../protocol/AssetProxy/MixinAuthorizable.sol | 7 ++- .../contracts/current/utils/Ownable/Ownable.sol | 5 +- .../contracts/test/asset_proxy/authorizable.ts | 69 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol index 8cb4254c5..37c12f861 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol @@ -69,7 +69,7 @@ contract MixinAuthorizable is ); delete authorized[target]; - for (uint i = 0; i < authorities.length; i++) { + for (uint256 i = 0; i < authorities.length; i++) { if (authorities[i] == target) { authorities[i] = authorities[authorities.length - 1]; authorities.length -= 1; @@ -87,7 +87,12 @@ contract MixinAuthorizable is uint256 index ) external + onlyOwner { + require( + authorized[target], + TARGET_NOT_AUTHORIZED + ); require( index < authorities.length, INDEX_OUT_OF_BOUNDS diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol index 296c6c856..99b93d0d2 100644 --- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol +++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol @@ -13,6 +13,9 @@ import "./IOwnable.sol"; contract Ownable is IOwnable { address public owner; + // Revert reasons + string constant ONLY_CONTRACT_OWNER = "ONLY_CONTRACT_OWNER"; + constructor () public { @@ -22,7 +25,7 @@ contract Ownable is IOwnable { modifier onlyOwner() { require( msg.sender == owner, - "Only contract owner is allowed to call this method." + ONLY_CONTRACT_OWNER ); _; } diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index ed582b63e..c35dc7882 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -1,4 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable'; @@ -102,6 +103,74 @@ describe('Authorizable', () => { }); }); + describe('removeAuthorizedAddressAtIndex', () => { + it('should throw if not called by owner', async () => { + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const index = new BigNumber(0); + return expectRevertOrAlwaysFailingTransactionAsync( + authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { + from: notOwner, + }), + ); + }); + it('should throw if index is >= authorities.length', async () => { + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const index = new BigNumber(1); + return expectRevertOrAlwaysFailingTransactionAsync( + authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { + from: owner, + }), + ); + }); + it('should throw if owner attempts to remove an address that is not authorized', async () => { + const index = new BigNumber(0); + return expectRevertOrAlwaysFailingTransactionAsync( + authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { + from: owner, + }), + ); + }); + it('should throw if address at index does not match target', async () => { + const address1 = address; + const address2 = notOwner; + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.addAuthorizedAddress.sendTransactionAsync(address1, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.addAuthorizedAddress.sendTransactionAsync(address2, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const address1Index = new BigNumber(0); + return expectRevertOrAlwaysFailingTransactionAsync( + authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, { + from: owner, + }), + ); + }); + it('should allow owner to remove an authorized address', async () => { + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const index = new BigNumber(0); + await web3Wrapper.awaitTransactionSuccessAsync( + await authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isAuthorized = await authorizable.authorized.callAsync(address); + expect(isAuthorized).to.be.false(); + }); + }); + describe('getAuthorizedAddresses', () => { it('should return all authorized addresses', async () => { const initial = await authorizable.getAuthorizedAddresses.callAsync(); -- cgit v1.2.3 From 82af1df3c3d62ab0a0dd4687bf9bb65d3b261054 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Sat, 23 Jun 2018 13:55:02 +0200 Subject: Fix typos in comments --- packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 81cd99cff..e4cbf318b 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -188,12 +188,12 @@ library LibBytes { return result; } - /// @dev Returns a slices from a byte array without preserving the input. + /// @dev Returns a slice from a byte array without preserving the input. /// @param b The byte array to take a slice from. Will be destroyed in the process. /// @param from The starting index for the slice (inclusive). /// @param to The final index for the slice (exclusive). /// @return result The slice containing bytes at indices [from, to) - /// @dev When `from == 0`, the original array will match the slice. In other cases it's state will be corrupted. + /// @dev When `from == 0`, the original array will match the slice. In other cases its state will be corrupted. function sliceDestructive(bytes memory b, uint256 from, uint256 to) internal pure -- cgit v1.2.3