-
Notifications
You must be signed in to change notification settings - Fork 28
add /assetid(to|from)hexbytes APIs #62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
add /assetid(to|from)hexbytes APIs #62
Conversation
Add /encodeassetid and /decodeassetid APIs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've requested some changes, mainly stylistic. In addition please rebase the commits on the latest master, sign the commits with gpg, remove the merge commit and rename the last commit to add test for assetid(to|from)hexbytes APIs
. Finally please remember to format and lint the code after making changes.
@zoedberg all fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for addressing the requested changes!
Please see 2 small suggestions, other than that LGTM.
Please also squash the commits into a single one, signed with gpg and with add /assetid(to|from)hexbytes APIs
as message. If you prefer keeping the commit adding the tests separated, then call the second commit add test for assetid(to|from)hexbytes APIs
. Then rebase the commit (or the 2 commits) on the latest master commit (d13a4ec).
Finally, please before requesting the next review make sure the code is still linted and formatted (using rust 1.88.0).
#[error("Invalid asset ID: {0}")] | ||
InvalidAssetID(String), | ||
|
||
#[error("Invalid hex bytes")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[error("Invalid hex bytes")] | |
#[error("Invalid asset ID hex bytes")] |
#[derive(Deserialize, Serialize)] | ||
pub(crate) struct AssetIdToHexBytesRequest { | ||
pub(crate) asset_id: String, | ||
} | ||
|
||
#[derive(Deserialize, Serialize)] | ||
pub(crate) struct AssetIdToHexBytesResponse { | ||
pub(crate) hex_bytes: String, | ||
} | ||
|
||
#[derive(Deserialize, Serialize)] | ||
pub(crate) struct AssetIdFromHexBytesRequest { | ||
pub(crate) hex_bytes: String, | ||
} | ||
|
||
#[derive(Deserialize, Serialize)] | ||
pub(crate) struct AssetIdFromHexBytesResponse { | ||
pub(crate) asset_id: String, | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please sort these alphabetically
encode and decode assetId format between hex and string by baid64
test : encode_decode_asset_id.rs