Cleanup storage create function (#1336)

* Cleanup storage create function

* Cleanup param ordering

* Cleaner expansion

* Small reworks

* Commenting

* String update (tests)

packages/api-metadata/src/storage/fromMetadata/createFunction.spec.ts

@@ -95,7 +95,7 @@ describe('createFunction', (): void => {
     it('needs two arguments', (): void => {
       expect(
         (): Uint8Array => storageFn(['5DXUeE5N5LtkW97F2PzqYPyqNkxqSWESdGSPTX6AvkUAhwKP'])
-      ).toThrow(/metaName expects two arguments/);
+      ).toThrow(/requires two arguments/);
     });
   });
 

packages/api-metadata/src/storage/fromMetadata/createFunction.ts

@@ -27,75 +27,75 @@ export interface CreateItemFn {
 
 type CreateArgType = boolean | string | number | null | BN | Uint8Array | Codec;
 
-/**
+const NULL_HASHER = (value: Uint8Array): Uint8Array => value;
- * From the schema of a function in the module's storage, generate the function
+
- * that will return the correct storage key.
+// with the prefix, method & options, create both the string & raw keys
- *
+function createKeys ({ method, prefix }: CreateItemFn, options: CreateItemOptions): [string, Uint8Array] {
- * @param schema - The function's definition schema to create the function from.
- * The schema is taken from state_getMetadata.
- * @param options - Additional options when creating the function. These options
- * are not known at runtime (from state_getMetadata), they need to be supplied
- * by us manually at compile time.
- */
-export default function createFunction ({ meta, method, prefix, section }: CreateItemFn, options: CreateItemOptions = {}): StorageEntry {
   const stringKey = options.key
     ? options.key
     : `${prefix} ${method}`;
-  const rawKey = stringToU8a(stringKey);
 
-  // Get the hashing function
+  return [
-  let hasher: HasherFunction;
+    stringKey,
-  let key2Hasher: HasherFunction;
+    stringToU8a(stringKey)
+  ];
+}
 
-  if (meta.type.isDoubleMap) {
+// get the hashers, the base (and  in the case of DoubleMap), the second key
-    hasher = getHasher(meta.type.asDoubleMap.hasher);
+function getHashers ({ meta: { type } }: CreateItemFn): [HasherFunction, HasherFunction?] {
-    key2Hasher = getHasher(meta.type.asDoubleMap.key2Hasher);
+  if (type.isDoubleMap) {
-  } else if (meta.type.isMap) {
+    return [
-    hasher = getHasher(meta.type.asMap.hasher);
+      getHasher(type.asDoubleMap.hasher),
-  } else {
+      getHasher(type.asDoubleMap.key2Hasher)
-    hasher = getHasher();
+    ];
+  } else if (type.isMap) {
+    return [getHasher(type.asMap.hasher)];
   }
 
-  // Can only have zero or one argument:
+  // the default
-  //   - storage.balances.freeBalance(address)
+  return [getHasher()];
-  //   - storage.timestamp.blockPeriod()
+}
-  // For doublemap queries the params is passed in as an tuple, [key1, key2]
-  const _storageFn = (arg?: CreateArgType | [CreateArgType?, CreateArgType?]): Uint8Array => {
-    let key = rawKey;
 
-    if (meta.type.isDoubleMap) {
+// create a key for a DoubleMap type
-      assert(Array.isArray(arg) && !isUndefined(arg[0]) && !isNull(arg[0]) && !isUndefined(arg[1]) && !isNull(arg[1]), `${meta.name} expects two arguments`);
+function createKeyDoubleMap ({ meta: { name, type } }: CreateItemFn, rawKey: Uint8Array, args: [CreateArgType, CreateArgType], [hasher, key2Hasher]: [HasherFunction, HasherFunction?]): Uint8Array {
+  // since we are passing an almost-unknown through, trust, but verify
+  assert(
+    Array.isArray(args) && !isUndefined(args[0]) && !isNull(args[0]) && !isUndefined(args[1]) && !isNull(args[1]),
+    `${name} is a DoubleMap and requires two arguments`
+  );
 
-      // we have checked that it is an array in the assert, so all ok
+  const [key1, key2] = args;
-      const [key1, key2] = arg as [CreateArgType, CreateArgType];
+  const type1 = type.asDoubleMap.key1.toString();
-      const type1 = meta.type.asDoubleMap.key1.toString();
+  const type2 = type.asDoubleMap.key2.toString();
-      const type2 = meta.type.asDoubleMap.key2.toString();
+  const param1Encoded = u8aConcat(rawKey, createTypeUnsafe(type1, [key1]).toU8a(true));
-      const param1Encoded = u8aConcat(key, createTypeUnsafe(type1, [key1]).toU8a(true));
+  const param1Hashed = hasher(param1Encoded);
-      const param1Hashed = hasher(param1Encoded);
-      const param2Hashed = key2Hasher(createTypeUnsafe(type2, [key2]).toU8a(true));
 
-      return Compact.addLengthPrefix(u8aConcat(param1Hashed, param2Hashed));
+  // @ts-ignore If this fails it means the getHashers function failed - and we have much bigger issues
-    }
+  const param2Hashed = key2Hasher(createTypeUnsafe(type2, [key2]).toU8a(true));
 
-    if (meta.type.isMap) {
+  // as per createKey, always add the length prefix (underlying it is Bytes)
-      assert(!isUndefined(arg) && !isNull(arg), `${meta.name} expects one argument`);
+  return Compact.addLengthPrefix(u8aConcat(param1Hashed, param2Hashed));
+}
 
-      const type = meta.type.asMap.key.toString();
+// create a key for either a map or a plain value
-      const param = createTypeUnsafe(type, [arg]).toU8a();
+function createKey ({ meta: { name, type } }: CreateItemFn, rawKey: Uint8Array, arg: CreateArgType, hasher: (value: Uint8Array) => Uint8Array): Uint8Array {
+  let key = rawKey;
 
-      key = u8aConcat(key, param);
+  if (type.isMap) {
-    }
+    assert(!isUndefined(arg) && !isNull(arg), `${name} is a Map and requires one argument`);
 
-    // StorageKey is a Bytes, so is length-prefixed
+    const mapType = type.asMap.key.toString();
-    return Compact.addLengthPrefix(
+    const param = createTypeUnsafe(mapType, [arg]).toU8a();
-      options.skipHashing
-        ? key
-        : hasher(key)
-    );
-  };
 
-  const storageFn = _storageFn as StorageEntry;
+    key = u8aConcat(key, param);
+  }
 
+  // StorageKey is a Bytes, so is length-prefixed
+  return Compact.addLengthPrefix(hasher(key));
+}
+
+// attach the metadata to expsnd to a StorageFunction
+function expandWithMeta ({ meta, method, prefix, section }: CreateItemFn, storageFn: StorageEntry): StorageEntry {
   storageFn.meta = meta;
   storageFn.method = stringLowerFirst(method);
   storageFn.prefix = prefix;
@@ -103,28 +103,68 @@ export default function createFunction ({ meta, method, prefix, section }: Creat
 
   // explicitly add the actual method in the toJSON, this gets used to determine caching and without it
   // instances (e.g. collective) will not work since it is only matched on param meta
-  storageFn.toJSON = (): any => ({ ...(meta.toJSON() as any), storage: { method, prefix, section } });
+  storageFn.toJSON = (): any => ({
+    ...(meta.toJSON() as any),
+    storage: { method, prefix, section }
+  });
 
-  if (meta.type.isMap && meta.type.asMap.linked.isTrue) {
+  return storageFn;
-    const headHash = new U8a(hasher(`head of ${stringKey}`));
+}
-    const headFn: any = (): U8a => headHash;
 
-    // metadata with a fallback value using the type of the key, the normal
+// attch the head key hashing for linked maps
-    // meta fallback only applies to actual entry values, create one for head
+function extendLinkedMap ({ meta: { documentation, name, type } }: CreateItemFn, storageFn: StorageEntry, stringKey: string, hasher: HasherFunction): StorageEntry {
-    headFn.meta = new StorageEntryMetadata({
+  const headHash = new U8a(hasher(`head of ${stringKey}`));
-      name: meta.name,
+  const headFn: any = (): U8a =>
-      modifier: createType('StorageEntryModifierV7', 1), // required
+    headHash;
-      type: new StorageEntryType(createType('PlainTypeV7', meta.type.asMap.key), 0),
-      fallback: new Bytes(createTypeUnsafe(meta.type.asMap.key.toString()).toHex()),
-      documentation: meta.documentation
-    });
 
-    // here we pass the section/method through as well - these are not on
+  // metadata with a fallback value using the type of the key, the normal
-    // the function itself, so specify these explicitly to the constructor
+  // meta fallback only applies to actual entry values, create one for head
-    storageFn.headKey = new StorageKey(headFn, {
+  headFn.meta = new StorageEntryMetadata({
-      method: storageFn.method,
+    name,
-      section: `head of ${storageFn.section}`
+    modifier: createType('StorageEntryModifierV7', 1), // required
-    });
+    type: new StorageEntryType(createType('PlainTypeV7', type.asMap.key), 0),
+    fallback: new Bytes(createTypeUnsafe(type.asMap.key.toString()).toHex()),
+    documentation
+  });
+
+  // here we pass the section/method through as well - these are not on
+  // the function itself, so specify these explicitly to the constructor
+  storageFn.headKey = new StorageKey(headFn, {
+    method: storageFn.method,
+    section: `head of ${storageFn.section}`
+  });
+
+  return storageFn;
+}
+
+/**
+ * From the schema of a function in the module's storage, generate the function
+ * that will return the correct storage key.
+ *
+ * @param item - The function's definition schema to create the function from.
+ * The schema is taken from state_getMetadata.
+ * @param options - Additional options when creating the function. These options
+ * are not known at runtime (from state_getMetadata), they need to be supplied
+ * by us manually at compile time.
+ */
+export default function createFunction (item: CreateItemFn, options: CreateItemOptions = {}): StorageEntry {
+  const { meta: { type } } = item;
+  const [stringKey, rawKey] = createKeys(item, options);
+  const [hasher, key2Hasher] = getHashers(item);
+
+  // Can only have zero or one argument:
+  //   - storage.balances.freeBalance(address)
+  //   - storage.timestamp.blockPeriod()
+  // For doublemap queries the params is passed in as an tuple, [key1, key2]
+  const _storageFn = (arg?: CreateArgType | [CreateArgType?, CreateArgType?]): Uint8Array =>
+    type.isDoubleMap
+      ? createKeyDoubleMap(item, rawKey, arg as [CreateArgType, CreateArgType], [hasher, key2Hasher])
+      : createKey(item, rawKey, arg as CreateArgType, options.skipHashing ? NULL_HASHER : hasher);
+
+  const storageFn = expandWithMeta(item, _storageFn as StorageEntry);
+
+  if (type.isMap && type.asMap.linked.isTrue) {
+    extendLinkedMap(item, storageFn, stringKey, hasher);
   }
 
   return storageFn;

packages/api-metadata/src/storage/fromMetadata/fromMetadata.spec.ts

@@ -19,7 +19,7 @@ const storage = fromMetadata(metadata);
 
 describe('fromMetadata', (): void => {
   it('should throw if the storage function expects an argument', (): void => {
-    expect((): any => storage.balances.freeBalance()).toThrowError(/expects one argument/);
+    expect((): any => storage.balances.freeBalance()).toThrowError(/requires one argument/);
   });
 
   it('should return a value if the storage function does not expect an argument', (): void => {

packages/api-metadata/src/storage/fromMetadata/getHasher.ts

@@ -6,11 +6,12 @@ import { StorageHasher } from '@polkadot/types/primitive';
 import { u8aConcat, u8aToU8a } from '@polkadot/util';
 import { blake2AsU8a, xxhashAsU8a } from '@polkadot/util-crypto';
 
-type HasherInput = string | Buffer | Uint8Array;
+export type HasherInput = string | Buffer | Uint8Array;
-type HasherCheck = 'isBlake2128' | 'isBlake2256' | 'isTwox128' | 'isTwox256' | 'isTwox64Concat';
 
 export type HasherFunction = (data: HasherInput) => Uint8Array;
 
+type HasherCheck = 'isBlake2128' | 'isBlake2256' | 'isTwox128' | 'isTwox256' | 'isTwox64Concat';
+
 const DEFAULT = (data: HasherInput): Uint8Array => xxhashAsU8a(data, 128);
 
 const map: Record<HasherCheck, HasherFunction> = {