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: use current exec state when querying validator table #1234

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

karlem
Copy link
Contributor

@karlem karlem commented Dec 15, 2024

Close #1233

The issue has been resolved by using the current execution state instead of the previous state retrieved through the read-only view. This ensures that the validators cache accurately reflects the latest set of validators, preventing the panic.

@karlem karlem requested a review from a team as a code owner December 15, 2024 20:43
@cryptoAtwill
Copy link
Contributor

@karlem Somehow the first impression that strikes is shouldnt we try to update the read_only_view to reflect latest state instead of passing mutable ref of state to a read only function?

Copy link
Contributor

@raulk raulk left a comment

Choose a reason for hiding this comment

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

@cryptoAtwill Unfortunately the FVM executor does not expose read-only semantics through its execute_message API.

So we'd have to add a method to "fork" the executor/machine using the intermediate state tree root (which you can get from machine.state_tree()) and a BufferedBlockstore over the original Blockstore (machine.blockstore()), which we discard after we finish the query.

Alternatively, we could forego this state query entirely and:

  1. Guarantee we have always have a fully initialized ValidatorCache at the start.
  2. Merge the power updates onto our ValidatorCache.

@raulk raulk changed the title fix: use current exec state fix: use current exec state when querying validator table Dec 17, 2024
Copy link
Contributor

@raulk raulk left a comment

Choose a reason for hiding this comment

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

Merging this and opening #1236 to fix the right way.

@raulk raulk merged commit 8f88bb7 into main Dec 17, 2024
16 checks passed
@raulk raulk deleted the fix-validator-cache-refresh branch December 17, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Panic During Execution: Validator Not Found in Validators Cache
3 participants