Have a formal definition of sandwich transactions

[howardtw]

Why

We've started to reference to sandwich transactions since the introduction of CAP-33 and CAP-18. We talked about the "sandwich approach" in CAP-33, and we also talked about the authorization sandwich in the control asset access section in our new doc.

Recently one of our partners has expressed the difficulty about communicating the use of sandwich transactions through approval_criteria in the currencies section of SEP-1.

Having a formal definition of sandwich transactions would facilitate such communications.

[howardtw]

Alternatively, if we can make the approval_criteria so that people can specify the transaction composition as one of the criteria, it would also work.

[JakeUrban]

I didn't realize sandwich transactions were used in other protocols.

My thinking was that SEP-8 doesn't make any attempt to define what kind of transactions the client should submit to the approval server, so its left to the organization implementing SEP-8 to communicate this to their clients. This is bad.

Instead, organizations implementing SEP-8 should be able to reference the SEP to their clients, and the SEP should have a format definition of "sandwich transactions". Something like this:

A sandwich transaction is a Stellar transaction containing at minimum 3 operations, where the first and last operations are Allow Trust ops authorizing and deauthorizing the client’s source account. The middle operation(s) are operations the client wants to perform.

The SEP also doesn't seem to mention that trust-lines to regulated assets are auth_required=True by default. In fact, SEP-8 doesn't even make the assumption that the asset is regulated. This seems silly to me because anyone interested in approving transactions for an asset will also want that approval to be mandatory. If the asset isn't regulated, nobody would ask for permission.

[shredding]

I always read approval_criteria as being a more or less legal definition (e.g. we use Only accounts that have passed KYC/AML at DSQ are allowed to trade this asset., not mentioning transaction composition).

We are as well not mentioning sandwich requirements. If the clients submits them, fine, if not we add them and add a revised flag.

