[TypeScript SDK] Dedupe object ids in transaction builder (#9136)

## Description We need to dedup object ids, otherwise it will fail on the rust side. This PR - implemented dedup - change the interface so that people need to provide hints about `pure` vs `object` ## Test Plan 1. added e2e tests 2. manually tested wallet(send + staking) and explorer(module function execution) ![CleanShot 2023-03-11 at 04 17 40](https://user-images.githubusercontent.com/76067158/224476957-5c8a3bf0-5107-4789-a195-8ce4989e8578.png) ![CleanShot 2023-03-11 at 04 16 25](https://user-images.githubusercontent.com/76067158/224476960-236ce3ef-e4df-4457-9dba-f089edce6735.png) --- If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process. ### Type of Change (Check all that apply) - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes
MystenLabs · Mar 11, 2023 · 47a4ac3 · 47a4ac3
1 parent e3edae8
commit 47a4ac3
Show file tree

Hide file tree

Showing 21 changed files with 204 additions and 105 deletions.
diff --git a/apps/explorer/src/components/module/module-functions-interaction/ModuleFunction.tsx b/apps/explorer/src/components/module/module-functions-interaction/ModuleFunction.tsx
@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import {
+    getPureSerializationType,
     getExecutionStatusType,
     getExecutionStatusError,
     Transaction,
@@ -68,6 +69,7 @@ export function ModuleFunction({
         functionDetails.parameters,
         resolvedTypeArguments
     );
+
     const execute = useMutation({
         mutationFn: async ({ params, types }: TypeOf<typeof argsSchema>) => {
             const tx = new Transaction();
@@ -76,7 +78,15 @@ export function ModuleFunction({
                 Commands.MoveCall({
                     target: `${packageId}::${moduleName}::${functionName}`,
                     typeArguments: types ?? [],
-                    arguments: params?.map((param) => tx.input(param)) ?? [],
+                    arguments:
+                        params?.map((param, i) =>
+                            getPureSerializationType(
+                                functionDetails.parameters[i],
+                                param
+                            )
+                                ? tx.pure(param)
+                                : tx.object(param)
+                        ) ?? [],
                 })
             );
             const result = await signAndExecuteTransaction({ transaction: tx });

diff --git a/apps/explorer/tests/utils/localnet.ts b/apps/explorer/tests/utils/localnet.ts
@@ -30,9 +30,9 @@ export async function mint(address: string) {
         Transaction.Commands.MoveCall({
             target: '0x2::devnet_nft::mint',
             arguments: [
-                tx.input('Example NFT'),
-                tx.input('An example NFT.'),
-                tx.input(
+                tx.pure('Example NFT'),
+                tx.pure('An example NFT.'),
+                tx.pure(
                     'ipfs://bafkreibngqhl3gaa7daob4i2vccziay2jjlp435cf66vhono7nrvww53ty'
                 ),
             ],

diff --git a/apps/wallet/src/ui/app/pages/home/nft-transfer/TransferNFTForm.tsx b/apps/wallet/src/ui/app/pages/home/nft-transfer/TransferNFTForm.tsx
@@ -49,8 +49,8 @@ export function TransferNFTForm({ objectId }: { objectId: string }) {
             tx.setGasBudget(DEFAULT_NFT_TRANSFER_GAS_FEE);
             tx.add(
                 Transaction.Commands.TransferObjects(
-                    [tx.input(objectId)],
-                    tx.input(to)
+                    [tx.object(objectId)],
+                    tx.pure(to)
                 )
             );
             return signer.signAndExecuteTransaction(tx);

diff --git a/apps/wallet/src/ui/app/pages/home/transfer-coin/index.tsx b/apps/wallet/src/ui/app/pages/home/transfer-coin/index.tsx
@@ -62,7 +62,7 @@ function TransferCoinPage() {
                     tx.add(
                         Transaction.Commands.TransferObjects(
                             [tx.gas],
-                            tx.input(formData.to)
+                            tx.pure(formData.to)
                         )
                     );
                     tx.setGasPayment(
@@ -87,36 +87,40 @@ function TransferCoinPage() {
                     const coin = tx.add(
                         Transaction.Commands.SplitCoin(
                             tx.gas,
-                            tx.input(bigIntAmount)
+                            tx.pure(bigIntAmount)
                         )
                     );
                     tx.add(
                         Transaction.Commands.TransferObjects(
                             [coin],
-                            tx.input(formData.to)
+                            tx.pure(formData.to)
                         )
                     );
                 } else {
-                    const primaryCoinInput = tx.input(primaryCoin);
+                    const primaryCoinInput = tx.object(
+                        primaryCoin.coinObjectId
+                    );
                     if (coins.length) {
                         // TODO: This could just merge a subset of coins that meet the balance requirements instead of all of them.
                         tx.add(
                             Transaction.Commands.MergeCoins(
                                 primaryCoinInput,
-                                coins.map((coin) => tx.input(coin.coinObjectId))
+                                coins.map((coin) =>
+                                    tx.object(coin.coinObjectId)
+                                )
                             )
                         );
                     }
                     const coin = tx.add(
                         Transaction.Commands.SplitCoin(
                             primaryCoinInput,
-                            tx.input(bigIntAmount)
+                            tx.pure(bigIntAmount)
                         )
                     );
                     tx.add(
                         Transaction.Commands.TransferObjects(
                             [coin],
-                            tx.input(formData.to)
+                            tx.pure(formData.to)
                         )
                     );
                 }

diff --git a/apps/wallet/src/ui/app/redux/slices/sui-objects/Coin.ts b/apps/wallet/src/ui/app/redux/slices/sui-objects/Coin.ts
@@ -77,6 +77,7 @@ export class Coin {
         );
     }
 
+    // TODO: we should replace this function with the SDK implementation
     /**
      * Stake `amount` of Coin<T> to `validator`. Technically it means user stakes `amount` of Coin<T> to `validator`,
      * such that `validator` will stake the `amount` of Coin<T> for the user.
@@ -102,15 +103,15 @@ export class Coin {
             const tx = new Transaction();
             tx.setGasBudget(DEFAULT_GAS_BUDGET_FOR_STAKE);
             const stakeCoin = tx.add(
-                Transaction.Commands.SplitCoin(tx.gas, tx.input(amount))
+                Transaction.Commands.SplitCoin(tx.gas, tx.pure(amount))
             );
             tx.add(
                 Transaction.Commands.MoveCall({
                     target: '0x2::sui_system::request_add_stake',
                     arguments: [
-                        tx.input(SUI_SYSTEM_STATE_OBJECT_ID),
+                        tx.object(SUI_SYSTEM_STATE_OBJECT_ID),
                         stakeCoin,
-                        tx.input(validator),
+                        tx.pure(validator),
                     ],
                 })
             );
@@ -134,9 +135,9 @@ export class Coin {
                 Transaction.Commands.MoveCall({
                     target: '0x2::sui_system::request_withdraw_stake',
                     arguments: [
-                        tx.input(SUI_SYSTEM_STATE_OBJECT_ID),
-                        tx.input(stake),
-                        tx.input(stakedSuiId),
+                        tx.object(SUI_SYSTEM_STATE_OBJECT_ID),
+                        tx.object(stake),
+                        tx.object(stakedSuiId),
                     ],
                 })
             );

diff --git a/sdk/typescript/src/builder/Commands.ts b/sdk/typescript/src/builder/Commands.ts
@@ -27,6 +27,7 @@ export const TransactionInput = object({
   index: integer(),
   name: optional(string()),
   value: optional(any()),
+  type: optional(union([literal('pure'), literal('object')])),
 });
 export type TransactionInput = Infer<typeof TransactionInput>;
 

diff --git a/sdk/typescript/src/builder/Inputs.ts b/sdk/typescript/src/builder/Inputs.ts
@@ -11,7 +11,7 @@ import {
   string,
   union,
 } from 'superstruct';
-import { SharedObjectRef, SuiObjectRef } from '../types';
+import { ObjectId, SharedObjectRef, SuiObjectRef } from '../types';
 import { builder } from './bcs';
 
 const ObjectArg = union([
@@ -25,28 +25,40 @@ const ObjectArg = union([
   }),
 ]);
 
-export const BuilderCallArg = union([
-  object({ Pure: array(integer()) }),
-  object({ Object: ObjectArg }),
-]);
+export const PureCallArg = object({ Pure: array(integer()) });
+export const ObjectCallArg = object({ Object: ObjectArg });
+export type PureCallArg = Infer<typeof PureCallArg>;
+export type ObjectCallArg = Infer<typeof ObjectCallArg>;
+
+export const BuilderCallArg = union([PureCallArg, ObjectCallArg]);
 export type BuilderCallArg = Infer<typeof BuilderCallArg>;
 
 export const Inputs = {
-  Pure(type: string, data: unknown): BuilderCallArg {
+  Pure(type: string, data: unknown): PureCallArg {
     return { Pure: Array.from(builder.ser(type, data).toBytes()) };
   },
-  ObjectRef(ref: SuiObjectRef): BuilderCallArg {
+  ObjectRef(ref: SuiObjectRef): ObjectCallArg {
     return { Object: { ImmOrOwned: ref } };
   },
-  SharedObjectRef(ref: SharedObjectRef): BuilderCallArg {
+  SharedObjectRef(ref: SharedObjectRef): ObjectCallArg {
     return { Object: { Shared: ref } };
   },
 };
 
+export function getIdFromCallArg(arg: ObjectId | ObjectCallArg) {
+  if (typeof arg === 'string') {
+    return arg;
+  }
+  if ('ImmOrOwned' in arg.Object) {
+    return arg.Object.ImmOrOwned.objectId;
+  }
+  return arg.Object.Shared.objectId;
+}
+
 export function getSharedObjectInput(
   arg: BuilderCallArg,
 ): SharedObjectRef | undefined {
-  return 'Object' in arg && 'Shared' in arg.Object
+  return typeof arg == 'object' && 'Object' in arg && 'Shared' in arg.Object
     ? arg.Object.Shared
     : undefined;
 }

diff --git a/sdk/typescript/src/builder/Transaction.ts b/sdk/typescript/src/builder/Transaction.ts
@@ -10,6 +10,7 @@ import {
   getObjectReference,
   getSharedObjectInitialVersion,
   normalizeSuiObjectId,
+  ObjectId,
   SuiMoveNormalizedType,
   SuiObjectRef,
   SUI_TYPE_ARG,
@@ -22,7 +23,13 @@ import {
   getTransactionCommandType,
   MoveCallCommand,
 } from './Commands';
-import { BuilderCallArg, Inputs } from './Inputs';
+import {
+  BuilderCallArg,
+  getIdFromCallArg,
+  Inputs,
+  isMutableSharedObjectInput,
+  ObjectCallArg,
+} from './Inputs';
 import { getPureSerializationType, isTxContext } from './serializer';
 import {
   TransactionDataBuilder,
@@ -185,23 +192,38 @@ export class Transaction {
    * is the format required for custom serialization.
    *
    */
-  input(value?: unknown) {
-    // For Uint8Array
-    // if (value instanceof Uint8Array) {
-    //   value = { Pure: value };
-    // }
-
+  #input(type: 'object' | 'pure', value?: unknown) {
     const index = this.#transactionData.inputs.length;
-    const input = create({ kind: 'Input', value, index }, TransactionInput);
+    const input = create(
+      { kind: 'Input', value, index, type },
+      TransactionInput,
+    );
     this.#transactionData.inputs.push(input);
     return input;
   }
 
-  // TODO: Do we want to support these helper functions for inputs?
-  // Maybe we can make an `Inputs` helper like commands that works seamlessly with these.
-  // objectRef() {}
-  // sharedObjectRef() {}
-  // pure() {}
+  /**
+   * Add a new object input to the transaction.
+   */
+  object(value: ObjectId | ObjectCallArg) {
+    const id = getIdFromCallArg(value);
+    // deduplicate
+    const inserted = this.#transactionData.inputs.find(
+      (i) => i.type === 'object' && id === getIdFromCallArg(i.value),
+    );
+    return inserted ?? this.#input('object', value);
+  }
+
+  /**
+   * Add a new non-object input to the transaction.
+   *
+   * TODO: take an optional second type parameter here and do the BCS encoding into the
+   * fully-resolved type if folks happen to know the pure type encoding.
+   */
+  pure(value: unknown) {
+    // TODO: we can also do some deduplication here
+    return this.#input('pure', value);
+  }
 
   // TODO: Currently, tx.input() takes in both fully-resolved input values, and partially-resolved input values.
   // We could also simplify the transaction building quite a bit if we force folks to use fully-resolved pure types
@@ -434,7 +456,6 @@ export class Transaction {
 
     if (objectsToResolve.length) {
       const dedupedIds = [...new Set(objectsToResolve.map(({ id }) => id))];
-      // TODO: Use multi-get objects when that API exists instead of batch:
       const objects = await expectProvider(provider).getObjectBatch(
         dedupedIds,
         { showOwner: true },
@@ -461,9 +482,14 @@ export class Transaction {
         const initialSharedVersion = getSharedObjectInitialVersion(object);
 
         if (initialSharedVersion) {
+          // There could be multiple commands that reference the same shared object.
+          // If one of them is a mutable reference, then we should mark the input
+          // as mutable.
           const mutable =
-            normalizedType != null &&
-            extractMutableReference(normalizedType) != null;
+            isMutableSharedObjectInput(input.value) ||
+            (normalizedType != null &&
+              extractMutableReference(normalizedType) != null);
+
           input.value = Inputs.SharedObjectRef({
             objectId: id,
             initialSharedVersion,

diff --git a/sdk/typescript/src/builder/TransactionData.ts b/sdk/typescript/src/builder/TransactionData.ts
@@ -8,6 +8,7 @@ import {
   define,
   Infer,
   integer,
+  is,
   literal,
   nullable,
   object,
@@ -19,7 +20,7 @@ import { sha256Hash } from '../cryptography/hash';
 import { normalizeSuiAddress, SuiObjectRef } from '../types';
 import { builder } from './bcs';
 import { TransactionCommand, TransactionInput } from './Commands';
-import { BuilderCallArg } from './Inputs';
+import { BuilderCallArg, PureCallArg } from './Inputs';
 import { create } from './utils';
 
 export const TransactionExpiration = optional(
@@ -86,7 +87,15 @@ export class TransactionDataBuilder {
         expiration: data.V1.expiration,
         gasConfig: data.V1.gasData,
         inputs: programmableTx.inputs.map((value: unknown, index: number) =>
-          create({ kind: 'Input', value, index }, TransactionInput),
+          create(
+            {
+              kind: 'Input',
+              value,
+              index,
+              type: is(value, PureCallArg) ? 'pure' : 'object',
+            },
+            TransactionInput,
+          ),
         ),
         commands: programmableTx.commands,
       },