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

Scoped EVM queue ID class #2537

Merged
merged 10 commits into from
Oct 9, 2023
Merged

Scoped EVM queue ID class #2537

merged 10 commits into from
Oct 9, 2023

Conversation

Bushstar
Copy link
Member

@Bushstar Bushstar commented Oct 5, 2023

Summary

  • Creates a class that will create and destroy an EVM queue ID
  • Only create a queue if EVM is enabled, allows the replacement, where applicable, of the isEvmEnabledForBlock check with a valid EVM queue ID check

Implications

  • Storage

    • Database reindex required
    • Database reindex optional
    • Database reindex not required
    • None
  • Consensus

    • Network upgrade required
    • Includes backward compatible changes
    • Includes consensus workarounds
    • Includes consensus refactors
    • None

@prasannavl
Copy link
Member

prasannavl commented Oct 7, 2023

@Bushstar nice work. I think we can do better though.

I think this might have complicated things a bit with isValid, as well as checking the count manually; and what happens when the count isn't.

However, this might be perfect for shared_ptr itself, who's whole goal is to keep track of these and destroy when everything goes out of scope automatically. By wrapping it and adding a destructor on top, I think we've actually increased complexity than simplify it.

Rather, how about, create a simple scope wrapper, who's destructor will always destroy the queue. Now, we keep track of that in the shared_ptr and pass the shared_ptr around. So, it has to be:

shared_ptr<ScopedCQueue> -- this automatically reference tracks the queue, and destroys it when all references go out of scope.

sieniven
sieniven previously approved these changes Oct 9, 2023
@Bushstar Bushstar merged commit d9ebc16 into master Oct 9, 2023
@Bushstar Bushstar deleted the bush/cscopedqueueid branch October 9, 2023 10:12
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.

3 participants