From 830b8d85b7ee5f5bb4c140d2d980db9f4e043ae6 Mon Sep 17 00:00:00 2001 From: Manolis Liolios Date: Wed, 18 Sep 2024 20:22:03 +0300 Subject: [PATCH] [SDK][MVR] Move to new naming standard (#19432) ## Description 1. After the recent decision to change our naming scheme to `{suins_name}/{app}/{optional_version}`, I am updating the plugin to also match the expected naming. 2. Also parse `MakeMoveVec` types. ## Test plan Updated tests to match this, added extra test case for `MakeMoveVec` --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API: --- .changeset/gentle-pugs-knock.md | 5 ++ .../plugins/NamedPackagesPlugin.ts | 6 +- .../src/transactions/plugins/utils.ts | 40 ++++++++-- sdk/typescript/src/utils/move-registry.ts | 29 +++++-- .../test/e2e/named-packages-plugin.test.ts | 24 +++--- .../test/unit/utils/move-registry.test.ts | 76 ++++++++++--------- 6 files changed, 120 insertions(+), 60 deletions(-) create mode 100644 .changeset/gentle-pugs-knock.md diff --git a/.changeset/gentle-pugs-knock.md b/.changeset/gentle-pugs-knock.md new file mode 100644 index 0000000000000..199b56145c992 --- /dev/null +++ b/.changeset/gentle-pugs-knock.md @@ -0,0 +1,5 @@ +--- +'@mysten/sui': minor +--- + +Introduce new naming scheme for named packages plugin diff --git a/sdk/typescript/src/transactions/plugins/NamedPackagesPlugin.ts b/sdk/typescript/src/transactions/plugins/NamedPackagesPlugin.ts index eb6d0e2841daa..f063796dfbfb6 100644 --- a/sdk/typescript/src/transactions/plugins/NamedPackagesPlugin.ts +++ b/sdk/typescript/src/transactions/plugins/NamedPackagesPlugin.ts @@ -27,10 +27,10 @@ export type NamedPackagesPluginOptions = { * Expected format example: * { * packages: { - * 'std@framework': '0x1234', + * '@framework/std': '0x1234', * }, * types: { - * 'std@framework::string::String': '0x1234::string::String', + * '@framework/std::string::String': '0x1234::string::String', * }, * } * @@ -42,7 +42,7 @@ export type NamedPackagesPluginOptions = { * @experimental This plugin is in experimental phase and there might be breaking changes in the future * * Adds named resolution so that you can use .move names in your transactions. - * e.g. `app@org::type::Type` will be resolved to `0x1234::type::Type`. + * e.g. `@org/app::type::Type` will be resolved to `0x1234::type::Type`. * This plugin will resolve all names & types in the transaction block. * * To install this plugin globally in your app, use: diff --git a/sdk/typescript/src/transactions/plugins/utils.ts b/sdk/typescript/src/transactions/plugins/utils.ts index cc08781cfb9d2..2d74b34a4b157 100644 --- a/sdk/typescript/src/transactions/plugins/utils.ts +++ b/sdk/typescript/src/transactions/plugins/utils.ts @@ -9,7 +9,7 @@ export type NamedPackagesPluginCache = { types: Record; }; -const NAME_SEPARATOR = '@'; +const NAME_SEPARATOR = '/'; export type NameResolutionRequest = { id: number; @@ -28,6 +28,12 @@ export const findTransactionBlockNames = ( const types: Set = new Set(); for (const command of builder.commands) { + if (command.MakeMoveVec?.type) { + getNamesFromTypeList([command.MakeMoveVec.type]).forEach((type) => { + types.add(type); + }); + continue; + } if (!('MoveCall' in command)) continue; const tx = command.MoveCall; @@ -39,12 +45,9 @@ export const findTransactionBlockNames = ( packages.add(pkg); } - for (const type of tx.typeArguments ?? []) { - if (type.includes(NAME_SEPARATOR)) { - if (!isValidNamedType(type)) throw new Error(`Invalid type with names: ${type}`); - types.add(type); - } - } + getNamesFromTypeList(tx.typeArguments ?? []).forEach((type) => { + types.add(type); + }); } return { @@ -53,12 +56,35 @@ export const findTransactionBlockNames = ( }; }; +/** + * Returns a list of unique types that include a name + * from the given list. + * */ +function getNamesFromTypeList(types: string[]) { + const names = new Set(); + for (const type of types) { + if (type.includes(NAME_SEPARATOR)) { + if (!isValidNamedType(type)) throw new Error(`Invalid type with names: ${type}`); + names.add(type); + } + } + return [...names]; +} + /** * Replace all names & types in a transaction block * with their resolved names/types. */ export const replaceNames = (builder: TransactionDataBuilder, cache: NamedPackagesPluginCache) => { for (const command of builder.commands) { + // Replacements for `MakeMoveVec` commands (that can include types) + if (command.MakeMoveVec?.type) { + if (!command.MakeMoveVec.type.includes(NAME_SEPARATOR)) continue; + if (!cache.types[command.MakeMoveVec.type]) + throw new Error(`No resolution found for type: ${command.MakeMoveVec.type}`); + command.MakeMoveVec.type = cache.types[command.MakeMoveVec.type]; + } + // Replacements for `MoveCall` commands (that can include packages & types) const tx = command.MoveCall; if (!tx) continue; diff --git a/sdk/typescript/src/utils/move-registry.ts b/sdk/typescript/src/utils/move-registry.ts index c374ef695a3a6..17bd8728c44ca 100644 --- a/sdk/typescript/src/utils/move-registry.ts +++ b/sdk/typescript/src/utils/move-registry.ts @@ -1,11 +1,31 @@ // Copyright (c) Mysten Labs, Inc. // SPDX-License-Identifier: Apache-2.0 +import { isValidSuiNSName } from './suins.js'; + /** The pattern to find an optionally versioned name */ -const NAME_PATTERN = /^([a-z0-9]+(?:-[a-z0-9]+)*)@([a-z0-9]+(?:-[a-z0-9]+)*)(?:\/v(\d+))?$/; +const NAME_PATTERN = /^([a-z0-9]+(?:-[a-z0-9]+)*)$/; +/** The pattern for a valid version number */ +const VERSION_REGEX = /^\d+$/; +/** The maximum size for an app */ +const MAX_APP_SIZE = 64; +/** The separator for the name */ +const NAME_SEPARATOR = '/'; export const isValidNamedPackage = (name: string): boolean => { - return NAME_PATTERN.test(name); + const parts = name.split(NAME_SEPARATOR); + // The name has to have 2 parts (without-version), or 3 parts (with version). + if (parts.length < 2 || parts.length > 3) return false; + + const [org, app, version] = parts; // split by {org} {app} {optional version} + + // If the version exists, it must be a number. + if (version !== undefined && !VERSION_REGEX.test(version)) return false; + // Check if the org is a valid SuiNS name. + if (!isValidSuiNSName(org)) return false; + + // Check if the app is a valid name. + return NAME_PATTERN.test(app) && app.length < MAX_APP_SIZE; }; /** @@ -16,9 +36,8 @@ export const isValidNamedType = (type: string): boolean => { // split our type by all possible type delimeters. const splitType = type.split(/::|<|>|,/); for (const t of splitType) { - if (t.includes('@') && !isValidNamedPackage(t)) return false; + if (t.includes(NAME_SEPARATOR) && !isValidNamedPackage(t)) return false; } - // TODO: Add `isValidStructTag` check once - // it's generally introduced. + // TODO: Add `isValidStructTag` check once it's introduced. return true; }; diff --git a/sdk/typescript/test/e2e/named-packages-plugin.test.ts b/sdk/typescript/test/e2e/named-packages-plugin.test.ts index c2c18c02d4dbb..f60a65c4ccf1f 100644 --- a/sdk/typescript/test/e2e/named-packages-plugin.test.ts +++ b/sdk/typescript/test/e2e/named-packages-plugin.test.ts @@ -15,12 +15,12 @@ Transaction.registerGlobalSerializationPlugin( }), overrides: { packages: { - 'std@framework': '0x1', - 'std@framework/v1': '0x1', + '@framework/std': '0x1', + '@framework/std/1': '0x1', }, types: { - 'std@framework::string::String': '0x1::string::String', - 'std@framework::vector::empty': + '@framework/std::string::String': '0x1::string::String', + '@framework/std::vector::empty<@framework/std::string::String>': '0x1::vector::empty<0x1::string::String>', }, }, @@ -33,20 +33,26 @@ describe('Name Resolution Plugin (.move)', () => { // replace .move names properly transaction.moveCall({ - target: 'std@framework::string::utf8', + target: '@framework/std::string::utf8', arguments: [transaction.pure.string('Hello, world!')], }); // replace type args properly transaction.moveCall({ - target: 'std@framework::vector::empty', - typeArguments: ['std@framework::string::String'], + target: '@framework/std::vector::empty', + typeArguments: ['@framework/std::string::String'], }); // replace nested type args properly transaction.moveCall({ - target: 'std@framework/v1::vector::empty', - typeArguments: ['std@framework::vector::empty'], + target: '@framework/std/1::vector::empty', + typeArguments: ['@framework/std::vector::empty<@framework/std::string::String>'], + }); + + // replace type args in `MakeMoveVec` call. + transaction.makeMoveVec({ + type: '@framework/std::string::String', + elements: [transaction.pure.string('Hello, world!')], }); const json = JSON.parse(await transaction.toJSON()); diff --git a/sdk/typescript/test/unit/utils/move-registry.test.ts b/sdk/typescript/test/unit/utils/move-registry.test.ts index 51d4ad3cbe0a8..6a88b0a9e5829 100644 --- a/sdk/typescript/test/unit/utils/move-registry.test.ts +++ b/sdk/typescript/test/unit/utils/move-registry.test.ts @@ -7,48 +7,52 @@ import { isValidNamedPackage, isValidNamedType } from '../../../src/utils'; describe('isValidNamedPackage', () => { test('Valid/Invalid .move names', () => { - expect(isValidNamedPackage('app@org')).toBe(true); - expect(isValidNamedPackage('app@org/v1')).toBe(true); - expect(isValidNamedPackage('app@org/v123')).toBe(true); - expect(isValidNamedPackage('app-test@org/v1')).toBe(true); - expect(isValidNamedPackage('1app@org/v1')).toBe(true); - expect(isValidNamedPackage('1-app@org/v1')).toBe(true); - expect(isValidNamedPackage('test@o-rg/v1')).toBe(true); + expect(isValidNamedPackage('@org/app')).toBe(true); + expect(isValidNamedPackage('@org/app/1')).toBe(true); + expect(isValidNamedPackage('@org/app/123')).toBe(true); + expect(isValidNamedPackage('@org/app-test/1')).toBe(true); + expect(isValidNamedPackage('@org/1app/1')).toBe(true); + expect(isValidNamedPackage('@org/1-app/1')).toBe(true); + expect(isValidNamedPackage('test@o-rg/1')).toBe(true); + expect(isValidNamedPackage('org.sui/app')).toBe(true); + expect(isValidNamedPackage('org.sui/app/1')).toBe(true); // failed scenarios. - expect(isValidNamedPackage('app.test@org/v123')).toBe(false); - expect(isValidNamedPackage('app@org/v')).toBe(false); - expect(isValidNamedPackage('app@org/v1/')).toBe(false); - expect(isValidNamedPackage('app@org/v1.')).toBe(false); - expect(isValidNamedPackage('app@org/v1.2')).toBe(false); - expect(isValidNamedPackage('app@org/v1.2.3')).toBe(false); - expect(isValidNamedPackage('@org/v1')).toBe(false); - expect(isValidNamedPackage('-org/v1')).toBe(false); - expect(isValidNamedPackage('.org/v1')).toBe(false); - expect(isValidNamedPackage('org/v1')).toBe(false); - expect(isValidNamedPackage('-test@1org/v1')).toBe(false); - expect(isValidNamedPackage('test@-org/v1')).toBe(false); - expect(isValidNamedPackage('test@o--rg/v1')).toBe(false); - expect(isValidNamedPackage('test@o-rg/v1-')).toBe(false); - expect(isValidNamedPackage('test@o-rg/v1@')).toBe(false); - expect(isValidNamedPackage('tes--t@org/v1')).toBe(false); + expect(isValidNamedPackage('@org/app.test/123')).toBe(false); + expect(isValidNamedPackage('@org/app/v')).toBe(false); + expect(isValidNamedPackage('@org/app/1/')).toBe(false); + expect(isValidNamedPackage('@org/app/1.')).toBe(false); + expect(isValidNamedPackage('@org/app/1.2')).toBe(false); + expect(isValidNamedPackage('@org/app/1.2.3')).toBe(false); + expect(isValidNamedPackage('@org1')).toBe(false); + expect(isValidNamedPackage('-org/1')).toBe(false); + expect(isValidNamedPackage('.org/1')).toBe(false); + expect(isValidNamedPackage('org/1')).toBe(false); + expect(isValidNamedPackage('@1org/-test/1')).toBe(false); + expect(isValidNamedPackage('@-org/test/1')).toBe(false); + expect(isValidNamedPackage('@o--rgtest/1')).toBe(false); + expect(isValidNamedPackage('@o-rg/test/1-')).toBe(false); + expect(isValidNamedPackage('@o-rg/test/1@')).toBe(false); + expect(isValidNamedPackage('@org/tes--t/1')).toBe(false); + expect(isValidNamedPackage('app@org')).toBe(false); }); test('Valid/Invalid .move types', () => { - expect(isValidNamedType('app@org::string::String')).toBe(true); - expect(isValidNamedType('app@org/v1::string::String')).toBe(true); - expect(isValidNamedType('app@org/v123::string::String')).toBe(true); - expect(isValidNamedType('app-test@org/v1::string::String')).toBe(true); - expect(isValidNamedType('1app@org/v1::string::String')).toBe(true); - expect(isValidNamedType('1-app@org/v1::string::String')).toBe(true); + expect(isValidNamedType('@org/app::string::String')).toBe(true); + expect(isValidNamedType('@org/app/1::string::String')).toBe(true); + expect(isValidNamedType('@org/app/123::string::String')).toBe(true); + expect(isValidNamedType('@org/app-test/1::string::String')).toBe(true); + expect(isValidNamedType('@org/1app/1::string::String')).toBe(true); + expect(isValidNamedType('@org/1-app/1::string::String')).toBe(true); // failed scenarios. - expect(isValidNamedType('app.test@org/v123::string::String')).toBe(false); - expect(isValidNamedType('app@org/v::string::String')).toBe(false); - expect(isValidNamedType('app@org/v1/::string::String')).toBe(false); - expect(isValidNamedType('app@org/v1.::string::String')).toBe(false); - expect(isValidNamedType('app@org/v1.2::string::String')).toBe(false); - expect(isValidNamedType('--app@org::string::String')).toBe(false); - expect(isValidNamedType('ap--p@org::string::String')).toBe(false); + expect(isValidNamedType('@org/app.test/123::string::String')).toBe(false); + expect(isValidNamedType('@org/app/v::string::String')).toBe(false); + expect(isValidNamedType('@org/app/::string::String')).toBe(false); + expect(isValidNamedType('@org/app/1/::string::String')).toBe(false); + expect(isValidNamedType('@org/app/1.::string::String')).toBe(false); + expect(isValidNamedType('@org/app/1.2::string::String')).toBe(false); + expect(isValidNamedType('@org/app--::string::String')).toBe(false); + expect(isValidNamedType('@org/ap--p::string::String')).toBe(false); }); });