tasitlabs / tasit-sdk

A JavaScript / TypeScript SDK for making native mobile Ethereum dapps using React Native
https://tasit.dev
MIT License
97 stars 10 forks source link

Investigate parallel listener calls #380

Open marcelomorgado opened 5 years ago

marcelomorgado commented 5 years ago

On the development environment, are being called simultaneously and resulting in unexpected behaviors. Actually, blocks are being mined so shortly that a listener for a block is being called before of the call from the last block. For now, a call will be ignored if has another call running. Comment extracted from: tasit-action > Action.js > #addConfirmationListener:

// 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 it possible that that behavior (listener concurrent calls for the same event) is desirable?

Extracted from: https://github.com/tasitlabs/TasitSDK/pull/369#discussion_r280218408

pcowgill commented 5 years ago

For now, a call will be ignored if has another call running.

Is this explicitly enforced in our code, or is this the default behavior?

marcelomorgado commented 5 years ago

@pcowgill commented:

Is the non-determinism mostly for tests where the listener is added before or after sending the tx? Or is it a little of both?

We're sure it's not just getting called again with another confirmation for each instantaneous block, right? If so, we could probably disable that in our code and check number of confirmations on each listener call. Or something like that.

marcelomorgado commented 5 years ago

For now, a call will be ignored if has another call running.

Is this explicitly enforced in our code, or is this the default behavior?

Is this explicitly enforced in our code:

From Subscription class:

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 });
};
pcowgill commented 5 years ago

Got it - thanks