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

fix bug when 2+ events are returned in logs subscription #1049

Closed
wants to merge 3 commits into from

Conversation

ewingrj
Copy link
Contributor

@ewingrj ewingrj commented Sep 16, 2017

If 2 or more of the same event type are logged in a single transaction,
the eth_subscription result will contain multiple logs.

If 2 or more of the same event type are logged in a single transaction,
the eth_subscription result will contain multiple logs.
@ewingrj
Copy link
Contributor Author

ewingrj commented Sep 16, 2017

I'm wondering if it might be better for output to always be an array. Otherwise, clients will need to issue an array check in the subscription handler if there is a possibility for multiple events.

I can update the PR if you agree.

@frozeman
Copy link
Contributor

The subscription notification return should only return objects, i discussed that a long time ago with @bas-vk and assumed it is fixed.
If there are multiple logs, it should fire multiple notifications, one per log.

@frozeman frozeman added the Bug Addressing a bug label Sep 18, 2017
@frozeman
Copy link
Contributor

Supposedly geth will only return one log at a time, did you experience multiple logs, and if so with which version of geth? @perissology

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 85.182% when pulling 8fe0d33 on perissology:subscription_result_array into f5bc2ba on ethereum:1.0.

@bas-vk
Copy link
Contributor

bas-vk commented Sep 19, 2017

This was fixed more than a year ago.
If you still have this issue with a recent version of geth it would help if you could provide an example.

@coveralls
Copy link

coveralls commented Sep 19, 2017

Coverage Status

Coverage decreased (-0.2%) to 85.135% when pulling 7e469ff on perissology:subscription_result_array into f3fc88a on ethereum:1.0.

@ewingrj
Copy link
Contributor Author

ewingrj commented Sep 20, 2017

ah sorry, I was actually using parity parity_subscription v1.7.0-beta-5f2cabd-20170727.

not sure about geths eth_subscription

@frozeman frozeman closed this Sep 25, 2017
@ewingrj
Copy link
Contributor Author

ewingrj commented Sep 25, 2017

so is this a parity bug then?

@ewingrj
Copy link
Contributor Author

ewingrj commented Sep 25, 2017

@frozeman ^

@frozeman
Copy link
Contributor

They will fix that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants