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 PermissionedDomain support for MPTs. #263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tapanito
Copy link

No description provided.

@Tapanito
Copy link
Author

cc: @sappenin @shawnxie999

Copy link
Collaborator

@sappenin sappenin left a comment

Choose a reason for hiding this comment

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

LGTM, nice work @Tapanito !

@sappenin sappenin requested review from shawnxie999, gregtatcam and mvadari and removed request for mvadari January 28, 2025 22:50
@shawnxie999
Copy link
Collaborator

DomainID would considered to be a change for another amendment, and not for XLS-33. I'm not opposing making this change (it makes things easier to find when everything is in one place). But I’m curious if there’s a formal process for updating a previous spec based on a later spec. @sappenin

@mDuo13
Copy link
Collaborator

mDuo13 commented Feb 4, 2025

I think it's important to be careful/explicit about dependencies among multiple amendments like this. What happens if MPT support is enabled on a given network and Permissioned Domains aren't?

@mDuo13
Copy link
Collaborator

mDuo13 commented Feb 4, 2025

Actually, let me take this a step further and say:

THIS SHOULD NOT BE MERGED AS-IS.

The MPTokensV1 amendment is already open for voting in a released stable version of the rippled server. We cannot implement transaction processing changes such as this without having a separate amendment for it. Otherwise, servers with the v2.3.0 version of the MPTokensV1 amendment would diverge from servers with the hypothetical future version of the amendment.

So any change to this spec should make it clear which amendment gates the addition/change to the spec.

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.

5 participants