Repository: ecadlabs/taquito (ts)

Issue:
Events are fired even if the transaction fails

If I have a tx failing, the event is still subscribed and fired on taquito side, we can see the result field with the error stacktrace on it

![image](https://user-images.githubusercontent.com/2536368/214626714-3b9047b0-d5d9-42e7-bda8-320f5776d139.png)

I am not really sure that developers are really interested by uncompleted events. 
To not break all current implementation I would suggest to filter all failing tx by default and add an option to not ignore failing tx events in case someone as a real use case around it 

Pull request description:
Implement the ability to filter out events from failed operations
closes #2319 

An optional field `excludeFailedOperations: boolean` is added to `EventFilter`. If this field is set to `true`, event will be propagated to the Dapp only when the result's `result.status` is `applied`.


- [x] Your code builds cleanly without any errors or warnings
- [x] You have run the linter against the changes
- [x] You have added unit tests (if relevant/appropriate)
- [x] You have added integration tests (if relevant/appropriate)
- [ ] All public methods or types have TypeDoc coverage with a complete description, and ideally an @example
- [ ] You have added or updated corresponding documentation
- [ ] If relevant, you have written a first draft summary describing the change for inclusion in Release Notes. 

In this PR, please also make sure: 

- [x] You have linked this PR to the issue by putting `closes #TICKETNUMBER` in the description box (when applicable)
- [x] You have added a concise description on your changes
## Release Note Draft Snippet

An optional field `excludeFailedOperations: boolean` is added to `EventFilter`. If this field is set to `true`, event will be propagated to the Dapp only when the result's `result.status` is `applied`.


Patch:
diff --git a/docs/events.md b/docs/events.md
index 8f1a04801..c61157057 100644
--- a/docs/events.md
+++ b/docs/events.md
@@ -29,7 +29,8 @@ Tezos.setStreamProvider(
 try {
   const sub = Tezos.stream.subscribeEvent({
     tag: 'tagName',
-    address: 'KT1_CONTRACT_ADDRESS'
+    address: 'KT1_CONTRACT_ADDRESS',
+    excludeFailedOperations: true
   });
     
   sub.on('data', console.log);
@@ -42,6 +43,7 @@ try {
 
 - `tag` is the tag string that was defined in the smart contract with the EMIT instruction
 - `address` is the address of the smart contract that was called 
+- `excludeFailedOperations`: In rare cases, events from failed operations can be received by the subscriber. You can use this field to filter out these events (if you pass `true` to this field, only events from successful operations will be received)
 
 :::info
 If you would like to subscribe to **_any_** event that goes through, you can call `subscribeEvent()` as is without any parameters
diff --git a/packages/taquito/src/subscribe/filters.ts b/packages/taquito/src/subscribe/filters.ts
index 75ca201f8..3c4c16f02 100644
--- a/packages/taquito/src/subscribe/filters.ts
+++ b/packages/taquito/src/subscribe/filters.ts
@@ -52,20 +52,20 @@ const destinationFilter = (x: OperationContent, filter: DestinationFilter) => {
   }
 };
 
-export const eventFilter = (result: InternalOperationResult, address?: string, tag?: string) => {
-  if (result.kind === 'event') {
-    if (address && tag) {
-      return result.source === address && result.tag === tag;
-    } else if (address && !tag) {
-      return result.source === address;
-    } else if (tag) {
-      return result.tag === tag;
-    } else {
-      return true;
-    }
-  } else {
+export const eventFilter = (result: InternalOperationResult, address?: string, tag?: string, excludeFailedOperations?: boolean) => {
+  if (result.kind !== 'event') {
+    return false;
+  }
+  if (tag && result.tag !== tag) {
+    return false;
+  }
+  if (address && result.source !== address) {
+    return false;
+  }
+  if (excludeFailedOperations && result.result.status !== 'applied') {
     return false;
   }
+  return true;
 };
 
 export const evaluateOpFilter = (op: OperationContent, filter: OpFilter) => {
diff --git a/packages/taquito/src/subscribe/interface.ts b/packages/taquito/src/subscribe/interface.ts
index af5c1ff58..d173f2c4a 100644
--- a/packages/taquito/src/subscribe/interface.ts
+++ b/packages/taquito/src/subscribe/interface.ts
@@ -24,6 +24,7 @@ export interface DestinationFilter {
 export interface EventFilter {
   address?: string;
   tag?: string;
+  excludeFailedOperations?: boolean;
 }
 
 export interface EventSubscription extends InternalOperationResult {
diff --git a/packages/taquito/src/subscribe/polling-subcribe-provider.ts b/packages/taquito/src/subscribe/polling-subcribe-provider.ts
index e6af02a86..7315791fd 100644
--- a/packages/taquito/src/subscribe/polling-subcribe-provider.ts
+++ b/packages/taquito/src/subscribe/polling-subcribe-provider.ts
@@ -68,7 +68,7 @@ const applyEventFilter = (filter?: EventFilter) =>
             const internalOpResults = tx.metadata.internal_operation_results;
             if (internalOpResults) {
               for (const event of internalOpResults) {
-                if (eventFilter(event, filter?.address, filter?.tag)) {
+                if (eventFilter(event, filter?.address, filter?.tag, filter?.excludeFailedOperations)) {
                   sub.next({
                     opHash: op.hash,
                     blockHash: block.hash,


Test patch:
diff --git a/integration-tests/polling-subscribe-provider.spec.ts b/integration-tests/polling-subscribe-provider.spec.ts
index 82a32a756..d8737b053 100644
--- a/integration-tests/polling-subscribe-provider.spec.ts
+++ b/integration-tests/polling-subscribe-provider.spec.ts
@@ -1,78 +1,319 @@
-import { CONFIGS, sleep } from "./config";
-import { PollingSubscribeProvider } from "@taquito/taquito";
+import { CONFIGS, sleep } from './config';
+import { PollingSubscribeProvider, TezosToolkit } from '@taquito/taquito';
+import { localForger } from '@taquito/local-forging';
+import { send } from 'process';
+import { validateAddress } from '@taquito/utils';
 
-CONFIGS().forEach(({ lib, rpc, setup }) => {
+/* mainContract.jsligo: This is the source code for the main contract.
+If you need to change the main contract, you can change this, use the ligo compiler to compile it, and update both the Michelson code below and the jsligo here.
+
+const _mainContractJsLigo = `type storage = int
+type parameter = {p: int, ad: address, t: tez, newStore: int}
+
+export const main = ({p, ad, t, newStore} : parameter, store : storage) => {
+    if (p == 1) {
+        return [list([]), newStore];
+    }
+    assert_with_error(p != 4, "The main contract fails if parameter is four");
+    let tran = Tezos.transaction(p, t, Option.unopt(Tezos.get_contract_opt(ad)));
+    let event1 = Tezos.emit("%intFromMainContract", p + 1);
+    if (store == 1) {
+        let event2 = Tezos.emit("%stringFromMainContract", "lorem ipsum");
+        return [list([event1, event2, tran]), newStore];
+    } else {
+        return [list([event1, tran]), newStore];
+    }
+}`; */
+
+const mainContractMichelson = `{ parameter (pair (pair (address %ad) (int %newStore)) (int %p) (mutez %t)) ;
+  storage int ;
+  code { UNPAIR ;
+         UNPAIR ;
+         UNPAIR ;
+         DIG 2 ;
+         UNPAIR ;
+         PUSH int 1 ;
+         DUP 2 ;
+         COMPARE ;
+         EQ ;
+         IF { SWAP ; DIG 2 ; DIG 4 ; DROP 4 ; NIL operation }
+            { PUSH int 4 ;
+              DUP 2 ;
+              COMPARE ;
+              NEQ ;
+              IF {}
+                 { PUSH string "The main contract fails if parameter is four" ; FAILWITH } ;
+              DIG 2 ;
+              CONTRACT int ;
+              IF_NONE { PUSH string "option is None" ; FAILWITH } {} ;
+              DIG 2 ;
+              DUP 3 ;
+              TRANSFER_TOKENS ;
+              PUSH int 1 ;
+              DIG 2 ;
+              ADD ;
+              EMIT %intFromMainContract int ;
+              PUSH int 1 ;
+              DIG 4 ;
+              COMPARE ;
+              EQ ;
+              IF { PUSH string "lorem ipsum" ;
+                   EMIT %stringFromMainContract string ;
+                   DIG 3 ;
+                   NIL operation ;
+                   DIG 4 ;
+                   CONS ;
+                   DIG 2 }
+                 { DIG 2 ; NIL operation ; DIG 3 } ;
+              CONS ;
+              DIG 2 ;
+              CONS } ;
+         PAIR } }`;
+
+/* CalledContract.jsligo:
+
+const _calledContractJSLigo = `type storage = unit
+type parameter = int
+
+export const main = (param: parameter, store: storage) => {
+  assert_with_error(param != 5, "The called contract fails if parameter is five");
+  return [list([Tezos.emit("%eventFromCalledContract", param + 1)]), store];
+}`; */
+
+const calledContractMichelson = `{ parameter int ;
+  storage unit ;
+  code { UNPAIR ;
+         PUSH int 5 ;
+         DUP 2 ;
+         COMPARE ;
+         NEQ ;
+         IF {}
+            { PUSH string "The called contract fails if parameter is five" ; FAILWITH } ;
+         SWAP ;
+         NIL operation ;
+         PUSH int 1 ;
+         DIG 3 ;
+         ADD ;
+         EMIT %eventFromCalledContract int ;
+         CONS ;
+         PAIR } }`;
+
+CONFIGS().forEach(({ lib, rpc, setup, createAddress }) => {
   const Tezos = lib;
-  let eventContractAddress: string;
+  let calledContractAddress: string;
+  let mainContractAddress: string;
+  let secondUser: TezosToolkit;
 
   describe(`Polling Subscribe Provider using ${rpc}`, () => {
-    beforeEach(async (done) => {
+    beforeAll(async (done) => {
       await setup();
 
-      Tezos.setStreamProvider(Tezos.getFactory(PollingSubscribeProvider)({ shouldObservableSubscriptionRetry: true, pollingIntervalMilliseconds: 1000 }));
+      secondUser = await createAddress();
+      const secondUserAddress = await secondUser.signer.publicKeyHash();
+      const transfer = await Tezos.contract.transfer({ to: secondUserAddress, amount: 1 });
+      await transfer.confirmation();
+
+      Tezos.setStreamProvider(
+        Tezos.getFactory(PollingSubscribeProvider)({
+          shouldObservableSubscriptionRetry: true,
+          pollingIntervalMilliseconds: 1000,
+        })
+      );
 
       try {
-        const op = await Tezos.contract.originate({
-          code: `{ parameter unit ;
-            storage unit ;
-            code { DROP ;
-                   UNIT ;
-                   PUSH nat 10 ;
-                   LEFT string ;
-                   EMIT %first ;
-                   PUSH string "lorem ipsum" ;
-                   RIGHT nat ;
-                   EMIT %second (or (nat %number) (string %words)) ;
-                   NIL operation ;
-                   SWAP ;
-                   CONS ;
-                   SWAP ;
-                   CONS ;
-                   PAIR } }`,
-          storage: 0
+        const calledContract = await Tezos.contract.originate({
+          code: calledContractMichelson,
+          storage: 0,
         });
-        await op.confirmation();
+        await calledContract.confirmation();
+        calledContractAddress = calledContract.contractAddress!;
 
-        eventContractAddress = op.contractAddress!;
-      } catch(e) {
+        let mainContract = await Tezos.contract.originate({
+          code: mainContractMichelson,
+          storage: 0,
+        });
+        await mainContract.confirmation();
+        mainContractAddress = mainContract.contractAddress!;
+      } catch (e) {
         console.log(e);
       }
       done();
     });
 
+    beforeEach(async (done) => {
+      const contract = await Tezos.contract.at(mainContractAddress!);
+      const resetStorageOperation = await contract.methodsObject.default({
+        ad: calledContractAddress,
+        newStore: 0,
+        p: 1,
+        t: 0
+      }).send();
+      await resetStorageOperation.confirmation();
+      done();
+    });
+
     it('should be able to subscribe to events with tag and address params given', async (done) => {
       const data: any = [];
 
       const eventSub = Tezos.stream.subscribeEvent({
-        tag: 'first',
-        address: eventContractAddress
+        tag: 'intFromMainContract',
+        address: mainContractAddress,
       });
 
       eventSub.on('data', (x) => {
         data.push(x);
       });
 
-      const contract = await Tezos.contract.at(eventContractAddress!);
-      const op = await contract.methods.default().send();
+      const contract1 = await Tezos.contract.at(mainContractAddress!);
+      const contract2 = await secondUser.contract.at(mainContractAddress!);
+      
 
-      await op.confirmation();
+      const operation1 = contract1.methodsObject.default({
+        ad: calledContractAddress,
+        newStore: 0,
+        p: 0,
+        t: 0
+      });
+
+      const operation2 = contract2.methodsObject.default({
+        ad: calledContractAddress,
+        newStore: 0,
+        p: 0,
+        t: 0
+      });
+
+      const sent1 = await operation1.send();
+      const sent2 = await operation2.send();
+
+      await sent1.confirmation();
+      await sent2.confirmation();
 
       await sleep(3000);
 
       eventSub.close();
 
-      expect(data.length).toEqual(1);
+      expect(data.length).toEqual(2);
       expect(data[0].opHash).toBeDefined();
       expect(data[0].blockHash).toBeDefined();
       expect(data[0].level).toBeDefined();
       expect(data[0].kind).toEqual('event');
-      expect(data[0].source).toEqual(eventContractAddress);
-      expect(data[0].tag).toEqual('first');
+      expect(data[0].source).toEqual(mainContractAddress);
+      expect(data[0].tag).toEqual('intFromMainContract');
       expect(data[0].type).toBeDefined();
       expect(data[0].payload).toBeDefined();
       expect(data[0].result).toBeDefined();
 
       done();
     });
-  })
-})
+
+    it('should include events from failed operations when not filtering', async (done) => {
+      const data: any = [];
+
+      const eventSub = Tezos.stream.subscribeEvent({
+        address: mainContractAddress,
+      });
+
+      eventSub.on('error', (event) => {
+        data.push({
+          type: 'error',
+          event,
+        });
+      });
+
+      eventSub.on('data', (event) => {
+        data.push({
+          type: 'data',
+          event,
+        });
+      });
+
+      const contract1 = await Tezos.contract.at(mainContractAddress!);
+      const contract2 = await secondUser.contract.at(mainContractAddress!);
+      try {
+        const operation1 = contract1.methodsObject.default({
+          ad: calledContractAddress,
+          newStore: 1,
+          p: 0,
+          t: 0
+        });
+  
+        const operation2 = contract2.methodsObject.default({
+          ad: calledContractAddress,
+          newStore: 0,
+          p: 0,
+          t: 0
+        });
+  
+        const sent1 = await operation1.send();
+        const sent2 = await operation2.send();
+  
+        await sent1.confirmation();
+        await sent2.confirmation();
+      } catch (e) {
+        // Failure is expected
+      }
+
+      await sleep(5000);
+      eventSub.close();
+
+      expect(data.length).toEqual(3);
+      expect(data[1].event.result.status).toEqual('backtracked');
+      done();
+    });
+
+    it('should properly filter events from failed operations', async (done) => {
+      const data: any = [];
+
+      const eventSub = Tezos.stream.subscribeEvent({
+        address: mainContractAddress,
+        excludeFailedOperations: true,
+      });
+
+      eventSub.on('error', (event) => {
+        data.push({
+          type: 'error',
+          event,
+        });
+      });
+
+      eventSub.on('data', (event) => {
+        data.push({
+          type: 'data',
+          event,
+        });
+      });
+
+      const contract1 = await Tezos.contract.at(mainContractAddress!);
+      const contract2 = await secondUser.contract.at(mainContractAddress!);
+      try {
+        const operation1 = contract1.methodsObject.default({
+          ad: calledContractAddress,
+          newStore: 1,
+          p: 0,
+          t: 0
+        });
+  
+        const operation2 = contract2.methodsObject.default({
+          ad: calledContractAddress,
+          newStore: 0,
+          p: 0,
+          t: 0
+        });
+  
+        const sent1 = await operation1.send();
+        const sent2 = await operation2.send();
+  
+        await sent1.confirmation();
+        await sent2.confirmation();
+      } catch (e) {
+        // Failure is expected
+      }
+
+      await sleep(5000);
+      eventSub.close();
+
+      expect(data.length).toEqual(1);
+      done();
+    });
+  });
+});
diff --git a/packages/taquito/test/subscribe/filters.spec.ts b/packages/taquito/test/subscribe/filters.spec.ts
index bc26e9cd6..106239b9a 100644
--- a/packages/taquito/test/subscribe/filters.spec.ts
+++ b/packages/taquito/test/subscribe/filters.spec.ts
@@ -31,6 +31,36 @@ const mockInternalOperationResult = {
   },
 };
 
+const mockFailedInternalOperationResult = {
+  kind: 'event',
+  source: 'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
+  nonce: 2,
+  type: {
+    prim: 'or',
+    args: [
+      {
+        prim: 'nat',
+      },
+      {
+        prim: 'string',
+      },
+    ],
+  },
+  tag: 'test',
+  payload: {
+    prim: 'Left',
+    args: [
+      {
+        int: '10',
+      },
+    ],
+  },
+  result: {
+    status: 'failed',
+    consumed_milligas: '1000000',
+  },
+};
+
 const mockNonEventInternalOperationResult = {
   kind: 'transaction',
   source: 'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
@@ -139,6 +169,46 @@ describe('Event filter', () => {
     );
     expect(result).toBeFalsy();
   });
+
+  it('should return true for a failed operation when not filtering failed operations', () => {
+    const result = eventFilter(
+      mockFailedInternalOperationResult as InternalOperationResult,
+      'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
+      'test',
+    );
+    expect(result).toBeTruthy();
+  });
+
+  it('should return true for a failed operation when passing false to excludeFailedOperations', () => {
+    const result = eventFilter(
+      mockFailedInternalOperationResult as InternalOperationResult,
+      'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
+      'test',
+      false
+    );
+    expect(result).toBeTruthy();
+  });
+
+  it('should return true for a successful operation when passing true to excludeFailedOperations', () => {
+    const result = eventFilter(
+      mockInternalOperationResult as InternalOperationResult,
+      'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
+      'test',
+      true
+    );
+    expect(result).toBeTruthy();
+  });
+
+  it('should return false for a failed operation when passing true to excludeFailedOperations', () => {
+    const result = eventFilter(
+      mockFailedInternalOperationResult as InternalOperationResult,
+      'KT1TzvwJxn8dNHc8M2FLvTiUg6LwjUfC4X94',
+      'test',
+      true
+    );
+    expect(result).toBeFalsy();
+  });
+
 });
 
 describe('Evaluate expression', () => {