Add OPT-IN support for SEP0023 #348
Conversation
| } | ||
|
|
||
|
|
||
| public MuxedAccount encode(String account) { |
There was a problem hiding this comment.
This converts an input account string in both G and M-form into a MuxedAccount with only the G-account representation, right? (that is, forcing the Ed25519 discriminant). A docstring here would be 💯
| @@ -47,7 +47,7 @@ public boolean getAuthorize() { | |||
| } | |||
|
|
|||
| @Override | |||
| org.stellar.sdk.xdr.Operation.OperationBody toOperationBody() { | |||
| org.stellar.sdk.xdr.Operation.OperationBody toOperationBody(AccountConverter accountConverter) { | |||
There was a problem hiding this comment.
Forcing this parameter for operations that don't support muxed accounts is a little awkward to me as an API consumer, but I'm not familiar enough with Java patterns to suggest alternatives.
There was a problem hiding this comment.
this method is only used internally. any method which does not have a public modifier is only visible within the SDK.
I just pushed another commit which ensures that, for all public methods that handle account strings, we have two implementations: one which allows you to provide your own account converter and another which uses AccountConverter.disableMuxed() by default
| checkNotNull(sourceAccount, "sourceAccount cannot be null"); | ||
| mSourceAccount = sourceAccount; | ||
| public Builder(AccountConverter accountConverter, TransactionBuilderAccount sourceAccount, Network network) { | ||
| mAccountConverter = checkNotNull(accountConverter, "accountConverter cannot be null"); |
There was a problem hiding this comment.
Maybe a default of null to a non-muxed AccountConverter (since we want opt-in) would be more convenient here as an SDK consumer?
| String source = "MA7QYNF7SOWQ3GLR2BGMZEHXAVIRZA4KVWLTJJFC7MGXUA74P7UJVAAAAAAAAAAAAAJLK"; | ||
| String destination = "MDQNY3PBOJOKYZSRMK2S7LHHGWZIUISD4QORETLMXEWXBI7KFZZMKAAAAAAMV7V2XYGQO"; |
There was a problem hiding this comment.
We should have a test case mixing muxed and non-muxed addresses in the same operation.
| @@ -84,8 +84,8 @@ public void testNewChallengeRejectsMuxedClientAccount() throws InvalidSep10Chall | |||
| timeBounds | |||
| ); | |||
| fail(); | |||
| } catch (FormatException e) { | |||
| assertEquals("Version byte is invalid", e.getMessage()); | |||
| } catch (InvalidSep10ChallengeException e) { | |||
There was a problem hiding this comment.
Isn't this an odd exception for this scenario?
There was a problem hiding this comment.
a sep10 challenge can only be generated using non-muxed accounts. Hence InvalidSep10ChallengeException is thrown when trying to validate a challenge token which has a muxed account
| return StrKey.encodeStellarMuxedAccount(account); | ||
| } | ||
|
|
||
| return StrKey.encodeStellarAccountId(StrKey.muxedAccountToAccountId(account)); |
There was a problem hiding this comment.
Just noting here that this is the same behavior as in the JS SDK: force a fully-muxed account to a G-address when enableMuxed = false ✔️
Previously, in #348 we added opt in support for muxed. But, by default, we rendered muxed accounts in their non muxed encoding. We are now changing the default behavior so that muxed accounts are rendered using ther 'M' address encoding.
Previously, in #348 we added opt in support for muxed. But, by default, we rendered muxed accounts in their non muxed encoding. We are now changing the default behavior so that muxed accounts are rendered using ther 'M' address encoding.
* Support muxed accounts by default. Previously, in #348 we added opt in support for muxed. But, by default, we rendered muxed accounts in their non muxed encoding. We are now changing the default behavior so that muxed accounts are rendered using ther 'M' address encoding.
Add opt-in support for SEP 23. SEP23 adds a strkey representation (M-strkeys) for MuxedAccounts.
Fixes #343