search

elucidsoft / dotnet-stellar-sdk

Stellar API SDK for .NET 6.x
Apache License 2.0
117 stars 54 forks source link

Add OPT-IN support for SEP0023 (Muxed Accounts M-strkeys) #315

Closed 2opremio closed 2 years ago

2opremio commented 3 years ago

TL;DR

Add opt-in support for SEP 23. SEP23 adds a strkey representation (M-strkeys) for MuxedAccounts.

For example, given the MuxedAccount MAQAA5L65LSYH7CQ3VTJ7F3HHLGCL3DSLAR2Y47263D56MNNGHSQSAAAAAAAAAAE2LP26 , you might get the following fields:

source_account: GAQAA5L65LSYH7CQ3VTJ7F3HHLGCL3DSLAR2Y47263D56MNNGHSQSTVY
source_account_muxed: MAQAA5L65LSYH7CQ3VTJ7F3HHLGCL3DSLAR2Y47263D56MNNGHSQSAAAAAAAAAAE2LP26
source_account_muxed_id: 1234

It should be implemented as an opt-in feature

SEP23 is still a Draft. We plan to (at least initally) use the M-strkey representation to explore the use of Muxed Accounts. Later on, we me consider it stable and enable it by default.

As such, it's critical to make M-strkeys an opt-in feature (e.g. only usable by the user through am explicitly enabled flag or other mechanisms).

In particular, no user should be enabling M-addresss unknowingly, either for new or existing code (e.g. through an SDK update).

Recent SEP23 representation change

If you were already familiar with SEP23, it's worth noting that the encoding ordering changed recently at https://github.com/stellar/stellar-protocol/pull/895

The SEP 23 specification already contains that modification.

Reference implementation

You can use the Go SDK implementation as a reference: https://github.com/stellar/go/pull/3527

In the Go SDK we have one entry point into generating transaction envelopes, NewTransaction() . We added a configuration parameter to NewTransaction() which toggles support for muxed accounts. If NewTransaction() is configured with muxed account support then it will interpret "M..." address strings as muxed accounts at the xdr level. However, if NewTransaction() is configured to disable muxed accounts, whenever an "M..." address is provided we will extract the underlying "G..." address from and use that at the XDR level.

We followed a similar approach for the other entry points of the SDK (namely TransactionFromXDR() and NewFeeBumpTransaction()).

fracek commented 3 years ago

It's not clear what the SDK should do. If the user provides a M-address and the option is set to false, we should silently convert it to a G-address every time to serialize to XDR? Likewise, when deserialising a M-address it should be converted to G-address?

2opremio commented 3 years ago

It's not clear what the SDK should do. If the user provides a M-address and the option is set to false, we should silently convert it to a G-address every time to serialize to XDR?

You should error

Likewise, when deserialising a M-address it should be converted to G-address?

When deserializing transactions you should convert silently convert to G-addresses when the user opts-out from M-addreses (which should be the current behavior, and the default one after implementing this issue) and only deserialize to M-addresses when the use opts in.