Remove truffle contracts dependency #151

LogvinovLeon · 2017-09-04T16:20:41Z

This PR:

Removes truffle-conract 🥇 💯 👍
Starts using custom contract abstraction
Stops awaiting transactions to be mined and returns transaction hash immidiately
Adds zeroEx.awaitTransactionMinedAsync
Removes web3_beta and tests, but still leaves support for the web3_beta provider, cause having both web3's breaks the instalation.

…a transaction to be mined

coveralls · 2017-09-05T09:42:14Z

Coverage increased (+71.6%) to 97.995% when pulling a12df1c on feature/remove-truffle-contracts into 0275ac9 on development.

fabioberger · 2017-09-05T09:51:39Z

src/0x.ts

@@ -249,4 +252,23 @@ export class ZeroEx {

        throw new Error(ZeroExError.InvalidSignature);
    }
+    /**
+     * Waits for transaction to be mined and returns the transaction receipt


Waits for a transaction to be mined and returns the transaction receipt

fabioberger · 2017-09-05T09:52:19Z

src/0x.ts

+     * @return  Web3.TransactionReceipt
+     */
+    public async awaitTransactionMinedAsync(txHash: string,
+                                            pollingIntervalMs: number = 500): Promise<TransactionReceipt> {


fabioberger · 2017-09-05T09:55:25Z

src/contract.ts

@@ -0,0 +1,68 @@
+import * as Web3 from 'web3';
+import * as _ from 'lodash';
+import promisify = require('es6-promisify');


Can we not use the regular import style?

Unfortunately. Whenever the package exports a function (non module object) we need to do it like that.
https://stackoverflow.com/a/35706271/3546986

fabioberger · 2017-09-05T09:57:36Z

src/contract.ts

+    public abi: Web3.ContractAbi;
+    private contract: Web3.ContractInstance;
+    private defaults: Partial<Web3.TxData>;
+    [name: string]: any;


Add comment explaining this and where it is going to be used.

fabioberger · 2017-09-05T10:06:51Z

src/contract.ts

+        _.forEach(functionsAbi, (functionAbi: Web3.MethodAbi) => {
+            const cbStyleFunction = this.contract[functionAbi.name];
+            if (functionAbi.constant) {
+                this[functionAbi.name] = promisify(cbStyleFunction, this.contract);


Let's remove this one and force people to use .callAsync

fabioberger · 2017-09-05T10:17:48Z

src/contract.ts

+            const promise = new Promise((resolve, reject) => {
+                const lastArg = args[args.length - 1];
+                let txData: Partial<Web3.TxData> = {};
+                if (_.isObject(lastArg) && !_.isArray(lastArg) && !lastArg.isBigNumber) {


Let's make this validation stricter. We should make sure that it's an object that conforms to the txData schema.

fabioberger · 2017-09-05T10:18:51Z

src/contract.ts

+                }
+                txData = {
+                    ...txData,
+                    ...this.defaults,


The defaults should not overwrite the txData.

fabioberger · 2017-09-05T10:27:58Z

src/contract_wrappers/exchange_wrapper.ts

@@ -182,10 +181,7 @@ export class ExchangeWrapper extends ContractWrapper {
                gas,
            },
        );
-        this._throwErrorLogsAsErrors(response.logs);


Remove this method

fabioberger · 2017-09-05T10:31:41Z

src/contract_wrappers/exchange_wrapper.ts

@@ -730,7 +721,9 @@ export class ExchangeWrapper extends ContractWrapper {
        if (!_.isUndefined(this._exchangeContractIfExists)) {
            return this._exchangeContractIfExists;
        }
-        const contractInstance = await this._instantiateContractIfExistsAsync((ExchangeArtifacts as any));
+        const contractInstance = await this._instantiateContractIfExistsAsync<ExchangeContract>(
+            (ExchangeArtifacts as any),


Why did we not have to cast as Artifact here?

fabioberger · 2017-09-05T10:43:53Z

src/web3_wrapper.ts

+            if (_.isUndefined(artifact.networks[networkIdIfExists])) {
+                throw new Error(ZeroExError.ContractNotDeployedOnNetwork);
+            }
+            address = artifact.networks[networkIdIfExists].address.toLowerCase();


Let's not overwrite function args.

fabioberger · 2017-09-05T19:59:50Z

src/globals.d.ts

+// abi-decoder declarations
+interface DecodedLogArg {
+    name: string;
+    value: string|BigNumber.BigNumber;


Can this really only be string or bignumber? What about boolean?

We already had this discussion.
The result of it is ContractEventArg type in types.ts, which I can't import in global.ts.
We have a rule to not define unneccessary types before they're needed.
Currently we will decode only our events and our events have only those types

fabioberger · 2017-09-05T20:00:50Z

src/index.ts

-    TransactionReceipt,
+    TransactionReceiptWithDecodedLogs,
+    LogWithDecodedArgs,
+    DecodedLogArgs,


please put the relevant ones in public types list in website.

Will do after this is finalized and merged

fabioberger · 2017-09-05T20:01:58Z

test/exchange_wrapper_test.ts

                        signedOrder, partialFillAmount, shouldThrowOnInsufficientBalanceOrAllowance, takerAddress);
+                    await zeroEx.awaitTransactionMinedAsync(txHash);
                    expect(await zeroEx.token.getBalanceAsync(makerTokenAddress, makerAddress))
                        .to.be.bignumber.equal(fillableAmount.minus(partialFillAmount));
                    expect(await zeroEx.token.getBalanceAsync(takerTokenAddress, makerAddress))


Can we add a test that makes sure that the decodedLogs that are returned are of the expected format and values?

coveralls · 2017-09-05T23:02:32Z

Coverage increased (+72.0%) to 98.393% when pulling 8ebc724 on feature/remove-truffle-contracts into 0275ac9 on development.

fabioberger · 2017-09-06T05:04:24Z

test/0x.js_test.ts

+        afterEach(async () => {
+            await blockchainLifecycle.revertAsync();
+        });
+        it('return transaction receipt with decoded logs', async () => {


coveralls · 2017-09-06T08:29:20Z

Coverage increased (+49.9%) to 76.339% when pulling 258b4fa on feature/remove-truffle-contracts into 0275ac9 on development.

LogvinovLeon added 7 commits September 4, 2017 12:07

Remove truffle-contract from deps

59fed02

Use custom contract abstraction

9818eb2

Fix tests descriptions

1c2d4cb

Make the functions immidiately return txHash instead of awaiting for …

1ad395c

…a transaction to be mined

Use Web3.ContractInstance type

2577d8f

Temporarily remove web3_beta, cause it breaks installation and tests

6325a03

Implement zeroEx.awaitTransactionMined

c9e490b

0xProject deleted a comment from coveralls Sep 4, 2017

LogvinovLeon added 2 commits September 5, 2017 10:07

Change non-exhange contracts to also return txHash

2b547f9

Increase timeout

5d57a2f