From b2e912c73c2627434f32f29e5087e60cb436a0c6 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Wed, 3 Apr 2024 14:54:05 -0400 Subject: [PATCH] address comments --- .../astria-sequencer/src/service/consensus.rs | 2 +- .../astria-sequencer/src/service/mempool.rs | 22 +++++++++---------- .../astria-sequencer/src/transaction/mod.rs | 14 ++++++++---- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 3a5ed26a1b..0906993e60 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -245,7 +245,7 @@ impl Consensus { response::DeliverTx { code: code.into(), info: code.to_string(), - log: format!("{e:?}"), + log: e.to_string(), ..Default::default() } } diff --git a/crates/astria-sequencer/src/service/mempool.rs b/crates/astria-sequencer/src/service/mempool.rs index e8041c3925..4ab349f32b 100644 --- a/crates/astria-sequencer/src/service/mempool.rs +++ b/crates/astria-sequencer/src/service/mempool.rs @@ -79,12 +79,12 @@ impl Service for Mempool { } } -// Handles a [`request::CheckTx`] request. -// -// Performs stateless checks (decoding and signature check), -// as well as stateful checks (nonce and balance checks). -// -// If the tx passes all checks, status code 0 is returned. +/// Handles a [`request::CheckTx`] request. +/// +/// Performs stateless checks (decoding and signature check), +/// as well as stateful checks (nonce and balance checks). +/// +/// If the tx passes all checks, status code 0 is returned. async fn handle_check_tx( req: request::CheckTx, state: S, @@ -109,7 +109,7 @@ async fn handle_check_tx( Err(e) => { return response::CheckTx { code: AbciErrorCode::INVALID_PARAMETER.into(), - log: format!("{e:?}"), + log: e.to_string(), info: "failed decoding bytes as a protobuf SignedTransaction".into(), ..response::CheckTx::default() }; @@ -123,7 +123,7 @@ async fn handle_check_tx( info: "the provided bytes was not a valid protobuf-encoded SignedTransaction, or \ the signature was invalid" .into(), - log: format!("{e:?}"), + log: e.to_string(), ..response::CheckTx::default() }; } @@ -133,7 +133,7 @@ async fn handle_check_tx( return response::CheckTx { code: AbciErrorCode::INVALID_PARAMETER.into(), info: "transaction failed stateless check".into(), - log: format!("{e:?}"), + log: e.to_string(), ..response::CheckTx::default() }; }; @@ -142,7 +142,7 @@ async fn handle_check_tx( return response::CheckTx { code: AbciErrorCode::INVALID_NONCE.into(), info: "failed verifying transaction nonce".into(), - log: format!("{e:?}"), + log: e.to_string(), ..response::CheckTx::default() }; }; @@ -151,7 +151,7 @@ async fn handle_check_tx( return response::CheckTx { code: AbciErrorCode::INSUFFICIENT_FUNDS.into(), info: "failed verifying account balance".into(), - log: format!("{e:?}"), + log: e.to_string(), ..response::CheckTx::default() }; }; diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index d2bc39c129..05d9aa8e9b 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -103,7 +103,12 @@ pub(crate) async fn check_balance_mempool( .and_modify(|amt| *amt += TRANSFER_FEE) .or_insert(TRANSFER_FEE); } - _ => { + Action::ValidatorUpdate(_) + | Action::SudoAddressChange(_) + | Action::Ibc(_) + | Action::IbcRelayerChange(_) + | Action::FeeAssetChange(_) + | Action::Mint(_) => { continue; } } @@ -460,7 +465,7 @@ mod test { } #[tokio::test] - async fn check_balance_mempool_insuffient_other_asset_balance() { + async fn check_balance_mempool_insufficient_other_asset_balance() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); @@ -501,8 +506,9 @@ mod test { }; let signed_tx = tx.into_signed(&alice_signing_key); - check_balance_mempool(&signed_tx, &state_tx) + let err = check_balance_mempool(&signed_tx, &state_tx) .await - .expect_err("insuffient funds for `other` asset"); + .expect_err("insufficient funds for `other` asset"); + assert!(err.to_string().contains(&other_asset.to_string())); } }