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

SEP-0010: Clarify the first manage data operation is the operation containing the client account, home domain, etc #754

Merged
merged 2 commits into from
Oct 29, 2020

Conversation

leighmcculloch
Copy link
Member

What

Clarify that the first manage data operation is the operation containing the client account, nonce, and home domain.

Why

The first manage data operation has always been the first operation. This is stated explicitly further down in the SEP when the challenge transaction structure is described. All SDKs I have reviewed also follow this pattern. The abstract is somewhat ambiguous though because it states that there is a single manage data operation with those values without stating it is the first.

This change is in response to the discussion here:
#746 (comment)

Note

This change is only improving the language and is not adding or changing the functionality of the SEP. This change is not urgent and may end up being merged before or after the v3 changes being proposed. If it is merged after the PR needs updating with an appropriate version and timestamp.

cc @nebolsin @JakeUrban @accordeiro

### What
Clarify that the first manage data operation is the operation containing the client account, nonce, and home domain.

### Why
The first manage data operation has always been the first operation. This is stated explicitly further down in the SEP when the challenge transaction structure is described. All SDKs I have reviewed also follow this pattern. The abstract is somewhat ambiguous though because it states that there is a single manage data operation with those values without stating it is the first.

### Note
This change is only improving the language and is not adding or changing the functionality of the SEP.
@leighmcculloch leighmcculloch self-assigned this Oct 23, 2020
@leighmcculloch leighmcculloch changed the title SEP-0010: Clarify the SEP-0010: Clarify the first manage data operation is the operation containing the client account, home domain, etc Oct 23, 2020
ecosystem/sep-0010.md Outdated Show resolved Hide resolved
Copy link
Contributor

@JakeUrban JakeUrban left a comment

Choose a reason for hiding this comment

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

LGTM. I don't think we ever resolved it, but are we expecting the operations in the challenge to be ordered or unordered? With this change, I think we need to go the ordered route.

@leighmcculloch
Copy link
Member Author

@JakeUrban The first operation must be the auth op so it must be ordered. Any other ops the order is not defined.

@JakeUrban
Copy link
Contributor

This should be merged prior to 3.0, can I merge it? @leighmcculloch

@leighmcculloch leighmcculloch merged commit 01660c9 into stellar:master Oct 29, 2020
@leighmcculloch leighmcculloch deleted the sep10clarifyabstract branch October 29, 2020 20:16
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.

3 participants