From 89e4cba131d5f41613b0cefd9adcf586adfead01 Mon Sep 17 00:00:00 2001 From: Jaco Date: Wed, 15 Jun 2022 10:23:38 +0200 Subject: [PATCH] Adjust PortableRegistry name extraction (#4938) * Adjust PortableRegistry name extraction * Adjust contract tests * Adjust * Adjust --- .../src/test/compare/ink_v0_erc20.test.json | 1 + .../src/test/compare/ink_v0_erc721.test.json | 3 + .../compare/ink_v0_multisigPlain.test.json | 2 + .../src/test/compare/ink_v1_psp22.test.json | 2 + .../src/test/compare/ink_v2_erc20.test.json | 1 + .../test/compare/ink_v3_traitErc20.test.json | 1 + .../test/compare/user_v0_withString.test.json | 1 + packages/types/src/metadata/Metadata.ts | 1 + .../PortableRegistry/PortableRegistry.ts | 286 +++++++++++------- 9 files changed, 194 insertions(+), 104 deletions(-) diff --git a/packages/api-contract/src/test/compare/ink_v0_erc20.test.json b/packages/api-contract/src/test/compare/ink_v0_erc20.test.json index 06c4176d57..027a871a09 100644 --- a/packages/api-contract/src/test/compare/ink_v0_erc20.test.json +++ b/packages/api-contract/src/test/compare/ink_v0_erc20.test.json @@ -160,6 +160,7 @@ { "info": "Enum", "lookupIndex": 10, + "lookupName": "InkStorageCollectionsStashEntry", "type": "{\"_enum\":{\"Vacant\":\"InkStorageCollectionsStashVacantEntry\",\"Occupied\":\"(AccountId,AccountId)\"}}", "docs": [], "namespace": "ink_storage::collections::stash::Entry", diff --git a/packages/api-contract/src/test/compare/ink_v0_erc721.test.json b/packages/api-contract/src/test/compare/ink_v0_erc721.test.json index f5aa54eb5e..fd77810b7f 100644 --- a/packages/api-contract/src/test/compare/ink_v0_erc721.test.json +++ b/packages/api-contract/src/test/compare/ink_v0_erc721.test.json @@ -180,6 +180,7 @@ { "info": "Struct", "lookupIndex": 10, + "lookupName": "InkStorageCollectionsHashmapValueEntryEntry", "type": "{\"value\":\"u32\",\"keyIndex\":\"u32\"}", "docs": [], "namespace": "ink_storage::collections::hashmap::ValueEntry", @@ -205,6 +206,7 @@ { "info": "Enum", "lookupIndex": 11, + "lookupName": "InkStorageCollectionsStashEntry", "type": "{\"_enum\":{\"Vacant\":\"InkStorageCollectionsStashVacantEntry\",\"Occupied\":\"(AccountId,AccountId)\"}}", "docs": [], "namespace": "ink_storage::collections::stash::Entry", @@ -275,6 +277,7 @@ { "info": "Struct", "lookupIndex": 13, + "lookupName": "InkStorageCollectionsHashmapValueEntryOption", "type": "{\"value\":\"bool\",\"keyIndex\":\"u32\"}", "docs": [], "namespace": "ink_storage::collections::hashmap::ValueEntry", diff --git a/packages/api-contract/src/test/compare/ink_v0_multisigPlain.test.json b/packages/api-contract/src/test/compare/ink_v0_multisigPlain.test.json index c9c5b3d727..66bbdc2857 100644 --- a/packages/api-contract/src/test/compare/ink_v0_multisigPlain.test.json +++ b/packages/api-contract/src/test/compare/ink_v0_multisigPlain.test.json @@ -226,6 +226,7 @@ { "info": "Struct", "lookupIndex": 12, + "lookupName": "InkStorageCollectionsHashmapValueEntry", "type": "{\"value\":\"u32\",\"keyIndex\":\"u32\"}", "docs": [], "namespace": "ink_storage::collections::hashmap::ValueEntry", @@ -373,6 +374,7 @@ { "info": "Enum", "lookupIndex": 19, + "lookupName": "InkStorageCollectionsStashEntry", "type": "{\"_enum\":{\"Vacant\":\"InkStorageCollectionsStashVacantEntry\",\"Occupied\":\"AccountId\"}}", "docs": [], "namespace": "ink_storage::collections::stash::Entry", diff --git a/packages/api-contract/src/test/compare/ink_v1_psp22.test.json b/packages/api-contract/src/test/compare/ink_v1_psp22.test.json index f7da9c9179..39fa53c766 100644 --- a/packages/api-contract/src/test/compare/ink_v1_psp22.test.json +++ b/packages/api-contract/src/test/compare/ink_v1_psp22.test.json @@ -310,6 +310,7 @@ { "info": "Enum", "lookupIndex": 15, + "lookupName": "InkStorageCollectionsStashEntry", "type": "{\"_enum\":{\"Vacant\":\"InkStorageCollectionsStashVacantEntry\",\"Occupied\":\"(u32,AccountId)\"}}", "docs": [], "namespace": "ink_storage::collections::stash::Entry", @@ -380,6 +381,7 @@ { "info": "Struct", "lookupIndex": 17, + "lookupName": "InkStorageCollectionsHashmapValueEntry", "type": "{\"value\":\"Null\",\"keyIndex\":\"u32\"}", "docs": [], "namespace": "ink_storage::collections::hashmap::ValueEntry", diff --git a/packages/api-contract/src/test/compare/ink_v2_erc20.test.json b/packages/api-contract/src/test/compare/ink_v2_erc20.test.json index d0010688e4..c034fecfba 100644 --- a/packages/api-contract/src/test/compare/ink_v2_erc20.test.json +++ b/packages/api-contract/src/test/compare/ink_v2_erc20.test.json @@ -83,6 +83,7 @@ { "info": "Struct", "lookupIndex": 6, + "lookupName": "InkStorageLazyMapping", "type": "{\"offsetKey\":\"InkPrimitivesKey\"}", "docs": [], "namespace": "ink_storage::lazy::mapping::Mapping", diff --git a/packages/api-contract/src/test/compare/ink_v3_traitErc20.test.json b/packages/api-contract/src/test/compare/ink_v3_traitErc20.test.json index 6c566e0ba4..365dc61c85 100644 --- a/packages/api-contract/src/test/compare/ink_v3_traitErc20.test.json +++ b/packages/api-contract/src/test/compare/ink_v3_traitErc20.test.json @@ -83,6 +83,7 @@ { "info": "Struct", "lookupIndex": 6, + "lookupName": "InkStorageLazyMapping", "type": "{\"offsetKey\":\"InkPrimitivesKey\"}", "docs": [], "namespace": "ink_storage::lazy::mapping::Mapping", diff --git a/packages/api-contract/src/test/compare/user_v0_withString.test.json b/packages/api-contract/src/test/compare/user_v0_withString.test.json index 9519db13b9..e27090b3b1 100644 --- a/packages/api-contract/src/test/compare/user_v0_withString.test.json +++ b/packages/api-contract/src/test/compare/user_v0_withString.test.json @@ -160,6 +160,7 @@ { "info": "Enum", "lookupIndex": 10, + "lookupName": "InkStorageCollectionsStashEntry", "type": "{\"_enum\":{\"Vacant\":\"InkStorageCollectionsStashVacantEntry\",\"Occupied\":\"(AccountId,AccountId)\"}}", "docs": [], "namespace": "ink_storage::collections::stash::Entry", diff --git a/packages/types/src/metadata/Metadata.ts b/packages/types/src/metadata/Metadata.ts index 94276f7c7b..0f01625a11 100644 --- a/packages/types/src/metadata/Metadata.ts +++ b/packages/types/src/metadata/Metadata.ts @@ -14,6 +14,7 @@ const VERSION_IDX = 4; // magic + lowest supported version const EMPTY_METADATA = new Uint8Array([0x6d, 0x65, 0x74, 0x61, 9]); +/** @internal */ function decodeU8a (registry: Registry, value: Uint8Array): MetadataVersioned | Uint8Array { const u8a = value.length === 0 ? EMPTY_METADATA diff --git a/packages/types/src/metadata/PortableRegistry/PortableRegistry.ts b/packages/types/src/metadata/PortableRegistry/PortableRegistry.ts index 6a6d63f486..8a6a9269ea 100644 --- a/packages/types/src/metadata/PortableRegistry/PortableRegistry.ts +++ b/packages/types/src/metadata/PortableRegistry/PortableRegistry.ts @@ -13,6 +13,24 @@ import { assert, assertUnreachable, isNumber, isString, logger, objectSpread, st const l = logger('PortableRegistry'); +interface ExtractBase { + lookupIndex: number; + name: string; +} + +interface Extract extends ExtractBase { + lookupIndex: number; + name: string; + params: SiTypeParameter[]; +} + +interface TypeInfo { + lookups: Record; + names: Record; + params: Record; + types: Record; +} + // Just a placeholder for a type.unrwapOr() const TYPE_UNWRAP = { toNumber: () => -1 }; @@ -63,18 +81,44 @@ const RESERVED = ['entries', 'hash', 'keys', 'new', 'size']; // Remove these from all paths at index 1 const PATH_RM_INDEX_1 = ['generic', 'misc', 'pallet', 'traits', 'types']; -function splitNamespace (values: string[]): string[][] { - return values.map((v) => v.split('::')); -} - -function createNamespace ({ path }: SiType): string { - return sanitizeDocs(path).join('::'); -} - +/** @internal */ function sanitizeDocs (docs: Text[]): string[] { - return docs.map((d) => d.toString()); + const result = new Array(docs.length); + + for (let i = 0; i < docs.length; i++) { + result[i] = docs[i].toString(); + } + + return result; } +/** @internal */ +function splitNamespace (values: string[]): string[][] { + const result = new Array(values.length); + + for (let i = 0; i < values.length; i++) { + result[i] = values[i].split('::'); + } + + return result; +} + +/** @internal */ +function createNamespace ({ path }: SiType): string { + if (!path.length) { + return ''; + } + + let result = path[0].toString(); + + for (let i = 1; i < path.length; i++) { + result += `::${path[i].toString()}`; + } + + return result; +} + +/** @internal */ function matchParts (first: string[], second: (string | Text)[]): boolean { return first.length === second.length && first.every((a, index) => { const b = second[index].toString(); @@ -110,6 +154,7 @@ function matchParts (first: string[], second: (string | Text)[]): boolean { } // check if the path matches the PATHS_ALIAS (with wildcards) +/** @internal */ function getAliasPath (path: SiPath): string | null { // TODO We need to handle ink! Balance in some way return path.length && PATHS_ALIAS.some((a) => matchParts(a, path)) @@ -117,26 +162,8 @@ function getAliasPath (path: SiPath): string | null { : null; } -function hasNoDupes (input: [number, string][]): boolean { - const count = input.length; - - for (let i = 0; i < count; i++) { - const [ai, an] = input[i]; - - for (let j = i + 1; j < count; j++) { - const [bi, bn] = input[j]; - - // if the indexes are not the same and the names match, we have a dupe - if (ai !== bi && an === bn) { - return false; - } - } - } - - return true; -} - -function extractNameFlat (portable: PortableType[], lookupId: number, params: SiTypeParameter[], path: AnyString[], isInternal = false): [number, string, SiTypeParameter[]] | null { +/** @internal */ +function extractNameFlat (portable: PortableType[], lookupIndex: number, params: SiTypeParameter[], path: AnyString[], isInternal = false): Extract | null { const last = path.length - 1; // if we have no path or determined as a wrapper, we just skip it @@ -145,10 +172,14 @@ function extractNameFlat (portable: PortableType[], lookupId: number, params: Si } const parts: string[] = []; - let typeName = ''; + let name = ''; for (let i = 0; i <= last; i++) { - const p = stringPascalCase(isInternal ? path[i].replace('pallet_', '') : path[i]); + const p = stringPascalCase( + isInternal + ? path[i].replace('pallet_', '') + : path[i] + ); const l = p.toLowerCase(); if ( @@ -162,9 +193,10 @@ function extractNameFlat (portable: PortableType[], lookupId: number, params: Si // sp_runtime::multiaddress::MultiAddress -> sp_runtime::MultiAddress i === last || l !== path[i + 1].toLowerCase() - )) { + ) + ) { parts.push(p); - typeName += p; + name += p; } } @@ -173,51 +205,106 @@ function extractNameFlat (portable: PortableType[], lookupId: number, params: Si const instanceType = portable[params[1].type.unwrap().toNumber()]; if (instanceType.type.path.length === 2) { - typeName = `${typeName}${instanceType.type.path[1].toString()}`; + name = `${name}${instanceType.type.path[1].toString()}`; } } - return [lookupId, typeName, params]; + return { lookupIndex, name, params }; } -function extractName (portable: PortableType[], { id, type: { params, path } }: PortableType): [number, string, SiTypeParameter[]] | null { - return extractNameFlat(portable, id.toNumber(), params, path); +/** @internal */ +function extractName (portable: PortableType[], lookupIndex: number, { type: { params, path } }: PortableType): Extract | null { + return extractNameFlat(portable, lookupIndex, params, path); } -function removeDuplicateNames (lookup: PortableRegistry, portable: PortableType[], names: [number, string | null, SiTypeParameter[]][]): [number, string, SiTypeParameter[]][] { +/** @internal */ +function nextDupeMatches (name: string, startAt: number, names: Extract[]): Extract[] { + const result = [names[startAt]]; + + for (let i = startAt + 1; i < names.length; i++) { + const v = names[i]; + + if (v.name === name) { + result.push(v); + } + } + + return result; +} + +/** @internal */ +function rewriteDupes (input: ExtractBase[], rewrite: Record): boolean { + const count = input.length; + + for (let i = 0; i < count; i++) { + const a = input[i]; + + for (let j = i + 1; j < count; j++) { + const b = input[j]; + + // if the indexes are not the same and the names match, we have a dupe + if (a.lookupIndex !== b.lookupIndex && a.name === b.name) { + return false; + } + } + } + + // add all the adjusted values to the rewite map + for (let i = 0; i < count; i++) { + const p = input[i]; + + rewrite[p.lookupIndex] = p.name; + } + + return true; +} + +/** @internal */ +function removeDupeNames (lookup: PortableRegistry, portable: PortableType[], names: Extract[]): Extract[] { const rewrite: Record = {}; return names - .map(([lookupIndex, name, params]): [number, string, SiTypeParameter[]] | null => { + .map((original, startAt): Extract | null => { + const { lookupIndex, name, params } = original; + if (!name) { + // the name is empty (this is not expected, but have a failsafe) return null; + } else if (rewrite[lookupIndex]) { + // we have already rewritten this one, we can skip it + return original; } - // those where the name is matching (since name is filtered, these all do have names) - const allSame = names.filter(([, oName]) => name === oName) as [number, string, SiTypeParameter[]][]; + // those where the name is matching starting from this index + const allSame = nextDupeMatches(name, startAt, names); - // are there among matching names - const anyDiff = allSame.some(([oIndex,, oParams]) => - lookupIndex !== oIndex && ( - params.length !== oParams.length || - params.some((p, index) => - !p.name.eq(oParams[index].name) || - p.type.unwrapOr(TYPE_UNWRAP).toNumber() !== oParams[index].type.unwrapOr(TYPE_UNWRAP).toNumber() - ) + // we only have one, so all ok + if (allSame.length === 1) { + return original; + } + + // are there param differences between matching names + const anyDiff = allSame.some((o) => + params.length !== o.params.length || + params.some((p, index) => + !p.name.eq(o.params[index].name) || + p.type.unwrapOr(TYPE_UNWRAP).toNumber() !== o.params[index].type.unwrapOr(TYPE_UNWRAP).toNumber() ) ); // everything matches, we can combine these - if (!anyDiff || !allSame[0][2].length) { - return [lookupIndex, name, params]; + if (!anyDiff) { + return original; } // TODO We probably want to attach all the indexes with differences, // not just the first // find the first parameter that yields differences - const paramIdx = allSame[0][2].findIndex(({ type }, index) => - allSame.every(([,, params]) => params[index].type.isSome) && - allSame.every(([,, params], aIndex) => + const paramIdx = params.findIndex(({ type }, index) => + allSame.every(({ params }) => + params[index].type.isSome + ) && + allSame.every(({ params }, aIndex) => aIndex === 0 || !params[index].type.eq(type) ) @@ -225,17 +312,17 @@ function removeDuplicateNames (lookup: PortableRegistry, portable: PortableType[ // No param found that is different if (paramIdx === -1) { - return [lookupIndex, name, params]; + return original; } // see if using the param type helps - const adjusted = new Array<[number, string]>(allSame.length); + const adjusted = new Array(allSame.length); // loop through all, specifically checking that index where the // first param yields differences for (let i = 0; i < allSame.length; i++) { - const [oIndex, oName, oParams] = allSame[i]; - const { def, path } = lookup.getSiType(oParams[paramIdx].type.unwrap()); + const { lookupIndex, name, params } = allSame[i]; + const { def, path } = lookup.getSiType(params[paramIdx].type.unwrap()); // if it is not a primitive and it doesn't have a path, we really cannot // do anything at this point @@ -243,68 +330,56 @@ function removeDuplicateNames (lookup: PortableRegistry, portable: PortableType[ return null; } - adjusted[i] = [ - oIndex, - def.isPrimitive - ? `${oName}${def.asPrimitive.toString()}` - : `${oName}${path[path.length - 1].toString()}` - ]; + adjusted[i] = { + lookupIndex, + name: def.isPrimitive + ? `${name}${def.asPrimitive.toString()}` + : `${name}${path[path.length - 1].toString()}` + }; } // check to see if the adjusted names have no issues - if (hasNoDupes(adjusted)) { - for (let i = 0; i < adjusted.length; i++) { - const [index, name] = adjusted[i]; - - rewrite[index] = name; - } - - return [lookupIndex, name, params]; + if (rewriteDupes(adjusted, rewrite)) { + return original; } // TODO This is duplicated from the section just above... // ... we certainly need a better solution here - - // last-ditch effort to use the full type path - ugly + // + // Last-ditch effort to use the full type path - ugly // loop through all, specifically checking that index where the // first param yields differences for (let i = 0; i < allSame.length; i++) { - const [oIndex, oName, oParams] = allSame[i]; - const { def, path } = lookup.getSiType(oParams[paramIdx].type.unwrap()); - - const flat = extractNameFlat(portable, oIndex, oParams, path, true); + const { lookupIndex, name, params } = allSame[i]; + const { def, path } = lookup.getSiType(params[paramIdx].type.unwrap()); + const flat = extractNameFlat(portable, lookupIndex, params, path, true); if (def.isPrimitive || !flat) { return null; } - adjusted[i] = [ - oIndex, - `${oName}${flat[1]}` - ]; + adjusted[i] = { + lookupIndex, + name: `${name}${flat.name}` + }; } // check to see if the adjusted names have no issues - if (hasNoDupes(adjusted)) { - for (let i = 0; i < adjusted.length; i++) { - const [index, name] = adjusted[i]; - - rewrite[index] = name; - } - - return [lookupIndex, name, params]; + if (rewriteDupes(adjusted, rewrite)) { + return original; } return null; }) - .filter((n): n is [number, string, SiTypeParameter[]] => !!n) - .map(([lookupIndex, name, params]) => [ + .filter((n): n is Extract => !!n) + .map(({ lookupIndex, name, params }): Extract => ({ lookupIndex, - rewrite[lookupIndex] || name, + name: rewrite[lookupIndex] || name, params - ]); + })); } +/** @internal */ function registerTypes (lookup: PortableRegistry, lookups: Record, names: Record, params: Record): void { // Register the types we extracted lookup.registry.register(lookups); @@ -345,6 +420,7 @@ function registerTypes (lookup: PortableRegistry, lookups: Record, isContract?: boolean): Record { const hasParams = Object.keys(params).some((k) => !k.startsWith('Pallet')); const alias: Record = {}; @@ -370,37 +446,39 @@ function extractAliases (params: Record, isContract?: return alias; } -function extractTypeInfo (lookup: PortableRegistry, portable: PortableType[]): [Record, Record, Record, Record] { - const nameInfo: [number, string, SiTypeParameter[]][] = []; +/** @internal */ +function extractTypeInfo (lookup: PortableRegistry, portable: PortableType[]): TypeInfo { + const nameInfo: Extract[] = []; const types: Record = {}; const porCount = portable.length; for (let i = 0; i < porCount; i++) { const type = portable[i]; - const extracted = extractName(portable, portable[i]); + const lookupIndex = type.id.toNumber(); + const extracted = extractName(portable, lookupIndex, portable[i]); if (extracted) { nameInfo.push(extracted); } - types[type.id.toNumber()] = type; + types[lookupIndex] = type; } - const dedup = removeDuplicateNames(lookup, portable, nameInfo); const lookups: Record = {}; const names: Record = {}; const params: Record = {}; - const dupCount = dedup.length; + const dedup = removeDupeNames(lookup, portable, nameInfo); + const dedupCount = dedup.length; - for (let i = 0; i < dupCount; i++) { - const [lookupIndex, name, p] = dedup[i]; + for (let i = 0; i < dedupCount; i++) { + const { lookupIndex, name, params: p } = dedup[i]; names[lookupIndex] = name; lookups[name] = lookup.registry.createLookupType(lookupIndex); params[name] = p; } - return [types, lookups, names, params]; + return { lookups, names, params, types }; } export class PortableRegistry extends Struct implements ILookup { @@ -418,7 +496,7 @@ export class PortableRegistry extends Struct implements ILookup { types: 'Vec' }, value); - const [types, lookups, names, params] = extractTypeInfo(this, this.types); + const { lookups, names, params, types } = extractTypeInfo(this, this.types); this.#alias = extractAliases(params, isContract); this.#lookups = lookups;