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

Refresh SAC CAP #1370

Merged
merged 8 commits into from
Aug 16, 2023
Merged

Refresh SAC CAP #1370

merged 8 commits into from
Aug 16, 2023

Conversation

sisuresh
Copy link
Contributor

No description provided.

@sisuresh sisuresh requested a review from dmkozh August 10, 2023 17:04
#### Clawback
Account balances can only be clawed back if the trustline has
`TRUSTLINE_CLAWBACK_ENABLED_FLAG` set. When a contract balance is created, it
will it will be clawbak enabled if the issuer account has
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant it will

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Account balances can only be clawed back if the trustline has
`TRUSTLINE_CLAWBACK_ENABLED_FLAG` set. When a contract balance is created, it
will it will be clawbak enabled if the issuer account has
`AUTH_CLAWBACK_ENABLED_FLAG` set. Not that the clawback enabled flag for
Copy link
Contributor

Choose a reason for hiding this comment

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

Not->Note?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

functionality to an administrator. The administrator can be a contract, a simple
Ed25519 key, or an existing Stellar account.
functionality to an administrator. The administrator can be a contract or an
existing Stellar account.

This design decision does not break compatibility with Stellar assets. If an
existing holder does not want to accept the new terms, they simply elect not to
Copy link
Contributor

Choose a reason for hiding this comment

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

If an existing holder does not want to accept the new terms, they simply elect not to wrap their asset. - I don't this statement is true. Anyone can wrap any asset. I assume what this actually should mean is that the issuer may choose not to change the token admin and retain authority over minting/clawbacking the token.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is out of date, but what it's trying to say is that a user that owns a token in classic does not have to use their token in soroban if they don't want to.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, but this is in the admin section, so I thought it concerns issuers... Users actually don't have a choice - if issuer creates a SAC and changes the admin, then the new admin will be able to e.g. clawback the token. I don't think that's an issue, but this paragraph doesn't make much sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this section should be removed. It was written at a time where we didn't have compliance controls in the token contract. Now we have controls that mimic classic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original concern was that Soroban gave the admin even more control, but as a token holder, you didn't have to care if you didn't use Soroban.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then I think we also should remove 'If an existing holder does not want to accept the new terms...' sentence as there aren't really new terms (the admin might change, but holder can't do anything about that).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this section

have a wrapper interface because those administered by contracts would lose
control over the wrapped assets. Smart asset issuers can always deploy their own
wrapper interface should they need it.

### No Total Supply

Total supply is confusing for classic assets because they can exist in wrapped
Copy link
Contributor

Choose a reason for hiding this comment

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

Total supply is confusing for classic assets because they can exist in wrapped and unwrapped form. - this is no longer correct - there are no 'wrapped' balances (maybe need to make a pass through the whole doc and clean this up). The real reason is that classic assets don't support total supply, so SAC doesn't support them as well.

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 point. I removed all references to "wrapped"

@sisuresh sisuresh enabled auto-merge (squash) August 16, 2023 00:53
`AUTH_CLAWBACK_ENABLED_FLAG` set. Not that the clawback enabled flag for
contract balances cannot be modified, which is a little different from
trustlines where the issuer can choose to disable it.
will be clawbak enabled if the issuer account has `AUTH_CLAWBACK_ENABLED_FLAG`
Copy link
Contributor

Choose a reason for hiding this comment

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

s/clawbak/clawback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

functionality to an administrator. The administrator can be a contract, a simple
Ed25519 key, or an existing Stellar account.
functionality to an administrator. The administrator can be a contract or an
existing Stellar account.

This design decision does not break compatibility with Stellar assets. If an
existing holder does not want to accept the new terms, they simply elect not to
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then I think we also should remove 'If an existing holder does not want to accept the new terms...' sentence as there aren't really new terms (the admin might change, but holder can't do anything about that).

burning. Tokens that need to track total supply can do so by having the
administrator contract update it when calling `mint` and `clawback`.
Total supply is not available because it isn't supported in Stellar Classic, so
there's no way to reasonable track the supply between trustlines and contract
Copy link
Contributor

Choose a reason for hiding this comment

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

s/reasonable/reasonably

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@sisuresh sisuresh merged commit b328b13 into stellar:master Aug 16, 2023
2 checks passed
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