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

x/participationrewards/keeper: Keeper.AfterEpochEnd has potential non-determinism from running k.prSubmodules.each.Hook from a map, if the order of the hooks matters #1338

Open
1 of 3 tasks
odeke-em opened this issue Mar 26, 2024 · 0 comments

Comments

@odeke-em
Copy link
Contributor

Summary of Bug

If we examine this code

for _, sub := range k.prSubmodules {
sub.Hooks(ctx, k)
}

we can infer from the definition

prSubmodules map[cmtypes.ClaimType]Submodule
that for the various claim types we have different submodules and run a hook.

The question is does it matter which the order that those hooks run? If so, we've got non-determinism here and that cause subtle behavior and in much more serious cases chain halts

Expected Behaviour

The simplest way to fix this would be to invoke claimTypes := maps.Keys(k.prSubmodules) + slice.Sort(claimTypes) and then using those sorted keys to iterate and infer the hook from the map; that'll maintain a deterministic sorted order for iterating over and applying those the exact same way for every single node/validator

/cc @faddat @joe-bowman


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
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

No branches or pull requests

1 participant