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

draft hotfix for chain halt #4995

Closed
wants to merge 1 commit into from
Closed

Conversation

hdevalence
Copy link
Member

FOR REVIEW

Commit posted for testing, commit message + description TK

Describe your changes

Issue ticket number and link

Checklist before requesting a review

  • I have added guiding text to explain how a reviewer should test these changes.

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    REPLACE THIS TEXT WITH RATIONALE (CAN BE BRIEF)

Commit posted for testing, commit message + description TK
"attempted to close a position with state {:?}, expected Opened or Closed",
prev_state.state
);
if prev_state.state == position::State::Opened {
Copy link
Member

Choose a reason for hiding this comment

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

ACK, inner logic gated on valid state antecedent.

if prev_state.state == position::State::Closed {
self.update_position(id, Some(prev_state), new_state)
.await?;
self.record_proto(event::EventPositionClose { position_id: *id }.to_proto());
Copy link
Member

Choose a reason for hiding this comment

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

ACK. no change to the inner logic block.

tracing::debug!(
?id,
"position is already closed so we can skip state updates"
?prev_state.state,
"position is not currently open, skipping"
Copy link
Member

Choose a reason for hiding this comment

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

ACK. short-circuit.

@conorsch
Copy link
Contributor

Closing in favor of #4994.

@conorsch conorsch closed this Jan 18, 2025
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.

3 participants