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

Reverse Merge Operation (or Rename) #276

Open
JeremyRubin opened this issue Mar 19, 2019 · 3 comments
Open

Reverse Merge Operation (or Rename) #276

JeremyRubin opened this issue Mar 19, 2019 · 3 comments
Labels
CAP Represents an issue that requires a CAP. needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process.

Comments

@JeremyRubin
Copy link
Contributor

Rough proposal of a new operation for a reverse merge, can open a SEP after discussion.

In a reverse merge, the operation specifies a target account ID. If the target account ID already exists, and the account has no sub-entries which preclude merging, the remaining lumen balance is sent to the source account and the account is deleted. Then, the source account's account ID becomes the max(target account ID's, current sequence number).

If the account is in a non-mergeable state, the transaction should be invalid (or, fail).

Such an operation is useful for SEP protocols like ATP3 where we might want funds to end up in a specific account without having to do much work of changing trustlines.

Open questions:

  • Should this copy over the target's signers?
  • Should the target be required to include a signature?

It also lets us rotate our account's masterkey in cases where we've lost it or forgotten it.

@tomquisel
Copy link
Contributor

Hey @JeremyRubin! This sounds reasonable. Two questions:

  • Why does it change the source account's ID? It seems like that makes it difficult for users to keep track of what their account ID is.
  • Does this operation move trustlines & non-XLM balances over?

@JeremyRubin
Copy link
Contributor Author

I'm fine with having either semantic as to which account gets changed. I was just making it the opposite of Account Merge because this is a reverse merge.

We may or may not be able to move subentries -- would need careful study.

@theaeolianmachine theaeolianmachine added CAP Represents an issue that requires a CAP. needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process. labels Mar 20, 2019
@MonsieurNicolas
Copy link
Contributor

So what we have is

Before:

  • Account S has a balance S_balance and trustlines T1, T2, ...
  • Account T (if it exists) only has a balance B_balance

then execute

ReverseMergeOp:
   source: S
   destination: T

After:

  • T has balance_A+balance_B, trustlines T1, ...
  • S is deleted

re: the name, maybe I am missing something but this seems to be the exact same thing than allowing mergeaccount to merge an account with trustlines into a destination that has no trustlines and (potentially) also move signers? So "MergeAccountAll" might be simpler to understand?

I don't see why we would necessarily fail if target is non empty... we're OK with failing the operation if T is not in the right state anyways.

Should the target be required to include a signature?

If we do not check signatures, this makes it possible to attack T, as mergeAccount with trustlines will fail (we still need to address that broader problem).

If we require both signatures, we can probably decompose further (maybe introduce a limitPayment operation that sends to up to a limit to a destination - can be used also in the context of closing channels by using "max" as the limit) and ideally not have any particular operation that needs signature for both source and destination (now we'd have high source, medium destination signatures required, this seems bad?!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CAP Represents an issue that requires a CAP. needs draft This an issue that has no corresponding draft, and as such has not entered the CAP/SEP process.
Projects
None yet
Development

No branches or pull requests

5 participants
@theaeolianmachine @JeremyRubin @tomquisel @MonsieurNicolas and others