Skip to content

Commit

Permalink
Merge pull request #383 from tasitlabs/feature/open-threads
Browse files Browse the repository at this point in the history 
Open threads
  • Loading branch information
@pcowgill
pcowgill authored May 7, 2019
2 parents e7dfa84 + ca7b719 commit bfe36a4
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 81 deletions.
4 changes: 2 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
"ganache:stop": "kill `ps ax|grep ganache-cli|grep -v grep| awk '{print $1}'` 2> /dev/null; exit 0",
"truffle:migrate": "npx lerna run migrate --scope tasit-contracts --stream",
"prepare:blockchain": "npm run ganache:stop && npm run ganache:start && npm run truffle:migrate",
"pretest": "npm run prepare:blockchain",
"test": "npm run lint && npx lerna run test --stream --concurrency 1",
"pretest": "npm run prepare:blockchain && npm run lint",
"test": "npx lerna run test --stream --concurrency 1",
"posttest": "npm run ganache:stop",
"start": "npm run clean:all && npm run bootstrap && npm run prepare:blockchain",
"lint": "npx prettier --write './packages/*/src/{*.js,**/*.js}' && npx eslint './packages/*/src/{*.js,**/*.js}'"
Expand Down
27 changes: 1 addition & 26 deletions packages/tasit-action/src/contract/Action.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ export class Action extends Subscription {
#txConfirmations = 0;
#timeout;
#lastConfirmationTime;
#isRunning = false;

constructor(rawAction, provider, signer) {
// Provider implements EventEmitter API and it's enough
Expand Down Expand Up @@ -124,7 +123,7 @@ export class Action extends Subscription {
const eventName = "confirmation";

// eslint-disable-next-line no-unused-vars
const baseEthersListener = async blockNumber => {
const ethersListener = async blockNumber => {
try {
const tx = await this.#tx;
if (!tx) {
Expand Down Expand Up @@ -198,30 +197,6 @@ export class Action extends Subscription {
}
};

// 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: 2 additions & 7 deletions packages/tasit-action/src/contract/Contract.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,6 @@ describe("TasitAction.Contract", () => {
const contractErrorListener = sinon.fake(error => {
const { message } = error;
console.info(message);

sampleContract.off("error");
});

sampleContract.on("error", contractErrorListener);
Expand Down Expand Up @@ -472,9 +470,7 @@ describe("TasitAction.Contract", () => {
// new difficultywise-longest well-formed blockchain which excludes one or more blocks that
// the client previously thought were part of the difficultywise-longest well-formed blockchain.
// These excluded blocks become orphans.
//
// Non-deterministic
it.skip("should emit error event when block reorganization occurs - tx confirmed twice", async () => {
it("should emit error event when block reorganization occurs - tx confirmed twice", async () => {
const confirmationListener = sinon.fake();
const errorFn = sinon.fake();

Expand Down Expand Up @@ -533,8 +529,7 @@ describe("TasitAction.Contract", () => {
expect(actionId).to.have.lengthOf(66);
});

// Non-deterministic
it.skip("should be able to listen to an event before sending", async () => {
it("should be able to listen to an event before sending", async () => {
const confirmationListener = sinon.fake(async () => {
action.off("confirmation");
});
Expand Down
64 changes: 48 additions & 16 deletions packages/tasit-action/src/contract/Subscription.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,7 @@ export class Subscription {

// TODO: Make protected
_setEventTimer = (eventName, timer) => {
const eventListener = this.#eventListeners.get(eventName);

if (!eventListener) {
console.warn(`A listener for event '${eventName}' isn't registered.`);
return;
}

const { listener } = eventListener;

this.#eventListeners.set(eventName, { listener, timer });
this.#decorateEventListener(eventName, { timer });
};

// TODO: Make protected
Expand Down Expand Up @@ -93,8 +84,9 @@ export class Subscription {
// If there is a error event already, it will be replaced by new listener function
// that will call both new and old functions
_addErrorListener = newListener => {
const eventName = "error";
let listener = newListener;
const oldErrorEventListener = this.#eventListeners.get("error");
const oldErrorEventListener = this.#eventListeners.get(eventName);

if (oldErrorEventListener) {
listener = error => {
Expand All @@ -103,13 +95,13 @@ export class Subscription {
};
}

this.#eventListeners.set("error", {
this.#eventListeners.set(eventName, {
listener,
});
};

// TODO: Make protected
_addEventListener = (eventName, listener) => {
_addEventListener = (eventName, baseListener) => {
if (eventName === "error")
throw new Error(
`Use _addErrorListener function to subscribe to an error event.`
Expand All @@ -120,9 +112,36 @@ export class Subscription {
`A listener for event '${eventName}' is already registered.`
);

this.#eventListeners.set(eventName, {
listener,
});
// 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
//
// Question:
// Is it possible that that behavior (listener concurrent calls for the same event) is desirable?
const listener = async (...args) => {
const eventListener = this.#eventListeners.get(eventName);
const { isRunning } = eventListener;

if (isRunning) {
console.info(`Listener is already running`);
return;
}

this.#decorateEventListener(eventName, { isRunning: true });
await baseListener(...args);
this.#decorateEventListener(eventName, { isRunning: false });
};

const eventListener = { listener, isRunning: false };

this.#eventListeners.set(eventName, eventListener);

this.#ethersEventEmitter.on(this._toEthersEventName(eventName), listener);
};
Expand All @@ -131,6 +150,19 @@ export class Subscription {
getEmitter = () => {
return this.#ethersEventEmitter;
};

#decorateEventListener = (eventName, newArgs) => {
let eventListener = this.#eventListeners.get(eventName);

if (!eventListener) {
console.warn(`A listener for event '${eventName}' isn't registered.`);
return;
}

eventListener = { ...eventListener, ...newArgs };

this.#eventListeners.set(eventName, eventListener);
};
}

export default Subscription;
5 changes: 1 addition & 4 deletions packages/tasit-action/src/erc721/ERC721Full.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,7 @@ describe("TasitAction.ERC721.ERC721Full", () => {
await expectExactTokenBalances(erc721, [ana.address], [1]);
});

// Non-deterministic
// Enable again after solve this isse:
// https://github.com/tasitlabs/TasitSDK/issues/367
it.skip("should transfer an owned token", async () => {
it("should transfer an owned token", async () => {
erc721 = new ERC721Full(ERC721_FULL_ADDRESS, ana);

const transferListener = sinon.fake(message => {
Expand Down
107 changes: 81 additions & 26 deletions packages/tasit-sdk/src/Decentraland.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,43 @@ describe("Decentraland", () => {
})();
});

it("should buy an estate", done => {
it("should buy an estate - using action events", done => {
(async () => {
const { assetId, nftAddress, priceInWei } = estateForSale;

await checkAsset(estate, mana, estateForSale, ephemeralAddress);

await expectExactTokenBalances(estate, [ephemeralAddress], [0]);

const fingerprint = await estate.getFingerprint(`${assetId}`);

marketplace.setWallet(ephemeralWallet);
const executeOrderAction = marketplace.safeExecuteOrder(
nftAddress,
`${assetId}`,
`${priceInWei}`,
`${fingerprint}`
);

const confirmationListener = async () => {
executeOrderAction.off("error");
await expectExactTokenBalances(estate, [ephemeralAddress], [1]);
done();
};

const errorListener = error => {
executeOrderAction.off("error");
done(error);
};

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

executeOrderAction.send();
})();
});

it("should buy an estate - using contract events", done => {
(async () => {
const { assetId, nftAddress, priceInWei } = estateForSale;

Expand All @@ -233,40 +269,67 @@ describe("Decentraland", () => {
`${fingerprint}`
);

// eslint-disable-next-line no-unused-vars
const orderSuccessfulListener = async message => {
const { data } = message;
const { args } = data;
const { buyer } = args;

marketplace.off("error");
expect(buyer).to.equal(ephemeralAddress);
await expectExactTokenBalances(estate, [ephemeralAddress], [1]);

done();
};

const errorListener = error => {
const { message } = error;
console.warn(message);
marketplace.off("error");
done(error);
};

marketplace.once("OrderSuccessful", orderSuccessfulListener);
marketplace.on("error", errorListener);

executeOrderAction.send();
})();
});

it("should buy a parcel of land - using action events", done => {
(async () => {
const { assetId, nftAddress, priceInWei } = landForSale;

await checkAsset(land, mana, landForSale, ephemeralAddress);

await expectExactTokenBalances(land, [ephemeralAddress], [0]);

// LANDRegistry contract doesn't implement getFingerprint function
const fingerprint = "0x";
marketplace.setWallet(ephemeralWallet);
const executeOrderAction = marketplace.safeExecuteOrder(
nftAddress,
`${assetId}`,
`${priceInWei}`,
`${fingerprint}`
);

const confirmationListener = async () => {
await expectExactTokenBalances(estate, [ephemeralAddress], [1]);
executeOrderAction.off("error");
await expectExactTokenBalances(land, [ephemeralAddress], [1]);
done();
};

const errorListener = error => {
marketplace.off("error");
executeOrderAction.off("error");
done(error);
};

// Note: These listeners aren't working properly
// See more: https://github.com/tasitlabs/TasitSDK/issues/367
//marketplace.once("OrderSuccessful", orderSuccessfulListener);
//marketplace.on("error", errorListener);
executeOrderAction.once("confirmation", confirmationListener);
executeOrderAction.on("error", errorListener);

executeOrderAction.send();
})();
});

it("should buy a parcel of land", done => {
it("should buy a parcel of land - using contract events", done => {
(async () => {
const { assetId, nftAddress, priceInWei } = landForSale;

Expand All @@ -284,20 +347,14 @@ describe("Decentraland", () => {
`${fingerprint}`
);

// eslint-disable-next-line no-unused-vars
const orderSuccessfulListener = async message => {
const { data } = message;
const { args } = data;
const { buyer } = args;

expect(buyer).to.equal(ephemeralAddress);
await expectExactTokenBalances(land, [ephemeralAddress], [1]);

done();
};

const confirmationListener = async () => {
await expectExactTokenBalances(land, [ephemeralAddress], [1]);
marketplace.off("error");
done();
};

Expand All @@ -306,12 +363,8 @@ describe("Decentraland", () => {
done(error);
};

// Note: These listeners aren't working properly
// See more: https://github.com/tasitlabs/TasitSDK/issues/367
// marketplace.once("OrderSuccessful", orderSuccessfulListener);
// marketplace.on("error", errorListener);
executeOrderAction.once("confirmation", confirmationListener);
executeOrderAction.on("error", errorListener);
marketplace.once("OrderSuccessful", orderSuccessfulListener);
marketplace.on("error", errorListener);

executeOrderAction.send();
})();
Expand Down Expand Up @@ -651,6 +704,8 @@ describe("Decentraland", () => {

const confirmationListener = async () => {
// WIP: Not working because of gas issue on Marketplace.safeExecuteOrder() call
//
// When that issue was solved, the correct expectation is having balance equal 1 instead of 0
//await expectExactTokenBalances(estate, [GNOSIS_SAFE_ADDRESS], [1]);
await expectExactTokenBalances(
estate,
Expand Down Expand Up @@ -720,6 +775,8 @@ describe("Decentraland", () => {
const confirmationListener = async () => {
// WIP: Not working because of gas issue on Marketplace.safeExecuteOrder() call
//await expectExactTokenBalances(land, [GNOSIS_SAFE_ADDRESS], [1]);
//
// When that issue was solved, the correct expectation is having balance equal 1 instead of 0
await expectExactTokenBalances(land, [GNOSIS_SAFE_ADDRESS], [0]);
done();
};
Expand Down Expand Up @@ -880,7 +937,6 @@ describe("Decentraland", () => {
// See more: https://github.com/tasitlabs/TasitSDK/issues/273
//await expectExactTokenBalances(estate, [ephemeralAddress], [1]);
await expectExactTokenBalances(estate, [ephemeralAddress], [0]);
done();
};

const errorListener = error => {
Expand Down Expand Up @@ -929,7 +985,6 @@ describe("Decentraland", () => {
// See more: https://github.com/tasitlabs/TasitSDK/issues/273
//await expectExactTokenBalances(land, [ephemeralAddress], [1]);
await expectExactTokenBalances(land, [ephemeralAddress], [0]);
done();
};

const errorListener = error => {
Expand Down

0 comments on commit bfe36a4

Please sign in to comment.