Refactor the account info output type to be compatible with `AIP-62` standard #441

0xmaayan · 2024-10-30T20:39:02Z

To support the different Public Key types and for developers to easily be able to pass the account info down to SDK functions, changing the Account Info return type to be compatible with AIP-62

An updated version with this change here: https://aptos-labs.github.io/aptos-wallet-adapter/nextjs-example-testing/

Will be released with a major version update

OLD

export type AccountInfo = {
  address: string;
  publicKey: string | string[];
  minKeysRequired?: number;
  ansName?: string | null;
};

NEW

export type AccountInfoOutput = {
  address: AccountAddress;
  publicKey: PublicKey | PublicKey[]; // PublicKey[] for backward compatibility
  minKeysRequired?: number;
  ansName?: string | null;
};

This way, one can do

  const signWithSDK = async () => {
    if (!account) return;

    // make sure it is not an array of public keys
    if (Array.isArray(account.publicKey)) return;

    const transaction = await aptosClient(network).transaction.build.simple({
      sender: account.address,
      data: {
        function: "0x1::coin::transfer",
        typeArguments: [APTOS_COIN],
        functionArguments: [account.address.toString(), 1], // 1 is in Octas
      },
    });
    const response = await aptosClient(network).transaction.simulate.simple({
      signerPublicKey: account.publicKey,
      transaction,
    });
  };

packages/wallet-adapter-core/src/WalletCore.ts

GhostWalker562 · 2024-10-30T21:50:44Z

packages/wallet-adapter-core/src/LegacyWalletPlugins/types.ts

@@ -74,7 +79,7 @@ export declare interface WalletCoreEvents {
  readyStateChange(wallet: Wallet): void;
  standardWalletsAdded(wallets: Wallet | AptosStandardSupportedWallet): void;
  networkChange(network: NetworkInfo | null): void;
-  accountChange(account: AccountInfo | null): void;
+  accountChange(account: AccountInfo | StandardAccountInfoInput | null): void;


Do you think it might be better to just share the AccountInfoOutput instead?

This is tricky as the adapter still supports legacy and new standard types, so the account the adapter gets back from the wallet can be of both types. Changing it gets us into a type-rabbit-hole, so we simply change the actual input type the adapter returns.

In the near future, we should probably support only AIP-62 wallets

I think we should use AccountInfo from @aptos-labs/wallet-standard here, that uses the PublicKey type.

If I understand correctly, this is an event that can be listened to externally by the dapp.
Since we're bumping the major, we might as well clean up the interface and have the "right" type here.

In case of legacy plugins, we should be able to adapt the return types to AccountInfo so that the dapp sees one consistent return type.

EDIT: here's a good spot where we could normalize

hardsetting

Thanks for making this change!
As per my comments, I think we should try using AccountInfo from the standard :)

packages/wallet-adapter-core/src/WalletCore.ts

hardsetting · 2024-10-31T02:21:23Z

packages/wallet-adapter-core/src/LegacyWalletPlugins/types.ts

