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

Transaction structure can be simplified #2

Open
TorstenStueber opened this issue Dec 4, 2018 · 5 comments
Open

Transaction structure can be simplified #2

TorstenStueber opened this issue Dec 4, 2018 · 5 comments

Comments

@TorstenStueber
Copy link

After reading the protocol specification (starlight/starlight/doc/Protocol.md) I wonder why the following transactions cannot be combined:

  1. The three SetupAccountTx can be combined into one single transaction that creates all the three accounts at once. This would simplify the specification, e.g., the following sentence would not be required anymore: "Specifically, the sequence number on the SetupAccountTx for the EscrowAccount must be higher than the sequence numbers on the other two transactions."

  2. The two settlement transactions PaymentSettleWithGuestTx and PaymentSettleWithHostTx can be combined into one transaction that would look precisely like CooperativeCloseTx (only with a higher sequence number). This would also simplify the logic about the three transactions SettleOnlyWithHostTx, SettleWithGuestTx, SettleWithHostTx which can then also be combined into one transaction.

@danrobinson
Copy link
Contributor

Thanks for examining the specification! I agree we could be more explicit about the reasons for some of those choices. Both of the choices you're referring to were made to safely handle certain kinds of transaction failure. Since transaction failure causes all operations in that transaction to fail, these operations are separated into separate transactions to ensure that the failure of one of them does not cause the others to fail.

  1. The separation of the SetupAccountTxs into three transactions is because it is possible for a CreateAccount operation to fail if the account already exists. If an attacker (who just wanted to annoy the party) were to create one of the accounts before this transaction hit the ledger (perhaps by seeing the transaction in flight and creating it in a transaction with a higher fee), it would cause the other operations to fail, so the party would have to create the other accounts in a separate transaction (or transactions). To avoid having to implement logic for recovering from this kind of failure, we separate it into three transactions, so that if any of them already exists, only that transaction will fail, and the other accounts will still be created as normal (leaving the ledger in the same state that they expected it to be: with all three accounts created).

  2. The separation of PaymentSettleWithGuestTx and PaymentSettleWithHostTx into two transactions is because a Payment operation fails if the destination account does not exist. If they were combined in one transaction, this would allow one of the parties to grief the other by deleting their account, causing the operation and thus the entire settlement transaction to fail and all money in the account to be lost.

@TorstenStueber
Copy link
Author

Thank you for explaining!

  1. So do you talk about the following scenario: (a) I submit a transaction containing an account creation operation to, say, a horizon server; (b) it is now sent around the peer-to-peer Stellar network even before the next ledger close; (c) someone can observe and send a similar transaction but with a higher fee also before the ledger closes; (d) the other transaction gets into the ledger; (e) my transaction fails because the operation fails.

So is this a general problem in Stellar and one should never combine two createAccount operations in one transaction?

  1. That makes a lot of sense. I think it would be helpful to mention this in the protocol specification. You scratch that topic in the last section "Stellar protocol background" but the connection is not obvious.

@JeremyRubin
Copy link

There's a lot of subtle correctness conditions required for it to work. The stellar documentation and execution semantics are poorly English specified so a lot of this understanding requires reading very closely and checking the code to confirm.

If you want to simplify it, I recommend looking at my CAPs on deterministic accounts, relative timeouts, linear accounts, fee bumping, etc.

Those will help reduce complexity substantially. Specifically, CAP7-8 address your concerns above.

@TorstenStueber
Copy link
Author

Thank you, @JeremyRubin. I coincidentally talked to @bartekn about CAP7 yesterday when discussing the starlight payment channel protocol.

@danrobinson feel free to close this issue. I guess that adding some clarifying remarks to the protocol specification would be helpful for other implementers, though. Otherwise, they might have an urge to "optimize" and combine transactions.

@tuchik
Copy link

tuchik commented Dec 11, 2018

👍

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

No branches or pull requests

4 participants