Panic in handle_batch_swaps involving ValueCircuitBreaker
Description
The call to handle_batch_swaps in Dex::end_block panics when given an open position and two swaps (one in each direction) for that position that seem to exercise some rounding-related behavior. The trading coefficients of the position imply an exchange rate of 1 of A for 1.2 of B, and a swap of 1 A for the current market price of B and a swap of 2 B for the current market price of A is given. This is minimized from an example where the implied exchange rate is approximately 22.9 ({ p: 9726, q: 425 }), and the swaps provide 1 A and 23 Bs.
TxCmd::SwapTest {} => {
use penumbra_dex::{DirectedTradingPair, lp::{Reserves, position::Position}};
let source = 0;
let fvk = app.config.full_viewing_key.clone();
let (claim_address, _dtk_d) =
fvk.incoming().payment_address(AddressIndex::new(source));
let cube = Value::from_str("1cube")?;
let pizza = Value::from_str("1pizza")?;
let pos = Position::new(OsRng, DirectedTradingPair::new(cube.asset_id, pizza.asset_id),
0, Amount::from(6u64), Amount::from(5u64),
Reserves { r1: Amount::from(0u64), r2: Amount::from(1u64), }
);
let plan = Planner::new(OsRng)
.set_gas_prices(gas_prices)
.set_fee_tier(FeeTier::Low.into())
.position_open(pos)
.swap(Value::from_str("1cube")?, pizza.asset_id, Fee::default(), claim_address)?
.swap(Value::from_str("2pizza")?, cube.asset_id, Fee::default(), claim_address)?
.plan(
app.view
.as_mut()
.context("view service must be initialized")?,
AddressIndex::new(source),
)
.await?;
app.build_and_submit_transaction(plan).await?;
}The relevant excerpt of the stack trace is
thread 'tokio-runtime-worker' panicked at /src/crates/core/component/d
ex/src/component/router/route_and_fill.rs:128:9:
asset 1 outflow exceeds available balance
stack backtrace:
0: rust_begin_unwind
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/std/src/p
anicking.rs:645:5
1: core::panicking::panic_fmt
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/
panicking.rs:72:14
2: penumbra_dex::component::router::route_and_fill::HandleBatchSwaps
::handle_batch_swaps::{{closure}}::{{closure}}
at ./crates/core/component/dex/src/component/router/route_and_fill.r
s:128:9
3: <tracing::instrument::Instrumented<T> as core::future::future::Futu
re>::poll
at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tr
acing-0.1.40/src/instrument.rs:321:9
4: penumbra_dex::component::router::route_and_fill::HandleBatchSwaps
::handle_batch_swaps::{{closure}}
at ./crates/core/component/dex/src/component/router/route_and_fill.r
s:27:5
5: <core::pin::Pin<P> as core::future::future::Future>::poll
at /rustc/82e1608dfa6e0b5569232559e3d385fea5a93112/library/core/src/
future/future.rs:125:9
6: <penumbra_dex::component::dex::Dex as cnidarium_component::componen
t::Component>::end_block::{{closure}}::{{closure}}
at ./crates/core/component/dex/src/component/dex.rs:64:18
7: <tracing::instrument::Instrumented<T> as core::future::future::Futu
re>::poll
at /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/tr
acing-0.1.40/src/instrument.rs:321:9
8: <penumbra_dex::component::dex::Dex as cnidarium_component::componen
t::Component>::end_block::{{closure}}
at ./crates/core/component/dex/src/component/dex.rs:39:5The patch that adds the property tests that found this and Finding ref↗ were provided to the client.
Impact
Causing a panic in end_block will halt the chain, causing a denial of service. This panic is easy to trigger with a modified PCLI command, requiring one token of any kind, and two tokens of any other kind distinct from the first. Additionally, since the panic is downstream of the root cause, it might be possible that if other positions are open, the value circuit breaker may not catch similar rounding issues, resulting in swaps happening at incorrect prices or value being extracted from the DEX.
Recommendations
The root cause of why either more value is getting swapped than is available or why the value circuit breaker is underestimating balance should be fixed.
Remediation
This issue has been acknowledged by Penumbra Labs, and a fix was implemented in commit 46a82a5f↗.