Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Id to have arbitrary length instead of 16 byte limit, also add in POJO and Map tests #10

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Jan 19, 2022

Description

As discussed in MatrixAI/Polykey#254, we want Id to be usable for NodeId as well. In such a case, we don't actually want the 16 byte limit that we have in our utils.ts atm. This is only relevant to UUID. So the limit can be constraint to UUID conversions, and all other functions can operate more freely.

Issues Fixed

Tasks

  1. Add 16 byte check to toUUID, and throw RangeError if the Id is not 16 bytes
  2. Remove 16 byte limit from other conversion utilities like string, buffer and multibase
  3. Create tests for Id.test.ts to test Id creation byitself
  4. Add POJO and Map tests to show that Id can be used as Map keys, and POJO keys without any problems

Final checklist

  • Domain specific tests
  • Full tests
  • Updated inline-comment documentation
  • Lint fixed
  • Squash and rebased
  • Sanity check the final build

@CMCDragonkai
Copy link
Member Author

This is ready, expect to use Id as NodeId in this way:

function publicKeyToFingerprintBytes(
  publicKey: PublicKey,
): PublicKeyFingerprintBytes {
  const fString = pki.getPublicKeyFingerprint(publicKey, {
    type: 'SubjectPublicKeyInfo',
    md: md.sha256.create(),
    encoding: 'binary',
  });
  return fString;
}

const b = Buffer.from(publicKeyToFingerprintBytes(publicKey), 'binary');

const nodeId = Id.create(b);

Note that the publicKeyToFingerPrintBytes currently returns a string due to how node-forge works. So we have to convert it to buffer which is an Uint8Array, and therefore can be used in nodeId.

@CMCDragonkai
Copy link
Member Author

Actually there's a better function:

  const fString = publicKeyToFingerprintBytes(publicKey);
  const fTypedArray = forgeUtil.binary.raw.decode(fString);

Then Id.create(fTypedArray) should work too.

@CMCDragonkai CMCDragonkai merged commit 5524fde into master Jan 19, 2022
@CMCDragonkai CMCDragonkai deleted the 16bytes branch January 19, 2022 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant