Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces V1 API traits and implementations for block-state, fee-state, and reward-state, along with corresponding Axum handlers, routes, and parity tests. Feedback suggests mapping client-side parsing errors to ApiError::BadRequest instead of ApiError::Internal for better HTTP status accuracy. Additionally, the reviewer noted a discrepancy where V1 URL prefixes return V2 data via shared handlers, recommending clarification in the documentation.
| let get_block_state_height = |State(state): State<S>| async move { | ||
| state | ||
| .get_block_state_height() | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_block_state_path = |State(state): State<S>, Path((height, key)): Path<(u64, u64)>| async move { | ||
| state | ||
| .get_block_state_path(height, key) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_block_state_path_by_commit = | ||
| |State(state): State<S>, Path((commit, key)): Path<(String, u64)>| async move { | ||
| state | ||
| .get_block_state_path_by_commit(commit, key) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
|
|
||
| // Fee-State API handlers | ||
| let get_fee_state_height = |State(state): State<S>| async move { | ||
| state | ||
| .get_fee_state_height() | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_fee_state_path = | ||
| |State(state): State<S>, Path((height, address)): Path<(u64, String)>| async move { | ||
| state | ||
| .get_fee_state_path(height, address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_fee_state_path_by_commit = | ||
| |State(state): State<S>, Path((commit, address)): Path<(String, String)>| async move { | ||
| state | ||
| .get_fee_state_path_by_commit(commit, address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_fee_balance = |State(state): State<S>, Path(address): Path<String>| async move { | ||
| state | ||
| .get_fee_balance(address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
|
|
||
| // Reward-State (V1) API handlers | ||
| let get_reward_state_height = |State(state): State<S>| async move { | ||
| state | ||
| .get_reward_state_height() | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_reward_state_path = | ||
| |State(state): State<S>, Path((height, address)): Path<(u64, String)>| async move { | ||
| state | ||
| .get_reward_state_path(height, address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_reward_state_path_by_commit = | ||
| |State(state): State<S>, Path((commit, address)): Path<(String, String)>| async move { | ||
| state | ||
| .get_reward_state_path_by_commit(commit, address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; | ||
| let get_reward_state_proof = | ||
| |State(state): State<S>, Path((height, address)): Path<(u64, String)>| async move { | ||
| state | ||
| .get_reward_state_proof(height, address) | ||
| .await | ||
| .map(Json) | ||
| .map_err(ApiError::Internal) | ||
| }; |
There was a problem hiding this comment.
The new handlers for block-state, fee-state, and reward-state map all errors to ApiError::Internal (500 Internal Server Error). This includes client-side errors such as invalid address formats or invalid commitment strings that fail to parse in the underlying implementation. Consider mapping these parsing errors to ApiError::BadRequest (400) to provide more accurate HTTP status codes.
| /// Path: GET /v1/reward-state/proof/{height}/{address} | ||
| pub const REWARD_STATE_PROOF_ROUTE: &str = "/v1/reward-state/proof/{height}/{address}"; | ||
|
|
||
| /// Get latest V1 reward account proof (shared handler with reward-state-v2) |
There was a problem hiding this comment.
The comment describes this as the 'latest V1 reward account proof', but the handler get_latest_reward_account_proof (shared with reward-state-v2) returns a V2 proof. This discrepancy between the URL prefix (/v1/reward-state/) and the returned data version (V2) should be clarified in the documentation or the comment.
Nextest failures (2) in this run
See the step summary for flaky tests and slowest tests. |
Nextest failures (1) in this run
See the step summary for flaky tests and slowest tests. |
1 participant
Migrates the fee-state, block-state and reward-state endpoints to axum