Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events are fired even if the transaction fails #2319

Closed
zamrokk opened this issue Jan 25, 2023 · 4 comments · Fixed by #2388
Closed

Events are fired even if the transaction fails #2319

zamrokk opened this issue Jan 25, 2023 · 4 comments · Fixed by #2388
Assignees
Labels
bug Something isn't working
Projects
Milestone

Comments

@zamrokk
Copy link

zamrokk commented Jan 25, 2023

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

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

@zamrokk zamrokk added the bug Something isn't working label Jan 25, 2023
@dsawali
Copy link
Collaborator

dsawali commented Jan 25, 2023

thanks for submitting this issue, yeah I think we can probably do something like filter the result of the subscribed event by the result status and put that as an option for our users. cc: @zamrokk

@Innkst Innkst added this to To do in dev via automation Jan 30, 2023
@Innkst Innkst added this to the v16.1 milestone Jan 30, 2023
@ac10n
Copy link
Collaborator

ac10n commented Mar 23, 2023

@zamrokk
We are addressing this in #2388 .

However, when writing the integration tests I'm unable to reproduce the behavior when a contract call fails but the event is shown.
Do you have some reproduction guide (including the contract code)?

Thanks

@zamrokk
Copy link
Author

zamrokk commented Mar 29, 2023

yes, there is a way to reproduce.

Do this training :) => https://github.com/marigold-dev/training-dapp-shifumi

On play function (https://github.com/marigold-dev/training-dapp-shifumi/blob/main/solution/app/src/pages/SessionScreen.tsx), look at the part in taquito where I override the default gas

 const op = await preparedCall.send({
        gasLimit: gasLimit + 1000, //we take a margin +100 for an eventual event in case of paralell execution

and remove the 1000

Then run the game on 2 browsers and click on a shifumi action (paper,stone,scissor) at same time on both windows. You can look on tzkt that quite often one of the transaction will be failing and the event will be emitted anyway

FYI : The cause of the failure is a bug on the cost estimator on Tezos code about event gas cost not taken into account. Ticket is open on it

@zamrokk
Copy link
Author

zamrokk commented Mar 29, 2023

FYI : same for reveal function, there is same bug and gas override

@Innkst Innkst moved this from To do to In progress in dev Mar 29, 2023
@Innkst Innkst modified the milestones: v16.1, v16.2 Apr 11, 2023
@ac10n ac10n moved this from In progress to Review in progress in dev Apr 19, 2023
dev automation moved this from Review in progress to Done Apr 19, 2023
@ac10n ac10n moved this from Done to Review approved in dev Apr 19, 2023
@ac10n ac10n moved this from Review approved to Done in dev Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
dev
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants