Adding API endpoint to get fee balance per account #2165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fkrell
requested review from
philippecamacho,
ImJeremyHe and
sveitser
as code owners
fkrell
changed the title
jbearer
reviewed
Oct 11, 2024
sequencer/api/merklized_state.toml
Outdated
| @@ -0,0 +1,4 @@ | |||
| [route.getfeebalance] | |||
| PATH = ["/fee_balance/:address"] | |||
There was a problem hiding this comment.
As a convention we usually use hyphens in URL segments eg fee-balance
sequencer/src/api/endpoints.rs
Outdated
| + Sync | ||
| + MerklizedStateDataSource<SeqTypes, FeeMerkleTree, 256> | ||
| + MerklizedStateHeightPersistence, | ||
| //for<'a> <<UniversalMerkleTree<FeeAmount,Sha3Digest,FeeAccount,256,Sha3Node> as hotshot_query_service::merklized_state::MerklizedState<Types, ARITY>>::Commit as TryFrom<&'a TaggedBase64>>::Error: Display, |
sveitser
previously requested changes
Oct 11, 2024
sequencer/src/api/endpoints.rs
Outdated
| .parse() | ||
| .map_err(|_| merklized_state::Error::Custom { | ||
| message: "failed to parse Key param".to_string(), | ||
| status: StatusCode::INTERNAL_SERVER_ERROR, |
There was a problem hiding this comment.
This should be a 400 / BadRequest error because it's the client's fault.
sequencer/src/api/endpoints.rs
Outdated
| + Sync | ||
| + MerklizedStateDataSource<SeqTypes, FeeMerkleTree, 256> | ||
| + MerklizedStateHeightPersistence, | ||
| //for<'a> <<UniversalMerkleTree<FeeAmount,Sha3Digest,FeeAccount,256,Sha3Node> as hotshot_query_service::merklized_state::MerklizedState<Types, ARITY>>::Commit as TryFrom<&'a TaggedBase64>>::Error: Display, |
sequencer/src/api/endpoints.rs
Outdated
| let key = address | ||
| .parse() | ||
| .map_err(|_| merklized_state::Error::Custom { | ||
| message: "failed to parse Key param".to_string(), |
There was a problem hiding this comment.
Failed to parse address?
sequencer/api/merklized_state.toml
Outdated
| [route.getfeebalance] | ||
| PATH = ["/fee_balance/:address"] | ||
| ":address" = "Literal" | ||
| DOC = "Get current balance in address account." |
There was a problem hiding this comment.
"Balance in fee state" or something like that?
There was a problem hiding this comment.
And mention that we are expecting an ethereum address in hex format.
sveitser
reviewed
Oct 11, 2024
|
Did manually test it and that looks good 👍 |
sveitser
dismissed
their stale review
I'll dismiss my change request because I will be OOO and none of it is critical. Would be nice if some of the suggestions were acted on though.
tbro
approved these changes
Oct 25, 2024
sveitser
approved these changes
Oct 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2105
This PR:
Adds get_fee_balance endpoint to the sequencer API to retrieve the current balance of an account.
This allows to detect whenever the Builder is low on funds and issue the corresponding alert
This PR does not:
Key places to review:
sequencer/api/merklized_state.toml: defines new endpoint.
sequencer/src/api/endpoints.rs: extends merklized state API with the new endpoint and define its behaviour.
In particular, given address as an input it retrieves current height from state, and uses height and address to retrieve merkle path from merklized state. Balance is extracted from leaf of this path.
Manual test is to run the native demo locally, retrieve the builder address from http://localhost:31003/v0/block_info/builderaddress, then use the address to call the new endpoint
http://localhost:24001/v0/fee-state/fee_balance/