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

[TS] Implement support for SEP-7 #141

Merged
merged 9 commits into from
Jun 27, 2024
Merged

[TS] Implement support for SEP-7 #141

merged 9 commits into from
Jun 27, 2024

Conversation

CassioMG
Copy link
Contributor

@CassioMG CassioMG commented Jun 21, 2024

Ticket: https://stellarorg.atlassian.net/jira/software/c/projects/WAL/boards/37?selectedIssue=WAL-1449

This PR adds support for SEP-7 through the following classes:

  • Sep7Base: abstract base class with common functions and attributes used by both tx and pay operations
  • Sep7Tx: implements the Sep-7 tx operation
  • Sep7Pay: implements the Sep-7 pay operation

This PR also adds some convenient parsing/helper functions as follow:

  • isValidSep7Uri: checks if uri is Sep-7 compliant, if not returns a explanatory reason message
  • parseSep7Uri: parses a Sep-7 uri to either a Sep7Tx or Sep7Pay instance
  • sep7ReplacementsFromString: converts a complex replacements string to a list of friendly replacements objects
  • sep7ReplacementsToString: converts a list of friendly replacements objects to a complex replacements string

Those classes and functions can be used to either generate a new Sep-7 URI or handle an existing Sep-7 URI.

This PR also adds a bunch of tests and JSDocs that should cover all new classes and functions.

This PR also adds a few missing types and exceptions from existing code (harmless changes).

*
* @returns {boolen} returns `true` if it's a valid Sep-7 uri, `false` otherwise.
*/
export const isValidSep7Uri = (uri: string): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this check that the xdr parameter for tx operations are a valid TransactionEnvelope?
Same question for destination on the pay operation, should it check that the value is a valid account ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I like your suggestions :)

I think I'll make this function returning something like:

{
  result: boolean;
  reason: string;
}

Where reason should be a message letting devs know why the verification failed.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah that sounds good to me, I like the more informational response. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implemented here: 64f6544
More tests to account for the error reason messages: 89c5045

Copy link
Contributor

Choose a reason for hiding this comment

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

this lgtm 👏

Copy link
Contributor

@aristidesstaffieri aristidesstaffieri left a comment

Choose a reason for hiding this comment

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

left one question but lgtm otherwise

@CassioMG CassioMG merged commit 8f9360f into main Jun 27, 2024
7 checks passed
@CassioMG CassioMG deleted the cg-sep-07 branch June 27, 2024 20:47
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.

2 participants