Sample (here's the result):

import requests
from stellar_sdk import Server, TransactionBuilder, Network


APPROVAL_SERVER = 'https://dsq.technology/sep-8/tx-approve/'
PUBLIC_KEY = 'GCD2XJSMQT4RHLGMILDJDTCHGP2PWH2RREFNNSW3XLPB7PAVT5UWQBZT'

server = Server(horizon_url='https://horizon.stellar.org')
builder = TransactionBuilder(server.load_account(PUBLIC_KEY), Network.PUBLIC_NETWORK_PASSPHRASE)

tx_xdr = builder.append_manage_buy_offer_op(
    selling_code='USD',
    selling_issuer='GDUKMGUGDZQK6YHYA5Z6AY2G4XDSZPSZ3SW5UN3ARVMO6QSRDWP5YLEX',
    buying_code='TSLA',
    buying_issuer='GBRDHSZL4ZKOI2PTUMM53N3NICZXC5OX3KPCD4WD4NG4XGCBC2ZA3KAG',
    amount='1.00',
    price='402.9721521'
).build().to_xdr()

requests.post(APPROVAL_SERVER, data={'tx': tx_xdr}).json()['tx']

The definition from @JakeUrban looks fine to me, though.

[JakeUrban]

@howardtw this is what I was confused about originally, why can't our partner do this:

We are as well not mentioning sandwich requirements. If the clients submits them, fine, if not we add them and add a revised flag.

Does our partner understand they can do this? Why do they feel the need to describe sandwich transactions in their TOML?

[howardtw]

Are we implicitly saying that clients should always:

1. submit the transaction containing only the middle operations of a sandwich
2. get a revised transaction, which is a sandwich transaction, from the approval server
3. sign and submit the sandwich transaction to the approval server again
4. get the sandwich transaction signed by the approval server
5. submit the transaction to Stellar

This process requires two round-trips, which is not so ideal in my opinion. It would be the best if clients know that they need to construct a sandwich transaction before submitting the transaction to the approval server so that it can be done with just 1 round-trip.

One might argue that the approval server can sign the revised transaction at step 2, but my thought is that it would be more consistent if the approval server will only sign/approve transactions that have been signed by the client. This way, approval servers that approves sandwich transactions can share step 3 ~ 5 with the ones that returns revised transactions.

My understanding is that @shredding has both flows implemented. That is, the approval server will approve sandwich transactions as well as return a revised transaction. However, it is unclear to me that whether the revised transaction is signed before returning to the client or not.

this is what I was confused about originally, why can't our partner do this

They haven't made use of the three intermediate statuses revised/pending/action_required at this moment. Their current implementation is fairly straightforward: the approval server will either approve or reject a transaction.

Does our partner understand they can do this? Why do they feel the need to describe sandwich transactions in their TOML?

I don't think they have thought about using the revised status to make the sandwich. And this is why they feel the need to communicate the approval criteria more clearly.

If we reach a consensus that using the "revised" status to make the sandwich is the way to go, we can tell them about it.

[JakeUrban]

I agree the double round trip is not ideal, and organizations running SEP-8 servers are incentivized to educate clients about sandwich transactions so they don't have to do this. I think a combination of educating our partner about the revised status AND updating the SEP/our docs will be sufficient. Our partner will be able to refer clients to the technical specification instead of having to rely on a text string that may or may not be read.

[shredding]

but my thought is that it would be more consistent if the approval server will only sign/approve transactions that have been signed by the client.

What is the benefit of this? We give back the signed transaction in step 2 and go for it (we add a time bound, but that's not even necessary). If we'd be happy with the transaction in step 4, why not already in step 2?

[howardtw]

What is the benefit of this? We give back the signed transaction in step 2 and go for it (we add a time bound, but that's not even necessary). If we'd be happy with the transaction in step 4, why not already in step 2?

Thanks @shredding! That's good to know. And I agree it's an easy optimization to make. My thought on the approval server in general is that it should be the entity that "approves" a transaction so that the transaction will become submittable. Dismissing a client-supplied transaction and giving the client a self-made transaction that's pre-approved seems a bit aggressive and assertive to me. That's why I feel like it would be better if it could use the "revised" status as a suggestion (transaction not signed).

From software's perspective, I would also favor that the approval server only signs transactions that have been signed by the client for consistency, consistency as in when the approval server would sign a transaction.

[shredding]

It's still a suggestion; it's up to the client to co-sign and broadcast the transaction.

Generally speaking, our SEP-8 implementation is quite complex; e.g. it supports stellar channels and does a lot of checks on the account level. So in general, the job of the approval servers (at least ours) tends to not be to validate transaction composition. That's merely a byproduct. It's approving the intent (in our case trading) of the user and transaction composition is just the way of approving the intent.

At least that's how I understood it.

[howardtw]

So in general, the job of the approval servers (at least ours) tends to not be to validate transaction composition. That's merely a byproduct. It's approving the intent (in our case trading) of the user and transaction composition is just the way of approving the intent.

I agree with you that returning a signed sandwich transaction that wraps the original intent/operations is a way for approval servers (at least yours) to say that it approves the intent, and it's up to the client to co-sign it.

I don't feel strongly about whether the approval server should sign the sandwich transaction or not in step 2. My goal is to make SEP-8 more useful, up-to-date, and more importantly, make sense to most people who implemented or plan on implementing SEP-8 in the ecosystem. And it seems like signing it prior to returning is more useful in your case. Really appreciate your feedback @shredding

I'd love your thoughts on this too @tomerweller @leighmcculloch

[leighmcculloch]

@howardtw In regards to if the server should sign the revised transaction, SEP-8 is explicit about this and states the transaction should be signed so that the client can submit it without making a second request to the server:

stellar-protocol/ecosystem/sep-0008.md

Line 38 in c6c006e

2. *Revised?* Transaction has been revised to be made compliant, and signed by the issuer. Wallet will show the changes to the user and ask them to sign the new transaction. Submit to Horizon once signed.

---

Speaking to the issue title and description, I think it might be helpful if there is developer documentation demonstrating how to group operations in a transaction to temporarily authorize an account for only that transaction rather than writing a formal definition for the term 'sandwich transaction'.

I agree with @shredding here #773 (comment), I wouldn't expect to see deep technical details about how a transaction should be constructed in the approval criteria. The description for approval criteria is a human readable string.

stellar-protocol/ecosystem/sep-0001.md

Line 138 in c6c006e

approval_criteria | string | a human readable string that explains the issuer's requirements for approving transactions.

I think it is simpler and less error prone for clients and servers to implement SEP-8 making use of the revised status rather than require clients to preemptively build the correct transaction upfront, as doing so would bind the client's implementation to the server's and any attempt to generically specify the type of sandwich may be insufficient for all cases. There is still only one request.

What am I missing?

[howardtw]

Based on the conversations in this PR, looks like we all agree on starting with a definition for a sandwich transaction in our developer documentations. And it should improve the words around both CAP-18 and CAP-33.

@JakeUrban is it something that you can take on?