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

Adds event searching. #618

Closed
wants to merge 1 commit into from
Closed

Adds event searching. #618

wants to merge 1 commit into from

Conversation

calina-c
Copy link
Contributor

Closes #617.

@calina-c calina-c requested a review from alexcos20 March 14, 2023 12:30
@calina-c calina-c marked this pull request as ready for review March 14, 2023 13:10
Copy link
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are equivalent in terms of results, and existing version is more concise
So we don't need this

if multiple events exists in a tx, this will return only the last one. This is wrong

@calina-c
Copy link
Contributor Author

they are equivalent in terms of results, and existing version is more concise So we don't need this if multiple events exists in a tx, this will return only the last one. This is wrong

Yes, that's why I'm proposing it as a temporary solution. It is better than the previous version, where the first event, regardless of type, was checked. This one at least targets the correct type. We can discuss a better filtering strategy when you come back from holiday.

@alexcos20
Copy link
Member

provider_fee_event_logs = datatoken_contract.events.ProviderFee().processReceipt( tx_receipt, errors=DISCARD )
This returns all events of type 'ProviderFee' from tx_receipt, so we have them all, no matter if it's only one or one hundred.
It will not store in provider_fee_event_logs events of type 'OrderStarted' for instance.

Taking either the first or the last is wrong.
We need to process them all and see which one is the one we are looking for.

@calina-c calina-c closed this Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect event validation for download/compute
2 participants