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

Feature Request (txnbuild): (Re-)implement multiplexed accounts in the SDK #3490

Closed
ire-and-curses opened this issue Mar 23, 2021 · 3 comments · Fixed by #3527
Closed

Feature Request (txnbuild): (Re-)implement multiplexed accounts in the SDK #3490

ire-and-curses opened this issue Mar 23, 2021 · 3 comments · Fixed by #3527
Assignees
Labels
feature request txnbuild 2nd-generation transaction build library for Go SDK
Milestone

Comments

@ire-and-curses
Copy link
Member

ire-and-curses commented Mar 23, 2021

What problem does your feature solve?

Multiplexed account support was implemented in the Stellar protocol in Protocol 13. Full ecosystem support was not implemented because the lift was large: see stellar/stellar-protocol#617.

Adding SDK support (only) for multiplexed addresses now will allow developers to experiment and identify valuable concrete use cases to drive full adoption (rationale). This issue does not propose adding any support to Horizon for M addresses.

What would you like to see?

M address support should default to off, so most users will see no functionality change.

  • expand G address validation to also cover M (opt-in)
  • allow construction of M addresses (opt-in)
  • allow M addresses in transactions in place of G, where allowed (opt in)
  • decode an M address from a provided hash (opt in)
  • API helpers for comparing M addresses and extracting underlying G addresses

One complication: M addresses are not fully interchangeable with G. If you grep for AccountID in Stellar-transaction.x, you'll see the places where AccountID was preserved. These are: CreateAccount, SetOptions, AllowTrust, and SponsoringFutureReserves, as well as various operation results. The parsing implementation should disallow such invalid address uses.

Note also: This change to SEP 23 will change the strkey implementation.

What alternatives are there?

Prior work was done and then removed here: #2612. Based on the requirements above it is probably not sufficient.

Once the implementation is clear, we can raise equivalent issues on the other SDKs.

@ire-and-curses ire-and-curses added txnbuild 2nd-generation transaction build library for Go SDK feature request labels Mar 23, 2021
@MonsieurNicolas
Copy link
Contributor

yeah when "off" you want to ensure that we never sees muxed accounts in either direction:

  • never produce a multiplexed address in the object model. From string, but also from XDR binary form.
    • so you need to reject (from string) or "downgrade" (by transforming into a regular AccountID) multiplexed addresses coming from any place
    • note that when turned "on" there should never be a downgrade as a muxed account is not the same thing than a non muxed account
  • never produce a multiplexed address from the object model. Basically avoid creating xdr (or other) that represent a muxed account ID
    • if there is no way to represent a muxed address in the object model this is trivial, but needs to be called out (for JS it might be necessary to have a check later in case of bugs in the business logic that used muxed accounts with the feature flag off)

@2opremio 2opremio self-assigned this Mar 24, 2021
@bartekn bartekn added this to the Horizon 2.2.0 milestone Mar 31, 2021
@2opremio
Copy link
Contributor

2opremio commented Apr 5, 2021

@MonsieurNicolas I generally agree with your comment (if by object model you mean SDK datastructures)

That said, (if only for backwards compatibility) we may not reach a perfect solution. Muxed accounts can already be leaked today (e.g. if you readily provide XDR for submitting a transaction, bypassing the txnbuild datastructures). I am still diving through the code, but there may be other cases.

@MonsieurNicolas
Copy link
Contributor

yes exactly @2opremio : what I was trying to say is that we can make this safe for 99% of the developers that use the SDK.
For people that do things directly with XDR (like hardware wallets), they will have to support it (and should already but probably don't).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request txnbuild 2nd-generation transaction build library for Go SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants