feat(sequencer): query full denomination from asset ID #1067
Conversation
github-actions
bot
added
proto
There was a problem hiding this comment.
Nothing critical, but a few suggestions.
I don't think a server should be returning a full stack trace as part of an error response message, hence I suggested changing the formatting of the anyhow errors from debug to alternate {:#} to only include the display-formatted source chain. This might not be what we do elsewhere, so feel free to ignore.
Also, it might be nice to have some sad-path tests for the new endpoint, but that's not a blocker IMO.
|
@Fraser999 addressed comments - I changed the error returned to be display-formatted, but in the account query module, we return the debug formatted errors. do you think we should change it there as well? |
|
@noot - yes, I would vote for changing them all to return the alternate display form. There's still one debug one lingering in query.rs on line 95 btw :) |
|
|
||
| // A response containing the denomination given an asset ID. | ||
| message DenomResponse { | ||
| uint64 height = 2; |
There was a problem hiding this comment.
i did it to match the existing responses:
// A response containing the balance of an account.
message BalanceResponse {
uint64 height = 2;
repeated AssetBalance balances = 3;
}
// A response containing the current nonce for an account.
message NonceResponse {
uint64 height = 2;
uint32 nonce = 3;
}
should we change all of those?
There was a problem hiding this comment.
Yeah, they should all start at 1. Too late now because that would be a breaking change. 🤷
Summary
implement ABCI query to get full denomination from an asset ID, if it exists in the latest sequencer state.
Background
useful for block explorers/UIs as txs only have asset IDs, not the full denom.
Changes
Testing
unit tests
Related Issues
closes #1053