stellar-protocol icon indicating copy to clipboard operation
stellar-protocol copied to clipboard

Merging account with trustlines

Open TorstenStueber opened this issue 5 years ago • 11 comments

In order to merge an account that has a trustline first the following operations need to be executed:

  • a payment operation that transfers the remaining balance of the trusted asset to some other account
  • a change trust operation to remove the trustline

Certain smart contract schemes (such as payment channels) involve refund transactions for escrow accounts that are created and signed well before they are submitted on chain. If such a scheme deals with an assets different from the native asset, then these refund transactions need to contain the following operations (as stated above):

  • a payment operation to clear the balance of that asset
  • remove the trustline of that asset
  • merge account

The refund transaction will fail if the payment operation does not contain the correct balance of the trusted asset at the time the transaction is submitted. Since the refund transaction is created and signed a long time before, this would allow an attacker to invalidate the transaction by sending a single stroop of the trusted asset to the escrow account before the refund transaction is submitted.

A workaround would be to always set the trust limit of the asset to the current balance on the account. However, this has some downsides – e.g., topping up a payment channel would not be possible.

We propose any of the following solutions:

  1. Extend the accountMerge operation: if the account to be merged has trustlines and the account to be merged into has the same trustlines, then merge all assets into the latter account.
  2. Add a "merge asset" operation that behaves like a payment operation but does not specify an amount – it will transfer the complete remaining amount of the specified asset on the source account instead.

TorstenStueber avatar Jun 11 '19 14:06 TorstenStueber

For reference, https://github.com/stellar/stellar-protocol/issues/56

johansten avatar Jun 13 '19 01:06 johansten

Agree that this is tricky. I believe the two mechanisms you propose still leave some corner cases. For example, the merge asset operation could exceed the trustline or could exceed 2^{63}, or authorization could be revoked by an asset issuer. I think you need two transactions to close a channel (in case one fails because a party does something weird like delete their account or trustline), and it's better just to change the signers on the escrow account to put it under one party's control than to try to merge that account.

stanford-scs avatar Feb 18 '20 18:02 stanford-scs

You mentioned a third alternative that would also work and is currently the only practical solution to that problem (it's also the only "workaround" we found). However, it has big legal implications if neither party is allowed to get complete control over the escrow account – they might require certain licenses first.

Therefore it is very critical for legal reasons to extend the protocol in the sense of one of the first two alternatives I mentioned.

TorstenStueber avatar Feb 20 '20 11:02 TorstenStueber

I believe the two mechanisms you propose still leave some corner cases.

There may be corner cases, but there is also the happy path, where it is indeed possible to do a "full" merge. Does it not make sense to accommodate this scenario?

erasmus avatar Feb 20 '20 11:02 erasmus

@TorstenStueber did you look at leveraging CAP-0023 that we're trying to finalize in the next couple months? It allows to decouple transfers from accounts and seems to have strong desirable properties for smart contracts in general (and we think for payment channels too). If it doesn't work in your particular scenario, we'd like to hear the blockers and potentially incorporate new changes into CAP-0023.

MonsieurNicolas avatar Feb 23 '20 01:02 MonsieurNicolas

@MonsieurNicolas I'm not sure CAP-0023 will solve our problem, but perhaps I'm missing something here. The issue is not the receiving account, the problem is guaranteeing that the escrow account is fully merged.

Example:

  1. In order to avoid gaining control of our customer's funds (control of customer funds would impose additional legal/compliance requirements on us), we put customer funds on an escrow account during a transfer. The escrow account is multi-sig with a time bound and it tends to get multiple trust lines with positive asset balances.
  2. At the time of creating the escrow account, we also need the customer to sign refund tx which will execute at the expiry of the maxTime limit. (This is a safety measure to ensure the funds will not get lost in the case that the customer loses their secret key)
  3. This signed refund tx will fail, if additional assets are sent to the account in the time between signing and the time bound expiry. That is: the payment operation that clears a trust line asset from the account has to specify the exact amount of assets to transfer, and as long as that value can change, there is no guarantee that the operation will really clear the asset. In that case, the asset balance will be positive and the account merge operation will fail.

We're looking for a robust way to fully merge an account without having to specify the exact balance of each trust line asset. It is not an option to let one one account take full control of the account at any point in time.

erasmus avatar Feb 25 '20 09:02 erasmus

@MonsieurNicolas I agree with what @erasmus said. CAP-0023 does not solve our problem. CAP-0023 would be helpful in a case where the target account that the escrow is to be merged into might not have the proper trust lines (yet).

But this is not our problem: the target account of the merge operation does have the required trust lines. See @erasmus explanation for some more details.

TorstenStueber avatar Mar 02 '20 17:03 TorstenStueber

I was not saying that CAP0023 will make merging accounts easier, but that it should make writing payment channels easier.

In the context of payment channels, as transactions can fail, you really don't want to have any of the pre-signed transactions fail (a "merge asset" operation can fail) as the failure modes get very tricky to handle as the number of participants or assets increases.

CAP0023 allows to do just that (we think): as we decouple what the channel does with pre-signed transactions (creates pending transfers, this cannot fail), and what each recipient does ("claims" can fail but can be retried).

MonsieurNicolas avatar Mar 02 '20 19:03 MonsieurNicolas

I understand and that makes sense. But wouldn't that still require the kind of operations I am asking for in my initial post? So maybe CAP0023 would be an additional improvement but we still need some way to merge assets. Or am I missing something?

TorstenStueber avatar Mar 10 '20 11:03 TorstenStueber

yeah merging assets becomes interactive and doesn't need to be done from within the smart contract @TorstenStueber so the existing operations should be enough

MonsieurNicolas avatar Mar 12 '20 22:03 MonsieurNicolas

@TorstenStueber what is your current status on this issue? Do you think that there are still fundamental problems here? I have some ideas based on what has been discussed in this thread (although I would want to make sure that I understand the problem fully before proposing any solutions), but obviously if the problem has been resolved then there is nothing of value for me to add.

jonjove avatar Aug 24 '20 15:08 jonjove