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

Add prefix handling for esdt prefix #526

Merged

Conversation

danielailie
Copy link
Contributor

No description provided.

@danielailie danielailie changed the title Add prefix handling for esdt Add prefix handling for esdt prefix Nov 7, 2024
@danielailie danielailie self-assigned this Nov 7, 2024
popenta
popenta previously approved these changes Nov 7, 2024
src/tokens.ts Outdated
@@ -249,19 +249,31 @@ export class TokenComputer {

extractIdentifierFromExtendedIdentifier(identifier: string): string {
const parts = identifier.split("-");
const { prefix, ticker, randomnes } = this.splitIdentifierIntoComponent(parts);
Copy link
Contributor

Choose a reason for hiding this comment

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

typo in randomness. can also be renamed to randomSequence.

src/tokens.ts Outdated
@@ -281,6 +293,15 @@ export class TokenComputer {
}
}

private checkLengthOfPrefix(prefix: string): void {
const TOKEN_PREFIX_LENGTH = 4;
if (prefix.length > TOKEN_PREFIX_LENGTH) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should also check that the length of the prefix is at least 1.

src/tokens.ts Outdated
@@ -249,19 +249,31 @@ export class TokenComputer {

extractIdentifierFromExtendedIdentifier(identifier: string): string {
const parts = identifier.split("-");
const { prefix, ticker, randomnes } = this.splitIdentifierIntoComponent(parts);
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo 🙃

randomness and components.

src/tokens.ts Outdated
return ticker + "-" + randomnes;
}

splitIdentifierIntoComponent(parts: string[]): { prefix: any; ticker: any; randomnes: any } {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have this as private?

src/tokens.ts Outdated
}

splitIdentifierIntoComponent(parts: string[]): { prefix: any; ticker: any; randomnes: any } {
if (parts.length >= 3 && parts[2].length === 6) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In other functions, we have constants defined (for 3 and 6).

Copy link
Contributor

Choose a reason for hiding this comment

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

How would it be to contain the const parts = identifier.split("-"); itself within this function, too? 🌨️

Copy link
Contributor Author

@danielailie danielailie Nov 7, 2024

Choose a reason for hiding this comment

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

I don't know what name should I use for 3, any suggestions
?

popenta
popenta previously approved these changes Nov 7, 2024
src/tokens.ts Show resolved Hide resolved
src/tokens.ts Outdated Show resolved Hide resolved
src/tokens.ts Outdated Show resolved Hide resolved
src/tokens.ts Outdated Show resolved Hide resolved
src/tokens.ts Outdated Show resolved Hide resolved
src/tokens.ts Outdated Show resolved Hide resolved
popenta
popenta previously approved these changes Nov 8, 2024
this.validateExtendedIdentifier(prefix, randomSequence, ticker, parts);
if (prefix) {
this.checkLengthOfPrefix(prefix);
return prefix + "-" + ticker + "-" + randomSequence;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could have had string interpolation / optional 🙃

@danielailie danielailie merged commit 0e17949 into main Nov 8, 2024
4 checks passed
@danielailie danielailie deleted the TOOL-307-add-esdt-prefix-to-support-sovereign-integration branch November 8, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants