stellar / stellar-protocol

Developer discussion about possible changes to the protocol.
517 stars 302 forks source link

Merging account with trustlines #324

Open TorstenStueber opened 5 years ago

TorstenStueber commented 5 years ago

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

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):

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.
johansten commented 5 years ago

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

stanford-scs commented 4 years ago

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.

TorstenStueber commented 4 years ago

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.

erasmus commented 4 years ago

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?

MonsieurNicolas commented 4 years ago

@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.

erasmus commented 4 years ago

@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.

TorstenStueber commented 4 years ago

@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.

MonsieurNicolas commented 4 years ago

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).

TorstenStueber commented 4 years ago

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?

MonsieurNicolas commented 4 years ago

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

jonjove commented 4 years ago

@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.