From 2cf74a7a963e97ea0c440e87286f1d61cbb67eab Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Tue, 27 Aug 2019 23:30:52 -0400 Subject: [PATCH] `@0x/utils`: Make `RevertError.decode()` return a `RawRevertError` if the selector is unknown. --- packages/utils/CHANGELOG.json | 4 ++ packages/utils/src/index.ts | 1 + packages/utils/src/revert_error.ts | 54 ++++++++++++++++++------ packages/utils/test/revert_error_test.ts | 27 +++++++++--- 4 files changed, 68 insertions(+), 18 deletions(-) diff --git a/packages/utils/CHANGELOG.json b/packages/utils/CHANGELOG.json index 1c38b77fa6..dca499b7fd 100644 --- a/packages/utils/CHANGELOG.json +++ b/packages/utils/CHANGELOG.json @@ -13,6 +13,10 @@ { "note": "Add `LibFixedMath` `RevertError` types.", "pr": "TODO" + }, + { + "note": "Make `RevertError.decode()` return a `RawRevertError` if the selector is unknown.", + "pr": "TODO" } ] }, diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 24ea805dde..4433e4eac7 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -27,6 +27,7 @@ export { decodeBytesAsRevertError, decodeThrownErrorAsRevertError, coerceThrownErrorAsRevertError, + RawRevertError, registerRevertErrorType, RevertError, StringRevertError, diff --git a/packages/utils/src/revert_error.ts b/packages/utils/src/revert_error.ts index 14ba63fae5..76da01048b 100644 --- a/packages/utils/src/revert_error.ts +++ b/packages/utils/src/revert_error.ts @@ -67,7 +67,7 @@ export function coerceThrownErrorAsRevertError(error: Error): RevertError { return decodeThrownErrorAsRevertError(error); } catch (err) { if (isGanacheTransactionRevertError(error)) { - return err; + throw err; } // Handle geth transaction reverts. if (isGethTransactionRevertError(error)) { @@ -88,6 +88,7 @@ export abstract class RevertError extends Error { private static readonly _typeRegistry: ObjectMap = {}; public readonly abi?: RevertErrorAbi; public readonly values: ValueMap = {}; + protected readonly _raw?: string; /** * Decode an ABI encoded revert error. @@ -99,7 +100,10 @@ export abstract class RevertError extends Error { const _bytes = bytes instanceof Buffer ? ethUtil.bufferToHex(bytes) : ethUtil.addHexPrefix(bytes); // tslint:disable-next-line: custom-no-magic-numbers const selector = _bytes.slice(2, 10); - const { decoder, type } = this._lookupType(selector); + if (!(selector in RevertError._typeRegistry)) { + return new RawRevertError(bytes); + } + const { type, decoder } = RevertError._typeRegistry[selector]; const instance = new type(); try { const values = decoder(_bytes); @@ -130,21 +134,13 @@ export abstract class RevertError extends Error { }; } - // Ge tthe registry info given a selector. - private static _lookupType(selector: string): RevertErrorRegistryItem { - if (selector in RevertError._typeRegistry) { - return RevertError._typeRegistry[selector]; - } - throw new Error(`Unknown revert error selector "${selector}"`); - } - /** * Create a RevertError instance with optional parameter values. * Parameters that are left undefined will not be tested in equality checks. * @param declaration Function-style declaration of the revert (e.g., Error(string message)) * @param values Optional mapping of parameters to values. */ - protected constructor(name: string, declaration?: string, values?: ValueMap) { + protected constructor(name: string, declaration?: string, values?: ValueMap, raw?: string) { super(createErrorMessage(name, values)); if (declaration !== undefined) { this.abi = declarationToAbi(declaration); @@ -152,6 +148,7 @@ export abstract class RevertError extends Error { _.assign(this.values, _.cloneDeep(values)); } } + this._raw = raw; // Extending Error is tricky; we need to explicitly set the prototype. Object.setPrototypeOf(this, new.target.prototype); } @@ -181,6 +178,10 @@ export abstract class RevertError extends Error { if (!_.isNil(this.abi)) { return toSelector(this.abi); } + if (this._isRawType) { + // tslint:disable-next-line: custom-no-magic-numbers + return (this._raw as string).slice(2, 10); + } return ''; } @@ -230,6 +231,10 @@ export abstract class RevertError extends Error { if (this._isAnyType || _other._isAnyType) { return true; } + // If either are raw types, they must match their raw data. + if (this._isRawType || _other._isRawType) { + return this._raw === _other._raw; + } // Must be of same type. if (this.constructor !== _other.constructor) { return false; @@ -252,6 +257,9 @@ export abstract class RevertError extends Error { } public encode(): string { + if (this._raw !== undefined) { + return this._raw; + } if (!this._hasAllArgumentValues) { throw new Error(`Instance of ${this.typeName} does not have all its parameter values set.`); } @@ -260,6 +268,9 @@ export abstract class RevertError extends Error { } public toString(): string { + if (this._isRawType) { + return `${this.constructor.name}(${this._raw})`; + } const values = _.omitBy(this.values, (v: any) => _.isNil(v)); const inner = _.isEmpty(values) ? '' : inspect(values); return `${this.constructor.name}(${inner})`; @@ -274,7 +285,11 @@ export abstract class RevertError extends Error { } private get _isAnyType(): boolean { - return _.isNil(this.abi); + return _.isNil(this.abi) && _.isNil(this._raw); + } + + private get _isRawType(): boolean { + return !_.isNil(this._raw); } private get _hasAllArgumentValues(): boolean { @@ -361,6 +376,20 @@ export class AnyRevertError extends RevertError { } } +/** + * Special RevertError type that is not decoded. + */ +export class RawRevertError extends RevertError { + constructor(encoded: string | Buffer) { + super( + 'RawRevertError', + undefined, + undefined, + typeof encoded === 'string' ? encoded : ethUtil.bufferToHex(encoded), + ); + } +} + /** * Create an error message for a RevertError. * @param name The name of the RevertError. @@ -488,3 +517,4 @@ function toSelector(abi: RevertErrorAbi): string { // Register StringRevertError RevertError.registerType(StringRevertError); +// tslint:disable-next-line max-file-line-count diff --git a/packages/utils/test/revert_error_test.ts b/packages/utils/test/revert_error_test.ts index d9dd9f001d..5b9bf7a5f3 100644 --- a/packages/utils/test/revert_error_test.ts +++ b/packages/utils/test/revert_error_test.ts @@ -1,7 +1,12 @@ import * as chai from 'chai'; import * as _ from 'lodash'; -import { AnyRevertError, RevertError, StringRevertError } from '../src/revert_error'; +import { + AnyRevertError, + RawRevertError, + RevertError, + StringRevertError, +} from '../src/revert_error'; import { chaiSetup } from './utils/chai_setup'; @@ -107,6 +112,16 @@ describe('RevertError', () => { const revert2 = new DescendantRevertError(message); expect(revert1.equals(revert2)).to.be.false(); }); + it('should equate two `RawRevertError` types with the same raw data', () => { + const revert1 = new RawRevertError('0x0123456789'); + const revert2 = new RawRevertError(revert1.encode()); + expect(revert1.equals(revert2)).to.be.true(); + }); + it('should not equate two `RawRevertError` types with the different raw data', () => { + const revert1 = new RawRevertError('0x0123456789'); + const revert2 = new RawRevertError(`${revert1.encode()}00`); + expect(revert1.equals(revert2)).to.be.false(); + }); }); describe('registering', () => { it('should throw when registering an already registered signature', () => { @@ -133,15 +148,15 @@ describe('RevertError', () => { const decoded = RevertError.decode(encoded); expect(decoded.equals(expected)).to.be.true(); }); - it('should fail to decode an ABI encoded revert error with an unknown selector', () => { + it('should decode an unknown selector as a `RawRevertError`', () => { const _encoded = encoded.substr(0, 2) + '00' + encoded.substr(4); - const decode = () => RevertError.decode(_encoded); - expect(decode).to.be.throw(); + const decoded = RevertError.decode(_encoded); + expect(decoded instanceof RawRevertError).to.be.true(); }); it('should fail to decode a malformed ABI encoded revert error', () => { - const _encoded = encoded.substr(0, encoded.substr.length - 1); + const _encoded = encoded.substr(0, encoded.length - 1); const decode = () => RevertError.decode(_encoded); - expect(decode).to.be.throw(); + expect(decode).to.throw(); }); }); describe('encoding', () => {