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

dex: hotfix for 2025-01-18 chain halt (alternative) #4994

Merged
merged 1 commit into from
Jan 18, 2025

Conversation

hdevalence
Copy link
Member

@hdevalence hdevalence commented Jan 18, 2025

Describe your changes

This commit fixes the chain halt that occurred on 2025-01-18 at block 3093519.

What happened (short version):

A transaction was submitted that contained both PositionClose and PositionWithdraw actions for the same position P, which had been auto-closed by the DEX. In DeliverTx, the PositionClose action queued the position for closure by ID, and the PositionWithdraw action checked that the position state was closed and then updated it to Withdrawn. Later, in EndBlock, the queued position closure was executed. The position state was not Opened or Closed, triggering an assertion.

Why it happened

  • Mempool tx checking does not execute end block so this didn't get filtered out
  • The position closure method allows the position closure to be a no-op, but only for positions in the closed state
  • Because the position was withdrawn, the state was unexpected

What this does

Avoid queueing the position for closure unless it is Opened.

Commit message and description TK
}
.to_proto(),
);
state.queue_close_position(self.position_id).await?;
Copy link
Member

Choose a reason for hiding this comment

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

ACK.

.ok_or_else(|| anyhow::anyhow!("could not find position {} to close", id))?
.tap(|lp| tracing::trace!(prev_state = ?lp, "retrieved previous lp state"));

if current_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. short-circuit on queueing.

);
let mut to_close = self.pending_position_closures();
to_close.push_back(id);
self.object_put(state_key::pending_position_closures(), to_close);
Copy link
Member

Choose a reason for hiding this comment

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

ACK. queuing gated on valid state antecedent.

// queue position close you will...
self.record_proto(event::EventQueuePositionClose { position_id: id }.to_proto());
} else {
tracing::debug!(
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-op on contentious state antecedent.

@hdevalence hdevalence marked this pull request as ready for review January 18, 2025 22:00
@conorsch conorsch mentioned this pull request Jan 18, 2025
2 tasks
@conorsch conorsch merged commit f7f780d into release/v0.81.x Jan 18, 2025
11 of 12 checks passed
@conorsch conorsch deleted the hotfix-2025-01-18-alt branch January 18, 2025 22:02
conorsch pushed a commit that referenced this pull request Jan 19, 2025
## Describe your changes

This commit fixes the chain halt that occurred on 2025-01-18 at block
3093519.

### What happened (short version):

A transaction was submitted that contained both `PositionClose` and
`PositionWithdraw` actions for the same position `P`, which had been
auto-closed by the DEX. In `DeliverTx`, the `PositionClose` action
queued the position for closure by ID, and the `PositionWithdraw` action
checked that the position state was closed and then updated it to
Withdrawn. Later, in `EndBlock`, the queued position closure was
executed. The position state was not Opened or Closed, triggering an
assertion.

### Why it happened

- Mempool tx checking does not execute end block so this didn't get
filtered out
- The position closure method allows the position closure to be a no-op,
but only for positions in the closed state
- Because the position was withdrawn, the state was unexpected

### What this does

Avoid queueing the position for closure unless it is Opened.

(cherry picked from commit f7f780d)
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