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

Remove incorrect implementation of DataStoreReadOps #3160

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

hrxi
Copy link
Contributor

@hrxi hrxi commented Nov 26, 2024

Previously, the function would try to block the whole thread whenever requesting something from other peers.

This wasn't noticed since the implementation is only used in light clients that were NOT compiled for WASM.

I noticed this implementation when reviewing #3159, which would have caused these thread blockings to continue for longer times.

@hrxi hrxi force-pushed the hrxi/remote_data_store_block_on branch from 8e7b270 to d72bdc5 Compare November 26, 2024 22:46
hrxi added 2 commits November 27, 2024 01:10
It's almost always incorrect to use it.
Previously, the function would try to block the whole thread whenever
requesting something from other peers.

This wasn't noticed since the implementation is only used in light
clients that were **NOT** compiled for WASM.

I noticed this implementation when reviewing #3159, which would have
caused these thread blockings to continue for longer times.
@hrxi hrxi force-pushed the hrxi/remote_data_store_block_on branch from 1617f7a to b4b0ff2 Compare November 27, 2024 00:10
It has no WASM dependencies and is not broken unlike the thing it
replaces.
@hrxi hrxi force-pushed the hrxi/remote_data_store_block_on branch from b4b0ff2 to eca46dc Compare November 27, 2024 00:11
@jsdanielh jsdanielh merged commit eca46dc into albatross Nov 27, 2024
8 checks passed
@jsdanielh jsdanielh deleted the hrxi/remote_data_store_block_on branch November 27, 2024 20:08
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.

2 participants