Skip to content
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

Formatted NFT identifier in api response #2949

Merged
merged 6 commits into from
Apr 1, 2021

Conversation

bogdan-rosianu
Copy link
Contributor

Formatted the NFT identifier in /address/:address/esdt endpoint.
Now, if a token is regular esdt, the key for it would follow the syntax TICKER-3e3e3e and if it the token is a NFT, then it would follow the syntax TICKER-3e3e3e-02 where 02 is the hex encoded nonce.

Before this PR, the token identifier for NFTs would have resulted in non readable characters

@bogdan-rosianu bogdan-rosianu added the type:feature New feature or request label Mar 31, 2021
@bogdan-rosianu bogdan-rosianu self-assigned this Mar 31, 2021
sasurobert
sasurobert previously approved these changes Mar 31, 2021
Copy link
Contributor

@sasurobert sasurobert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. Even a unit test :)

sasurobert
sasurobert previously approved these changes Mar 31, 2021
@@ -49,10 +49,7 @@
{ Name = "/:address/esdt/:tokenIdentifier", Open = true },

# /address/:address/nft/:tokenIdentifier/nonce/:nonce will return data of an nft esdt token for a given account, tokenID and nonce
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data of a nft instead data of an nft

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, using an is correct

node/node.go Outdated
@@ -60,6 +60,8 @@ import (
// SendTransactionsPipe is the pipe used for sending new transactions
const SendTransactionsPipe = "send transactions pipe"

const esdtTickerNumChars = 6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

group these constants
const(
..
..
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -390,6 +390,73 @@ func TestNode_GetAllESDTTokens(t *testing.T) {
assert.Equal(t, esdtData, value[esdtToken])
}

func TestNode_GetAllESDTTokensShouldReturnEsdtAndFormattedNft(t *testing.T) {
acc, _ := state.NewUserAccount([]byte("newaddress"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

t.Parallel ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no test in this file contains t.Parallel() so I would leave it as it is

sasurobert
sasurobert previously approved these changes Apr 1, 2021
@bogdan-rosianu bogdan-rosianu merged commit ccb00a9 into feat/eip-esdt-local-mint Apr 1, 2021
@bogdan-rosianu bogdan-rosianu deleted the fix-format-for-nft-id-api branch April 1, 2021 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants