From 37f9e3841bf62de5ad51d12e732ffb87975b1539 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 25 Oct 2024 15:25:13 +0200 Subject: [PATCH 1/3] replace all query routes by constants --- .../src/service/info/abci_query_router.rs | 15 +++- .../astria-sequencer/src/service/info/mod.rs | 69 ++++++++----------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/crates/astria-sequencer/src/service/info/abci_query_router.rs b/crates/astria-sequencer/src/service/info/abci_query_router.rs index 988e0b7791..00a127772d 100644 --- a/crates/astria-sequencer/src/service/info/abci_query_router.rs +++ b/crates/astria-sequencer/src/service/info/abci_query_router.rs @@ -40,7 +40,6 @@ use std::{ use cnidarium::Storage; use matchit::{ - InsertError, Match, MatchError, }; @@ -49,6 +48,13 @@ use tendermint::abci::{ response, }; +#[derive(Debug, thiserror::Error)] +#[error("`{route}` is an invalid route")] +pub(crate) struct InsertError { + route: String, + source: matchit::InsertError, +} + /// `Router` is a wrapper around [`matchit::Router`] to route abci queries /// to handlers. #[derive(Clone)] @@ -75,8 +81,13 @@ impl Router { route: impl Into, handler: impl AbciQueryHandler, ) -> Result<(), InsertError> { + let route = route.into(); self.query_router - .insert(route, BoxedAbciQueryHandler::from_handler(handler)) + .insert(route.clone(), BoxedAbciQueryHandler::from_handler(handler)) + .map_err(|source| InsertError { + route, + source, + }) } } diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 07d71015f2..45588da934 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -45,48 +45,37 @@ pub(crate) struct Info { query_router: abci_query_router::Router, } +const ACCOUNT_BALANCE: &str = "accounts/balance/:account"; +const ACCOUNT_NONCE: &str = "accounts/nonce/:account"; +const ASSET_DENOM: &str = "asset/denom/:id"; +const FEE_ALLOWED_ASSETS: &str = "asset/allowed_fee_assets"; + +const BRIDGE_ACCOUNT_LAST_TX_ID: &str = "bridge/account_last_tx_hash/:address"; +const BRIDGE_ACCOUNT_INFO: &str = "bridge/account_info/:address"; + +const TRANSACTION_FEE: &str = "transaction/fee"; + impl Info { pub(crate) fn new(storage: Storage) -> Result { let mut query_router = abci_query_router::Router::new(); - query_router - .insert( - "accounts/balance/:account", - crate::accounts::query::balance_request, - ) - .wrap_err("invalid path: `accounts/balance/:account`")?; - query_router - .insert( - "accounts/nonce/:account", - crate::accounts::query::nonce_request, - ) - .wrap_err("invalid path: `accounts/nonce/:account`")?; - query_router - .insert("asset/denom/:id", crate::assets::query::denom_request) - .wrap_err("invalid path: `asset/denom/:id`")?; - query_router - .insert( - "asset/allowed_fee_assets", - crate::fees::query::allowed_fee_assets_request, - ) - .wrap_err("invalid path: `asset/allowed_fee_asset_ids`")?; - query_router - .insert( - "bridge/account_last_tx_hash/:address", - crate::bridge::query::bridge_account_last_tx_hash_request, - ) - .wrap_err("invalid path: `bridge/account_last_tx_hash/:address`")?; - query_router - .insert( - "transaction/fee", - crate::fees::query::transaction_fee_request, - ) - .wrap_err("invalid path: `transaction/fee`")?; - query_router - .insert( - "bridge/account_info/:address", - crate::bridge::query::bridge_account_info_request, - ) - .wrap_err("invalid path: `bridge/account_info/:address`")?; + + // NOTE: Skipping error context because `InsertError` contains all required information. + query_router.insert(ACCOUNT_BALANCE, crate::accounts::query::balance_request)?; + query_router.insert(ACCOUNT_NONCE, crate::accounts::query::nonce_request)?; + query_router.insert(ASSET_DENOM, crate::assets::query::denom_request)?; + query_router.insert( + FEE_ALLOWED_ASSETS, + crate::fees::query::allowed_fee_assets_request, + )?; + query_router.insert( + BRIDGE_ACCOUNT_LAST_TX_ID, + crate::bridge::query::bridge_account_last_tx_hash_request, + )?; + query_router.insert( + BRIDGE_ACCOUNT_INFO, + crate::bridge::query::bridge_account_info_request, + )?; + query_router.insert(TRANSACTION_FEE, crate::fees::query::transaction_fee_request)?; Ok(Self { storage, query_router, @@ -366,7 +355,7 @@ mod tests { InfoResponse::Query(query) => query, other => panic!("expected InfoResponse::Query, got {other:?}"), }; - assert!(query_response.code.is_ok()); + assert!(query_response.code.is_ok(), "{query_response:?}"); let allowed_fee_assets_resp = raw::AllowedFeeAssetsResponse::decode(query_response.value) .unwrap() From 2b6ba391054a8d9ff4806c79fd787472b40a3b10 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 25 Oct 2024 17:55:38 +0200 Subject: [PATCH 2/3] feat(sequencer): allow querying fee components --- Cargo.lock | 1 + crates/astria-sequencer/Cargo.toml | 1 + crates/astria-sequencer/src/fees/query.rs | 174 +++++++++++++++ .../astria-sequencer/src/service/info/mod.rs | 210 +++++++++++++++++- 4 files changed, 385 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 7503e07525..2ef955a9c4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -794,6 +794,7 @@ dependencies = [ name = "astria-sequencer" version = "1.0.0-rc.2" dependencies = [ + "assert-json-diff", "astria-build-info", "astria-config", "astria-core", diff --git a/crates/astria-sequencer/Cargo.toml b/crates/astria-sequencer/Cargo.toml index ca1c440af3..e5dd50a625 100644 --- a/crates/astria-sequencer/Cargo.toml +++ b/crates/astria-sequencer/Cargo.toml @@ -81,6 +81,7 @@ paste = "1.0.15" maplit = "1.0.2" rand_chacha = "0.3.1" tokio = { workspace = true, features = ["test-util"] } +assert-json-diff = "2.0.2" [build-dependencies] astria-build-info = { path = "../astria-build-info", features = ["build"] } diff --git a/crates/astria-sequencer/src/fees/query.rs b/crates/astria-sequencer/src/fees/query.rs index 6342843668..04600de6c3 100644 --- a/crates/astria-sequencer/src/fees/query.rs +++ b/crates/astria-sequencer/src/fees/query.rs @@ -42,6 +42,7 @@ use tendermint::abci::{ Code, }; use tokio::{ + join, sync::OnceCell, try_join, }; @@ -134,6 +135,44 @@ pub(crate) async fn allowed_fee_assets_request( } } +pub(crate) async fn components( + storage: Storage, + request: request::Query, + _params: Vec<(String, String)>, +) -> response::Query { + let snapshot = storage.latest_snapshot(); + + let height = async { + snapshot + .get_block_height() + .await + .wrap_err("failed getting block height") + }; + let fee_components = get_all_fee_components(&snapshot).map(Ok); + let (height, fee_components) = match try_join!(height, fee_components) { + Ok(vals) => vals, + Err(err) => { + return response::Query { + code: Code::Err(AbciErrorCode::INTERNAL_ERROR.value()), + info: AbciErrorCode::INTERNAL_ERROR.info(), + log: format!("{err:#}"), + ..response::Query::default() + }; + } + }; + + let height = tendermint::block::Height::try_from(height).expect("height must fit into an i64"); + response::Query { + code: tendermint::abci::Code::Ok, + key: request.path.into_bytes().into(), + value: serde_json::to_vec(&fee_components) + .expect("object does not contain keys that don't map to json keys") + .into(), + height, + ..response::Query::default() + } +} + pub(crate) async fn transaction_fee_request( storage: Storage, request: request::Query, @@ -458,3 +497,138 @@ fn preprocess_fees_request(request: &request::Query) -> Result From>> for FetchResult +where + FeeComponent: From, +{ + fn from(value: eyre::Result>) -> Self { + match value { + Ok(Some(val)) => Self::Component(val.into()), + Ok(None) => Self::Missing("not set"), + Err(err) => Self::Err(err.to_string()), + } + } +} + +async fn get_all_fee_components(state: &S) -> AllFeeComponents { + let ( + transfer_fees, + rollup_data_submission_fees, + ics20_withdrawal_fees, + init_bridge_account_fees, + bridge_lock_fees, + bridge_unlock_fees, + bridge_sudo_change_fees, + ibc_relay_fees, + validator_update_fees, + fee_asset_change_fees, + fee_change_fees, + ibc_relayer_change_fees, + sudo_address_change_fees, + ibc_sudo_change_fees, + ) = join!( + state.get_transfer_fees().map(fetch_to_response), + state + .get_rollup_data_submission_fees() + .map(fetch_to_response), + state.get_ics20_withdrawal_fees().map(fetch_to_response), + state.get_init_bridge_account_fees().map(fetch_to_response), + state.get_bridge_lock_fees().map(fetch_to_response), + state.get_bridge_unlock_fees().map(fetch_to_response), + state.get_bridge_sudo_change_fees().map(fetch_to_response), + state.get_ibc_relay_fees().map(fetch_to_response), + state.get_validator_update_fees().map(fetch_to_response), + state.get_fee_asset_change_fees().map(fetch_to_response), + state.get_fee_change_fees().map(fetch_to_response), + state.get_ibc_relayer_change_fees().map(fetch_to_response), + state.get_sudo_address_change_fees().map(fetch_to_response), + state.get_ibc_sudo_change_fees().map(fetch_to_response), + ); + AllFeeComponents { + transfer_fees, + rollup_data_submission_fees, + ics20_withdrawal_fees, + init_bridge_account_fees, + bridge_lock_fees, + bridge_unlock_fees, + bridge_sudo_change_fees, + ibc_relay_fees, + validator_update_fees, + fee_asset_change_fees, + fee_change_fees, + ibc_relayer_change_fees, + sudo_address_change_fees, + ibc_sudo_change_fees, + } +} + +fn fetch_to_response(value: eyre::Result>) -> FetchResult +where + FeeComponent: From, +{ + value.into() +} + +#[derive(serde::Serialize)] +struct FeeComponent { + base: u128, + multiplier: u128, +} + +macro_rules! impl_from_domain_fee_component { + ( $($dt:ty ),* $(,)?) => { + $( + impl From<$dt> for FeeComponent { + fn from(val: $dt) -> Self { + Self { + base: val.base, + multiplier: val.multiplier, + } + } + } + )* + } +} +impl_from_domain_fee_component!( + astria_core::protocol::fees::v1::BridgeLockFeeComponents, + astria_core::protocol::fees::v1::BridgeSudoChangeFeeComponents, + astria_core::protocol::fees::v1::BridgeUnlockFeeComponents, + astria_core::protocol::fees::v1::FeeAssetChangeFeeComponents, + astria_core::protocol::fees::v1::FeeChangeFeeComponents, + astria_core::protocol::fees::v1::IbcRelayFeeComponents, + astria_core::protocol::fees::v1::IbcRelayerChangeFeeComponents, + astria_core::protocol::fees::v1::IbcSudoChangeFeeComponents, + astria_core::protocol::fees::v1::Ics20WithdrawalFeeComponents, + astria_core::protocol::fees::v1::InitBridgeAccountFeeComponents, + astria_core::protocol::fees::v1::RollupDataSubmissionFeeComponents, + astria_core::protocol::fees::v1::SudoAddressChangeFeeComponents, + astria_core::protocol::fees::v1::TransferFeeComponents, + astria_core::protocol::fees::v1::ValidatorUpdateFeeComponents, +); diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 45588da934..6bf2ff84bb 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -55,6 +55,8 @@ const BRIDGE_ACCOUNT_INFO: &str = "bridge/account_info/:address"; const TRANSACTION_FEE: &str = "transaction/fee"; +const FEES_COMPONENTS: &str = "fees/components"; + impl Info { pub(crate) fn new(storage: Storage) -> Result { let mut query_router = abci_query_router::Router::new(); @@ -76,6 +78,7 @@ impl Info { crate::bridge::query::bridge_account_info_request, )?; query_router.insert(TRANSACTION_FEE, crate::fees::query::transaction_fee_request)?; + query_router.insert(FEES_COMPONENTS, crate::fees::query::components)?; Ok(Self { storage, query_router, @@ -172,9 +175,28 @@ mod tests { protocol::{ account::v1::BalanceResponse, asset::v1::DenomResponse, + fees::v1::{ + BridgeLockFeeComponents, + BridgeSudoChangeFeeComponents, + BridgeUnlockFeeComponents, + FeeAssetChangeFeeComponents, + FeeChangeFeeComponents, + IbcRelayFeeComponents, + IbcRelayerChangeFeeComponents, + IbcSudoChangeFeeComponents, + Ics20WithdrawalFeeComponents, + InitBridgeAccountFeeComponents, + RollupDataSubmissionFeeComponents, + SudoAddressChangeFeeComponents, + TransferFeeComponents, + ValidatorUpdateFeeComponents, + }, }, }; - use cnidarium::StateDelta; + use cnidarium::{ + StateDelta, + StateWrite, + }; use prost::Message as _; use tendermint::v0_38::abci::{ request, @@ -372,4 +394,190 @@ mod tests { ); } } + + #[tokio::test] + async fn handle_fee_components() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let mut state = StateDelta::new(storage.latest_snapshot()); + + let height = 99; + + state.put_block_height(height).unwrap(); + write_all_the_fees(&mut state); + storage.commit(state).await.unwrap(); + + let info_request = InfoRequest::Query(request::Query { + path: "fees/components".to_string(), + data: vec![].into(), + height: u32::try_from(height).unwrap().into(), + prove: false, + }); + + let response = { + let storage = (*storage).clone(); + let info_service = Info::new(storage).unwrap(); + info_service + .handle_info_request(info_request) + .await + .unwrap() + }; + let query_response = match response { + InfoResponse::Query(query) => query, + other => panic!("expected InfoResponse::Query, got {other:?}"), + }; + assert!(query_response.code.is_ok(), "{query_response:?}"); + + let actual_fees = + serde_json::from_slice::(&query_response.value).unwrap(); + + assert_json_diff::assert_json_eq!(expected_fees(), actual_fees); + } + + fn expected_fees() -> serde_json::Value { + serde_json::json!({ + "bridge_lock_fees": { + "base": 1, + "multiplier": 1 + }, + "bridge_sudo_change_fees": { + "base": 3, + "multiplier": 3 + }, + "bridge_unlock_fees": { + "base": 2, + "multiplier": 2 + }, + "fee_asset_change_fees": { + "base": 4, + "multiplier": 4 + }, + "fee_change_fees": { + "base": 5, + "multiplier": 5 + }, + "ibc_relay_fees": { + "base": 7, + "multiplier": 7 + }, + "ibc_relayer_change_fees": { + "base": 8, + "multiplier": 8 + }, + "ibc_sudo_change_fees": { + "base": 9, + "multiplier": 9 + }, + "ics20_withdrawal_fees": { + "base": 10, + "multiplier": 10 + }, + "init_bridge_account_fees": { + "base": 6, + "multiplier": 6 + }, + "rollup_data_submission_fees": { + "base": 11, + "multiplier": 11 + }, + "sudo_address_change_fees": { + "base": 12, + "multiplier": 12 + }, + "transfer_fees": { + "base": 13, + "multiplier": 13 + }, + "validator_update_fees": { + "base": 14, + "multiplier": 14 + } + }) + } + + fn write_all_the_fees(mut state: S) { + state + .put_bridge_lock_fees(BridgeLockFeeComponents { + base: 1, + multiplier: 1, + }) + .unwrap(); + state + .put_bridge_unlock_fees(BridgeUnlockFeeComponents { + base: 2, + multiplier: 2, + }) + .unwrap(); + state + .put_bridge_sudo_change_fees(BridgeSudoChangeFeeComponents { + base: 3, + multiplier: 3, + }) + .unwrap(); + state + .put_fee_asset_change_fees(FeeAssetChangeFeeComponents { + base: 4, + multiplier: 4, + }) + .unwrap(); + state + .put_fee_change_fees(FeeChangeFeeComponents { + base: 5, + multiplier: 5, + }) + .unwrap(); + state + .put_init_bridge_account_fees(InitBridgeAccountFeeComponents { + base: 6, + multiplier: 6, + }) + .unwrap(); + state + .put_ibc_relay_fees(IbcRelayFeeComponents { + base: 7, + multiplier: 7, + }) + .unwrap(); + state + .put_ibc_relayer_change_fees(IbcRelayerChangeFeeComponents { + base: 8, + multiplier: 8, + }) + .unwrap(); + state + .put_ibc_sudo_change_fees(IbcSudoChangeFeeComponents { + base: 9, + multiplier: 9, + }) + .unwrap(); + state + .put_ics20_withdrawal_fees(Ics20WithdrawalFeeComponents { + base: 10, + multiplier: 10, + }) + .unwrap(); + state + .put_rollup_data_submission_fees(RollupDataSubmissionFeeComponents { + base: 11, + multiplier: 11, + }) + .unwrap(); + state + .put_sudo_address_change_fees(SudoAddressChangeFeeComponents { + base: 12, + multiplier: 12, + }) + .unwrap(); + state + .put_transfer_fees(TransferFeeComponents { + base: 13, + multiplier: 13, + }) + .unwrap(); + state + .put_validator_update_fees(ValidatorUpdateFeeComponents { + base: 14, + multiplier: 14, + }) + .unwrap(); + } } From 99a3b4b4e016c9047ec146fa076f6f952ef4c631 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 25 Oct 2024 18:12:45 +0200 Subject: [PATCH 3/3] clippy (remove _fees postfix) --- crates/astria-sequencer/src/fees/query.rs | 84 +++++++++---------- .../astria-sequencer/src/service/info/mod.rs | 28 +++---- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/crates/astria-sequencer/src/fees/query.rs b/crates/astria-sequencer/src/fees/query.rs index 04600de6c3..cf4bcd2478 100644 --- a/crates/astria-sequencer/src/fees/query.rs +++ b/crates/astria-sequencer/src/fees/query.rs @@ -500,20 +500,20 @@ fn preprocess_fees_request(request: &request::Query) -> Result(state: &S) -> AllFeeComponents { let ( - transfer_fees, - rollup_data_submission_fees, - ics20_withdrawal_fees, - init_bridge_account_fees, - bridge_lock_fees, - bridge_unlock_fees, - bridge_sudo_change_fees, - ibc_relay_fees, - validator_update_fees, - fee_asset_change_fees, - fee_change_fees, - ibc_relayer_change_fees, - sudo_address_change_fees, - ibc_sudo_change_fees, + transfer, + rollup_data_submission, + ics20_withdrawal, + init_bridge_account, + bridge_lock, + bridge_unlock, + bridge_sudo_change, + ibc_relay, + validator_update, + fee_asset_change, + fee_change, + ibc_relayer_change, + sudo_address_change, + ibc_sudo_change, ) = join!( state.get_transfer_fees().map(fetch_to_response), state @@ -572,20 +572,20 @@ async fn get_all_fee_components(state: &S) -> AllFeeComponents { state.get_ibc_sudo_change_fees().map(fetch_to_response), ); AllFeeComponents { - transfer_fees, - rollup_data_submission_fees, - ics20_withdrawal_fees, - init_bridge_account_fees, - bridge_lock_fees, - bridge_unlock_fees, - bridge_sudo_change_fees, - ibc_relay_fees, - validator_update_fees, - fee_asset_change_fees, - fee_change_fees, - ibc_relayer_change_fees, - sudo_address_change_fees, - ibc_sudo_change_fees, + transfer, + rollup_data_submission, + ics20_withdrawal, + init_bridge_account, + bridge_lock, + bridge_unlock, + bridge_sudo_change, + ibc_relay, + validator_update, + fee_asset_change, + fee_change, + ibc_relayer_change, + sudo_address_change, + ibc_sudo_change, } } diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 6bf2ff84bb..dcfbbddfdf 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -435,59 +435,59 @@ mod tests { fn expected_fees() -> serde_json::Value { serde_json::json!({ - "bridge_lock_fees": { + "bridge_lock": { "base": 1, "multiplier": 1 }, - "bridge_sudo_change_fees": { + "bridge_sudo_change": { "base": 3, "multiplier": 3 }, - "bridge_unlock_fees": { + "bridge_unlock": { "base": 2, "multiplier": 2 }, - "fee_asset_change_fees": { + "fee_asset_change": { "base": 4, "multiplier": 4 }, - "fee_change_fees": { + "fee_change": { "base": 5, "multiplier": 5 }, - "ibc_relay_fees": { + "ibc_relay": { "base": 7, "multiplier": 7 }, - "ibc_relayer_change_fees": { + "ibc_relayer_change": { "base": 8, "multiplier": 8 }, - "ibc_sudo_change_fees": { + "ibc_sudo_change": { "base": 9, "multiplier": 9 }, - "ics20_withdrawal_fees": { + "ics20_withdrawal": { "base": 10, "multiplier": 10 }, - "init_bridge_account_fees": { + "init_bridge_account": { "base": 6, "multiplier": 6 }, - "rollup_data_submission_fees": { + "rollup_data_submission": { "base": 11, "multiplier": 11 }, - "sudo_address_change_fees": { + "sudo_address_change": { "base": 12, "multiplier": 12 }, - "transfer_fees": { + "transfer": { "base": 13, "multiplier": 13 }, - "validator_update_fees": { + "validator_update": { "base": 14, "multiplier": 14 }