-
Notifications
You must be signed in to change notification settings - Fork 308
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
Provide an easier deposit/airdrop workflow when users don't have a trustline or account #303
Comments
As a general update on this issue — the protocol working group for this issue is meeting again this week to reconsider other CAP forward designs to help solve this problem. If a sufficient solution can be proposed and approved, it will likely change the content of this substantially. Will continue to update here as progress is made. |
One improvement we need to make to |
So it looks like this is the issue tracking CAP-0023. CAP23 is interesting, @jonjove General questions:
Nits:
|
@msfeldstein I just wanted to put this issue on your radar as an important SEP-6 improvement. CAP-23 (sending a balance via a 2-part payment) that Nico mentions above is probably pretty far out, so I'm thinking the best approach is to improve the flow that Alex points out in To just review the context, the problem is that the current way we deal with users who make a deposit but have no account goes like this:
This is a really bad user experience because they must take two actions, and wait (possibly days) two times to make the deposit happen. If they mess it up or don't log back into the wallet, it appears that the anchor or someone simply took their money. The |
I'm ramping up on the details here, but out of curiosity, could we do the existing flow but have the end user create a pre-signed transaction creating the trustline that the anchor can then submit after creating the account? Then the user doesn't need to come back online for anything.
|
an account is not created with sequence number |
I assumed create_account would be the transaction with seq 0 so the
trustline transaction would be seq 1. Though I guess the create_account
transaction would use up one of the issuers sequence numbers and not the
account being created, so the trustline operation could be seq 0
Do accounts not start at seq no 0?
…On Fri, Aug 30, 2019, 3:55 PM MonsieurNicolas ***@***.***> wrote:
User initiates deposit in wallet via SEP-6 giving the issuer both their
account id and a signed transaction adding the trustline with sequence#1
an account is not created with sequence number 1, it would be possible to
use bumpSeqOp but this would have to be done using an account controlled
by the issuer. transfer.py is an implementation of a protocol that works
around most of the problems that come with this scenario.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#303?email_source=notifications&email_token=AABHK36SZVZ4XDYRDO4OGJ3QHGQM3A5CNFSM4HLNPPV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5S6TJQ#issuecomment-526772646>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABHK36AQTOHO53JLT5XWTDQHGQM3ANCNFSM4HLNPPVQ>
.
|
@msfeldstein, no. the starting seq number depends on the ledger in which the account was created: |
@msfeldstein I just wanted to check in that you're feeling like |
Yea i think it's still the best approach, just wanted to understand the
need for the complications.
…On Wed, Sep 4, 2019 at 2:37 PM Tom Quisel ***@***.***> wrote:
@msfeldstein <https://github.com/msfeldstein> I just wanted to check in
that you're feeling like transfer.py is a solid approach, or if you're
still exploring an alternative based on pre-signed transactions. We
explored many of those approaches and ultimately decided they were worse,
but we may have missed something.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#303?email_source=notifications&email_token=AABHK355NRZ7HB54F7WQUBLQIAS7ZA5CNFSM4HLNPPV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD55CATI#issuecomment-528097357>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABHK37ARCNA7EDK44S7UC3QIAS7ZANCNFSM4HLNPPVQ>
.
|
It seems like this needs the "Accounts for Signer" endpoint to be enabled before we can really make the right experience here, am i understanding that correctly? |
@msfeldstein yep, great point that Accounts for Signer is needed. Since it's already live on horizon.stellar.org, though, it's probably not a blocking dependency. |
I have a javascript version of this running that shows pretty clearly whats supposed to be happening on both ends, including the new accounts-for-signer endpoint. I do feel this will work in real wallets. It currently runs through accounts that are created but have no trustline and works great, i need to verify it works for accounts that haven't been created yet. https://github.com/msfeldstein/airdrop-script |
Cool! This great. Have you dealt with the case of an |
@jonjove here are some comments based on the latest CAP0023 draft ClaimPredicate
ClaimCondition
ClaimableBalanceEntry
CreateClaimableBalanceOp
AuthorizeClaimableBalanceOp
ClaimClaimableBalanceOp
|
ClaimPredicate
ClaimCondition
ClaimableBalanceEntry
CreateClaimableBalanceOp
AuthorizeClaimableBalanceOp
ClaimClaimableBalanceOp
|
Great updates. xdr cleanupAs xdr doesn't allow to extend enums, we should give relative time predicates a name that gives a hint that those predicates don't actually exist in the ledger and are only used in operations. CreateClaimableBalanceOp"amount is non-positive": do you mean Step 4: need to be explicit on what you mean that the account is "authorized". I suspect you need Condition on
|
Thanks for providing more feedback! CreateClaimableBalanceOp
Condition on
|
I wrote some thoughts related to extending the proposal for utxo support (I think also replace deterministic accounts, a question I was asking earlier). See https://gist.github.com/MonsieurNicolas/25e6bd235151a197be84a0712555082e @jonjove |
Here is another round of feedback. XDR
Should Prepared Trust LineThe semantics in the doc are written in a fairly confusing way. Right now to understand what it is, the reader has to derive it from what happens in the various operations and/or from the rationale (the fact that it's mutually exclusive with a trust line is a key property that is right now only mentioned in the "rationale"). AllowTrustOpNeed to describe what "analogous" really is. I would expect revoking to delete the "prepared trust line"? I guess, if you want to separate "issuer" from the account sponsoring trust lines, you don't want the issuer to receive the base reserve in this situation, so you need an accountID that specifies who can delete the prepared trust line (and you need an explicit operation to delete those things). CreatePreparedTrustLineOpI think we should have separate "success" codes so that we can distinguish if the (pending) trust line already existed or not as users may do something different based on this. CreateClaimableBalanceOpPlease add the error code for each of the invalid cases. ClaimClaimableBalanceOpNeed a section in "rationale" that explains why sending back the reserve to "createdBy" makes sense. In particular, it looks like "createdBy" exists in Threshold should be "medium" as it changes the balances associated with the account (and claiming something can be as bad as sending from a regulatory standpoint). Should it be possible to increase the amount of a ClaimableBalanceEntry?I disagree with what is written here. We should give the guarantee that a |
@MonsieurNicolas Let’s discuss how we think prepared trust lines (or whatever we decide to call them) should work. Prepared trust lines in this proposalIn this proposal, I was working with the following definition of a prepared trust line: A prepared trust line is a ledger entry with the following properties
Property 1I don’t think property (1) is controversial, so I will not provide any analysis of it. Property 2Property (2) is not, to me, a strict requirement although other people may disagree. One advantage of including this property is that it makes it possible to use prepared trust lines without requiring Medium threshold on the issuer account (allow trust only requires Low). Property 3Property (3) has important implications. A prepared trust line can have authorization flags set by the issuer. There are only two ways to change the authorization flags on a trust line: the issuer can change them with So what could go wrong? It is easy to see that if any account can remove the prepared trust line, then it is possible for an attacker to remove any prepared trust line at any time; this would render the prepared trust lines nearly useless. If only the account that created the prepared trust line can remove it, then there is a a more restricted attack in which an attacker learns that a prepared trust line will be created by some other account so the attacker creates the prepared trust line first. When the issuer authorizes the prepared trust line, the attacker removes and re-creates the prepared trust line in a single transaction. This clears the flags while extending the attack. Property 4Property (4) is in many ways a consequence of property (3). We can’t permit accounts to remove the prepared trust line, so how do you increase the reserve should you want to? You simply create it again. I want to address this remark specifically:
Even if there were no attacks associated with allowing pending trust lines to be removed, this does not provide a good user-experience because it would clear the flags. There are other benefits to this approach. It allows issuers to bundle creating a prepared trust line and allow trust into a single transaction without concern over whether creating the prepared trust line will fail due to (for example) a race with the target account in which it creates the actual trust line first. A different model for prepared trust linesIf we change property (2) above to “It can only be created by the issuer for any account”, this produces a considerably different model. The new definition is: A prepared trust line is a ledger entry with the following properties Property 2’Property (2’) is much more restrictive than property (2) above, but still captures the most important use case in which the issuer wants to prepare a trust line into a certain authorization state for an account without interacting with that account. Property 3’Property (3’) is less restrictive than property (3) above, because all of the potential attacks have been removed. As we noted, the authorization of a trust line can already be changed by the issuer so there is no reason that an issuer should not be able to remove a prepared trust line (there will need to be some constraints around Property 4’Property (4’) is weaker than property (4) above, because only the issuer can create a prepared trust line so there is no risk of racing with the creation of a prepared trust line by another account. It is fine for the issuer to remove a prepared trust line with flags set in order to increase the reserve because the issuer can set the flags again afterwards. The only downside of this simplification is that the issuer might not be able to re-create a prepared trust line if |
To sum up our conversation today, our take is that:
|
Implemented in stellar/stellar-core#2591 |
As discussed with @tomquisel, this issue aims to document an alternative approach for making the deposit and sending assets workflow better in Stellar.
Tom Quisel and Jeremy Rubin have created a Python script named transfer.py as a proof of concept. It is basically a simplified special case of SEP-016.
Our initial thought was to create a new SEP that explains what
transfer.py
does, splitting it into:A few important details to keep in mind:
The text was updated successfully, but these errors were encountered: