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 SimulateActions State Keys #1918

Merged
merged 2 commits into from
Feb 5, 2025
Merged

Fix SimulateActions State Keys #1918

merged 2 commits into from
Feb 5, 2025

Conversation

RodrigoVillar
Copy link
Contributor

@RodrigoVillar RodrigoVillar commented Feb 5, 2025

This PR fixes a bug relating to the state keys being returned by the SimulateActions API method.

Currently, SimulateActions takes in a slice of actions and executes them one-by-one, keeping track of the state key each action touches via the use of scope:

scope := state.SimulatedKeys{}

After each action is executed, the state keys that it touched are stored in its respective result and the for-loop moves on to the next action. Before moving onto the next iteration, scope is cleared. Since the underlying type of state.SimulatedKeys is a map (and therefore, a pointer), the recorded state keys of all previous actions is wiped, which should not be the case.

This PR fixes the bug above by using maps.Clone to copy the KV-pairs of scope into the actions recorded state keys.

@RodrigoVillar RodrigoVillar self-assigned this Feb 5, 2025
@RodrigoVillar RodrigoVillar marked this pull request as draft February 5, 2025 19:52
@RodrigoVillar RodrigoVillar marked this pull request as ready for review February 5, 2025 20:02
@@ -330,7 +331,7 @@ func (j *JSONRPCServer) SimulateActions(
if err != nil {
return err
}
actionResult.StateKeys = scope.StateKeys()
actionResult.StateKeys = maps.Clone(scope.StateKeys())
Copy link
Collaborator

Choose a reason for hiding this comment

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

(separate from this PR) - could we add a regression test to this API?

@aaronbuchwald aaronbuchwald merged commit 9d4618a into main Feb 5, 2025
9 checks passed
@aaronbuchwald aaronbuchwald deleted the fix-simulateactions branch February 5, 2025 22: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.

2 participants