@@ -74,7 +79,7 @@ export declare interface WalletCoreEvents {
  readyStateChange(wallet: Wallet): void;
  standardWalletsAdded(wallets: Wallet | AptosStandardSupportedWallet): void;
  networkChange(network: NetworkInfo | null): void;
-  accountChange(account: AccountInfo | null): void;
+  accountChange(account: AccountInfo | StandardAccountInfoInput | null): void;


I think we should use AccountInfo from @aptos-labs/wallet-standard here, that uses the PublicKey type.

If I understand correctly, this is an event that can be listened to externally by the dapp.
Since we're bumping the major, we might as well clean up the interface and have the "right" type here.

In case of legacy plugins, we should be able to adapt the return types to AccountInfo so that the dapp sees one consistent return type.

EDIT: here's a good spot where we could normalize

hardsetting · 2024-10-31T02:28:04Z

packages/wallet-adapter-core/src/WalletCore.ts

@@ -131,7 +131,7 @@ export class WalletCore extends EventEmitter<WalletCoreEvents> {
  private _wallet: Wallet | null = null;

  // Current connected account
-  private _account: AccountInfo | null = null;
+  private _account: AccountInfo | StandardAccountInfoInput | null = null;


Same here, let's just use AccountInfo from the standard. We can convert it to the right type before setting its value

hardsetting · 2024-10-31T02:28:20Z

packages/wallet-adapter-core/src/WalletCore.ts

@@ -465,7 +465,7 @@ export class WalletCore extends EventEmitter<WalletCoreEvents> {
   * @param account An account
   */
  private ensureAccountExists(
-    account: AccountInfo | null
+    account: AccountInfo | StandardAccountInfoInput | null


hardsetting · 2024-10-31T02:29:26Z

packages/wallet-adapter-core/src/WalletCore.ts

+  setAccount(account: AccountInfo | StandardAccountInfo | null): void {
+    this._account = account;
  }


This would be a good spot where we support both AccountInfos (the old internal type, and the standard type) as inputs, and normalize it to the standard one.

hardsetting · 2024-10-31T02:30:27Z

packages/wallet-adapter-core/src/WalletCore.ts

@@ -687,9 +642,30 @@ export class WalletCore extends EventEmitter<WalletCoreEvents> {
   * @return account info
   * @throws WalletAccountError
   */
-  get account(): AccountInfo | null {
+  get account(): AccountInfoOutput | null {


Let's just return AccountInfo from the wallet standard package

hardsetting · 2024-10-31T02:32:03Z

packages/wallet-adapter-core/src/WalletCore.ts

+            : // for backward compatibility, if the account public key is of type `string[]` convert each string to Ed25519PublicKey
+              Array.isArray(this._account.publicKey) &&
+                this._account.publicKey.every(
+                  (item) => typeof item === "string"
+                )
+              ? this._account.publicKey.map((key) => new Ed25519PublicKey(key))


Ideally we should convert string[] and minKeysRequired to MultiEd25519PublicKey that extends PublicKey

hardsetting · 2024-10-31T02:33:40Z

apps/nextjs-example/src/app/page.tsx

@@ -147,7 +150,7 @@ function WalletSelection() {
 }

 interface WalletConnectionProps {
-  account: AccountInfo | null;
+  account: AccountInfoOutput | null;


let's use AccountInfo from the standard

hardsetting · 2024-10-31T02:34:58Z

packages/wallet-adapter-core/src/utils/walletSelector.ts

@@ -44,9 +45,9 @@ export function isInstallRequired(wallet: AnyAptosWallet) {
 }

 /** Truncates the provided wallet address at the middle with an ellipsis. */
-export function truncateAddress(address: string | undefined) {
+export function truncateAddress(address: AccountAddress | undefined) {


small nit: we could use AccountAddressInput, but nbd

hardsetting · 2024-10-31T02:46:59Z

By quickly looking at the code, I think we could do this:

define a normalizeAccountInfo that normalizes any AccountInfo to the standard's one. Here you would put the code that converts string to Ed25519PublicKey and (string[], minKeysRequired) to MultiEd25519PublicKey
when calling connectWallet here, we normalize the account info before calling setAccount
when we receive an "accountChange" event here, we normalize the account info before setAccount and emit
setAccount should only take the standard's AccountInfo as input

basically.. let's normalize as soon as we have a chance to make our life easier :)

0xmaayan requested review from kaw2k, hardsetting and kent-white as code owners October 30, 2024 20:39

0xmaayan requested a review from GhostWalker562 October 30, 2024 20:40

0xmaayan changed the title ~~Refactor the account info output type to be compatible with API-62 standard~~ Refactor the account info output type to be compatible with AIP-62 standard Oct 30, 2024

GhostWalker562 reviewed Oct 30, 2024

View reviewed changes

Refactor the account output type to match API-62 standard

3eec0de

0xmaayan force-pushed the account_output_type_aip_62 branch from 97d2efe to 3eec0de Compare October 30, 2024 22:52

GhostWalker562 approved these changes Oct 30, 2024

View reviewed changes

hardsetting requested changes Oct 31, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor the account info output type to be compatible with `AIP-62` standard #441

Refactor the account info output type to be compatible with `AIP-62` standard #441

0xmaayan commented Oct 30, 2024

GhostWalker562 Oct 30, 2024

0xmaayan Oct 30, 2024

hardsetting Oct 31, 2024 •

edited

Loading

hardsetting left a comment

hardsetting Oct 31, 2024 •

edited

Loading

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting Oct 31, 2024

hardsetting commented Oct 31, 2024 •

edited

Loading

Refactor the account info output type to be compatible with AIP-62 standard #441

Are you sure you want to change the base?

Refactor the account info output type to be compatible with AIP-62 standard #441

Conversation

0xmaayan commented Oct 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardsetting Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

hardsetting left a comment

Choose a reason for hiding this comment

hardsetting Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hardsetting commented Oct 31, 2024 • edited Loading

Refactor the account info output type to be compatible with `AIP-62` standard #441

Refactor the account info output type to be compatible with `AIP-62` standard #441

hardsetting Oct 31, 2024 •

edited

Loading

hardsetting Oct 31, 2024 •

edited

Loading

hardsetting commented Oct 31, 2024 •

edited

Loading