Skip to content

Commit

Permalink
dex: hotfix for 2025-01-18 chain halt (alternative) (#4994)
Browse files Browse the repository at this point in the history
## 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)
  • Loading branch information
hdevalence authored and conorsch committed Jan 19, 2025
1 parent 4f39372 commit 7a65c6a
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use anyhow::Result;
use async_trait::async_trait;
use cnidarium::StateWrite;
use cnidarium_component::ActionHandler;
use penumbra_proto::{DomainType as _, StateWriteProto as _};

use crate::{component::PositionManager, event, lp::action::PositionClose};
use crate::{component::PositionManager, lp::action::PositionClose};

#[async_trait]
/// Debits an opened position NFT and credits a closed position NFT.
Expand All @@ -22,15 +21,7 @@ impl ActionHandler for PositionClose {
// lose the ability to do block-scoped JIT liquidity, where a single
// transaction opens and closes a position, keeping liquidity live only
// during that block's batch swap execution.
state.queue_close_position(self.position_id);

// queue position close you will...
state.record_proto(
event::EventQueuePositionClose {
position_id: self.position_id,
}
.to_proto(),
);
state.queue_close_position(self.position_id).await?;

Ok(())
}
Expand Down
35 changes: 31 additions & 4 deletions crates/core/component/dex/src/component/position_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,37 @@ pub trait PositionManager: StateWrite + PositionRead {
}

/// Queues a position to be closed at the end of the block, after batch execution.
fn queue_close_position(&mut self, id: position::Id) {
let mut to_close = self.pending_position_closures();
to_close.push_back(id);
self.object_put(state_key::pending_position_closures(), to_close);
async fn queue_close_position(&mut self, id: position::Id) -> Result<()> {
tracing::debug!(
?id,
"checking current position state before queueing for closure"
);
let current_state = self
.position_by_id(&id)
.await
.expect("fetching position should not fail")
.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 {
tracing::debug!(
?current_state.state,
"queueing opened position for closure"
);
let mut to_close = self.pending_position_closures();
to_close.push_back(id);
self.object_put(state_key::pending_position_closures(), to_close);

// queue position close you will...
self.record_proto(event::EventQueuePositionClose { position_id: id }.to_proto());
} else {
tracing::debug!(
?current_state.state,
"skipping queueing for closure of non-opened position"
);
}

Ok(())
}

/// Close all positions that have been queued for closure.
Expand Down

0 comments on commit 7a65c6a

Please sign in to comment.