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

Support muxed accounts by default. #399

Merged
merged 2 commits into from
Jan 10, 2022
Merged

Conversation

tamirms
Copy link
Contributor

@tamirms tamirms commented Jan 7, 2022

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.

Fixes #397

@sreuland
Copy link
Contributor

sreuland commented Jan 7, 2022

looks good, really enabled by the groundwork done earlier too. Just to confirm understanding, there's no way in sdk usage to disable Muxed account encoding right, the 'switch' here is hard-coded.

[edit], if configurable opt-out for muxed was needed, AccountConverter pretty much has it, minor tweaks to treat it like a jvm singleton would probably suffice:

public AccountConverter {
  private static boolean gEnableMuxed = true;
  private static AccountConverter instance;

  public static AccountConverter getInstance() {
     if (instance == null) {
         synchronized (AccountConverter.class) {
             if (instance == null) {
                 instance = new AccountConverter(gEnableMuxed);
              }
         }
     }
     return instance;
  }

  public static disableMuxed() {
     gEnabledMuxed = false;
  }

  ... remove enabledMuxed(), disableMuxed()

}

replace invocations of enabledMuxed() and disabledMuxed with getInstance(), SDK users would need to optionally call AccountConverter.dsableMuxed() once before invoking anything else in the SDK to disable, i.e. 'opt out' of muxed account encodings from that point on in the jvm.

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.
@tamirms tamirms merged commit ae4f0d1 into master Jan 10, 2022
@tamirms tamirms deleted the support-muxed-by-default branch January 10, 2022 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support muxed addresses (SEP-23) by default.
2 participants