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

feat: fix re-entrancy in strategies #801

Merged
merged 6 commits into from
Dec 23, 2024
Merged

feat: fix re-entrancy in strategies #801

merged 6 commits into from
Dec 23, 2024

Conversation

nbaztec
Copy link
Collaborator

@nbaztec nbaztec commented Dec 21, 2024

What 💻

  • Fixes re-entrancy issue by separating the context from the strategy.
  • Strategy is split into Runner and Context, which can be partially borrowed.
  • Runner implements all methods with immutable &self and takes the &mut Context to mutate.
  • Context is implemented via Any and is downcastable and specific to each implementation.
  • In places where self parameter is passed into the strategy objects:
    • &mut Context may be omitted as it's available via self.strategy.context.
    • The stateless self.strategy.runner can be cloned and dissociated from self: self.strategy.runner.clone().foo(self).

Why ✋

  • Required for re-entrancy

Evidence 📷

Tests pass

@nbaztec nbaztec requested a review from a team as a code owner December 21, 2024 21:04
@nbaztec nbaztec changed the title feat: fix re-entrancy feat: fix re-entrancy in strategies Dec 22, 2024
@nbaztec nbaztec merged commit 6234614 into main Dec 23, 2024
23 checks passed
@nbaztec nbaztec deleted the nish-fix-reentrancy branch December 23, 2024 13:17
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