Skip to content

Commit

Permalink
Merge pull request #369 from tasitlabs/feature/no-waiting
Browse files Browse the repository at this point in the history 
Test cases from waiting to event driven style
  • Loading branch information
@pcowgill
pcowgill authored May 2, 2019
2 parents e93eb68 + df47472 commit 4111170
Show file tree
Hide file tree
Showing 9 changed files with 877 additions and 520 deletions.
85 changes: 49 additions & 36 deletions packages/tasit-action/src/contract/Action.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ export class Action extends Subscription {
#signer;
#rawTx;
#tx;
#txConfirmations;
#txConfirmations = 0;
#timeout;
#lastConfirmationTime;
#isRunning = false;

constructor(rawTx, provider, signer) {
// Provider implements EventEmitter API and it's enough
Expand All @@ -33,28 +34,28 @@ export class Action extends Subscription {
};

#signAndSend = async () => {
// TODO: Go deep on gas handling.
// Without that, VM returns a revert error instead of out of gas error.
// See: https://github.com/tasitlabs/TasitSDK/issues/173
//
// This command isn't enough
// const gasLimit = await this.#provider.estimateGas(this.#rawTx);
const gasParams = {
gasLimit: 7e6,
gasPrice: 1e9,
};
try {
// TODO: Go deep on gas handling.
// Without that, VM returns a revert error instead of out of gas error.
// See: https://github.com/tasitlabs/TasitSDK/issues/173
//
// This command isn't enough
// const gasLimit = await this.#provider.estimateGas(this.#rawTx);
const gasParams = {
gasLimit: 7e6,
gasPrice: 1e9,
};

const nonce = await this.#provider.getTransactionCount(
this.#signer.address
);
const nonce = await this.#provider.getTransactionCount(
this.#signer.address
);

let rawTx = await this.#rawTx;
let rawTx = await this.#rawTx;

rawTx = { ...rawTx, nonce, ...gasParams };
rawTx = { ...rawTx, nonce, ...gasParams };

const signedTx = await this.#signer.sign(rawTx);
const signedTx = await this.#signer.sign(rawTx);

try {
this.#tx = await this.#provider.sendTransaction(signedTx);
} catch (error) {
this._emitErrorEvent(new Error(`Action with error: ${error.message}`));
Expand Down Expand Up @@ -109,7 +110,7 @@ export class Action extends Subscription {
try {
const tx = await this.#tx;
if (!tx) {
console.warn(`The action wasn't sent yet.`);
console.info(`The action wasn't sent yet.`);
return;
}

Expand All @@ -128,12 +129,11 @@ export class Action extends Subscription {
);
}

if (receipt === null) {
this.#txConfirmations = 0;
return;
}
if (!receipt) return;

const { confirmations, status } = receipt;
const txFailed = status === 0;

const txFailed = receipt.status == 0;
if (txFailed) {
this._emitErrorEventFromEventListener(
new Error(`Action failed.`),
Expand Down Expand Up @@ -161,8 +161,6 @@ export class Action extends Subscription {

this._setEventTimer(eventName, timer);

const { confirmations } = receipt;

this.#txConfirmations = confirmations;

const message = {
Expand All @@ -171,7 +169,9 @@ export class Action extends Subscription {
},
};

// Note: Unsubscribing should be done after the user's listener function is called
await listener(message);
if (once) this.off(eventName);
} catch (error) {
this._emitErrorEventFromEventListener(
new Error(`Listener function with error: ${error.message}`),
Expand All @@ -180,16 +180,29 @@ export class Action extends Subscription {
}
};

let ethersListener;

if (once) {
ethersListener = async blockNumber => {
await baseEthersListener(blockNumber);
this.off(eventName);
};
} else {
ethersListener = baseEthersListener;
}
// Note:
// On the development env (using ganache-cli)
// Blocks are being mined simultaneously and generating a sort of unexpected behaviors like:
// - once listeners called many times
// - sequential blocks giving same confirmation to a transaction
// - false-positive reorg event emission
// - collaborating for tests non-determinism
//
// Tech debt:
// See if there is another way to avoid these problems, if not
// this solution should be improved with a state structure identifying state per event
//
// Question:
// Is possible that behavior (listener concurrency calls for the same event) be desirable?
const ethersListener = async blockNumber => {
if (this.#isRunning) {
console.info(`Listener is already running`);
return;
}
this.#isRunning = true;
await baseEthersListener(blockNumber);
this.#isRunning = false;
};

this._addEventListener(eventName, ethersListener);
};
Expand Down
9 changes: 4 additions & 5 deletions packages/tasit-action/src/contract/Contract.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ export class Contract extends Subscription {
},
};

if (once) this.off(eventName);

// Note: Unsubscribing should be done after the user's listener function is called
await listener(message);
if (once) this.off(eventName);
} catch (error) {
this._emitErrorEventFromEventListener(
new Error(`Listener function with error: ${error.message}`),
Expand Down Expand Up @@ -177,9 +177,8 @@ export class Contract extends Subscription {

const action = new Action(rawTx, this.#provider, signer);

const errorListener = message => {
const { error } = message;
this._emitErrorEvent(new Error(`${error.message}`));
const errorListener = error => {
this._emitErrorEvent(error);
};

action.on("error", errorListener);
Expand Down
77 changes: 48 additions & 29 deletions packages/tasit-action/src/contract/Contract.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe("TasitAction.Contract", () => {
});

it("and Action error event on action error", async () => {
const contractErrorListener = sinon.fake(() => {
const contractErrorListener = sinon.fake(error => {
sampleContract.off("error");
});

Expand All @@ -159,7 +159,7 @@ describe("TasitAction.Contract", () => {
// See more: https://github.com/tasitlabs/TasitSDK/issues/253
action.on("confirmation", () => {});

const actionErrorListener = sinon.fake(() => {
const actionErrorListener = sinon.fake(error => {
action.off("error");
});

Expand All @@ -175,24 +175,19 @@ describe("TasitAction.Contract", () => {
expect(actionErrorListener.callCount).to.equal(1);
});

// Non-deterministic
it.skip("on contract event listener error", async () => {
const errorListener = sinon.fake();
it("on contract event listener error", done => {
action = sampleContract.setValue("hello world");

const errorListener = sinon.fake(error => {
expect(eventListener.callCount).to.be.at.least(1);
done();
});
const eventListener = sinon.fake.throws(new Error());

sampleContract.on("error", errorListener);
sampleContract.on("ValueChanged", eventListener);

action = sampleContract.setValue("hello world");
await action.send();

await action.waitForOneConfirmation();

await mineBlocks(provider, 4);

// Non-deterministic
expect(eventListener.callCount).to.be.at.least(1);
expect(errorListener.callCount).to.be.at.least(1);
action.send();
});
});

Expand Down Expand Up @@ -331,8 +326,7 @@ describe("TasitAction.Contract", () => {
action = sampleContract.setValue("hello world");
action.setEventsTimeout(100);

const errorListener = sinon.fake(message => {
const { error } = message;
const errorListener = sinon.fake(error => {
expect(error.eventName).to.equal("confirmation");
expect(error.message).to.equal("Event confirmation reached timeout.");
action.off("error");
Expand Down Expand Up @@ -421,11 +415,10 @@ describe("TasitAction.Contract", () => {
confirmationFn();
};

const errorListener = message => {
const { error } = message;

const errorListener = error => {
const { message } = error;
// Note: This assertion will not fail the test case (UnhandledPromiseRejectionWarning)
expect(error.message).to.equal(
expect(message).to.equal(
"Your action's position in the chain has changed in a surprising way."
);

Expand Down Expand Up @@ -468,11 +461,11 @@ describe("TasitAction.Contract", () => {
const confirmationListener = sinon.fake();
const errorFn = sinon.fake();

const errorListener = message => {
const { error } = message;
const errorListener = error => {
const { message } = error;

// Note: This assertion will not fail the test case (UnhandledPromiseRejectionWarning)
expect(error.message).to.equal(
expect(message).to.equal(
"Your action's position in the chain has changed in a surprising way."
);

Expand Down Expand Up @@ -541,7 +534,35 @@ describe("TasitAction.Contract", () => {

await mineBlocks(provider, 2);

expect(confirmationListener.callCount).to.equal(1);
// Non-determinitic
expect(confirmationListener.callCount).to.be.at.least(1);
expect(errorListener.called).to.be.false;
});

it("'once' listener should be unsubscribed only after user listener function was called", async () => {
action = sampleContract.setValue(rand);

const errorListener = sinon.fake(error => {
console.log(error);
action.off("error");
});

const confirmationListener = sinon.fake(message => {
console.log(message);
});

action.on("error", errorListener);
action.once("confirmation", confirmationListener);

// Forcing internal (block) listener to be called before the transaction is sent
await mineBlocks(provider, 2);

await action.send();
await action.waitForOneConfirmation();

await mineBlocks(provider, 2);

expect(confirmationListener.called).to.be.true;
expect(errorListener.called).to.be.false;
});
});
Expand All @@ -557,8 +578,7 @@ describe("TasitAction.Contract", () => {
const fakeFn = sinon.fake();
const errorFakeFn = sinon.fake();

const errorListener = message => {
const { error } = message;
const errorListener = error => {
errorFakeFn();
};

Expand Down Expand Up @@ -591,8 +611,7 @@ describe("TasitAction.Contract", () => {
const fakeFn = sinon.fake();
const errorFakeFn = sinon.fake();

const errorListener = message => {
const { error } = message;
const errorListener = error => {
errorFakeFn();
};

Expand Down
3 changes: 1 addition & 2 deletions packages/tasit-action/src/contract/Subscription.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ export class Subscription {
return;
}

const message = { error };
errorEventListener.listener(message);
errorEventListener.listener(error);
};

// TODO: Make protected
Expand Down
6 changes: 2 additions & 4 deletions packages/tasit-action/src/erc721/ERC721Full.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,7 @@ describe("TasitAction.ERC721.ERC721Full", () => {

erc721 = new ERC721Full(ERC721_FULL_ADDRESS, ana);

const contractErrorListener = errorMessage => {
const { error } = errorMessage;
const contractErrorListener = error => {
const { message } = error;
expect(message).to.equal("Action failed.");
contractErrorFakeFn();
Expand Down Expand Up @@ -240,8 +239,7 @@ describe("TasitAction.ERC721.ERC721Full", () => {

erc721 = new ERC721Full(ERC721_FULL_ADDRESS, ana);

const actionErrorListener = errorMessage => {
const { error } = errorMessage;
const actionErrorListener = error => {
const { message } = error;
expect(message).to.equal("Action failed.");
actionErrorFakeFn();
Expand Down
4 changes: 2 additions & 2 deletions packages/tasit-contract-based-account/src/GnosisSafe.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe("GnosisSafe", () => {
gnosisSafe.setWallet(gnosisSafeOwner);
const action = gnosisSafe.transferEther(toAddress, ONE);

const errorListener = async message => {
const errorListener = async error => {
onError();
action.unsubscribe();
};
Expand Down Expand Up @@ -184,7 +184,7 @@ describe("GnosisSafe", () => {
gnosisSafe.setWallet(ephemeralAccount);
const action = erc20.transferFrom(GNOSIS_SAFE_ADDRESS, toAddress, ONE);

const errorListener = async message => {
const errorListener = async error => {
onError();
action.unsubscribe();
};
Expand Down
18 changes: 9 additions & 9 deletions packages/tasit-contracts/src/scripts/populateDecentralandContracts.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ const createParcel = async parcel => {
resolve(id);
});

action.on("error", message => {
const { error } = message;
console.log(error.message);
action.on("error", error => {
const { message } = error;
console.log(message);
action.unsubscribe();
reject();
});
Expand Down Expand Up @@ -252,16 +252,16 @@ const createEstate = async estate => {
// See more: https://github.com/tasitlabs/TasitSDK/issues/253
action.on("confirmation", message => {});

action.on("error", message => {
const { error } = message;
console.log(error.message);
action.on("error", error => {
const { message } = error;
console.log(message);
action.unsubscribe();
reject();
});

estateContract.on("error", message => {
const { error } = message;
console.log(error.message);
estateContract.on("error", error => {
const { message } = error;
console.log(message);
estateContract.unsubscribe();
reject();
});
Expand Down
Loading

0 comments on commit 4111170

Please sign in to comment.