Improve update_position API and position execution events.
#4121
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit changes `update_position` so that the caller is responsible for fetching the previous state and then passing it in. While this adds misuse potential into the API, the method is only used by the `PositionManager` and it allows the high-level modification methods to do their own invariant checks without blocking on extra reads.
This commit improves event handling for PositionExecution: - we add the previous balances to the event - we add a `context` to the event that allows connecting the micro-scale execution with a high-level context - we fix a bug where events were emitted multiple times - we avoid emitting events for no-op executions.
hdevalence
commented
Mar 27, 2024
hdevalence
commented
Mar 27, 2024
erwanor
approved these changes
Mar 27, 2024
| } | ||
|
|
||
| // Update the available liquidity for this position's trading pair. | ||
| self.update_available_liquidity(&position, &prev).await?; | ||
| self.update_available_liquidity(&new_state, &prev_state) |
There was a problem hiding this comment.
This PR actually improves on the previous impl because prev_state is Option<Position> but noting that the parent method signature is:
async fn update_position(
&mut self,
prev_state: Option<Position>,
new_state: Position,
) -> Result<()>;And this method is:
async fn update_available_liquidity(
&mut self,
position: &Position,
prev_position: &Option<Position>,
) -> Result<()>;but since the types are different this is probably fine
There was a problem hiding this comment.
I'd rather streamline that code in a later PR that implements eviction, but good spotting. Added a TODO.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I believe that both of these changes are not state-breaking or consensus-breaking but would appreciate review of that.
Closes #3844 (cc @philipjames44)