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

feat: add cns-js library #80

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add cns-js library #80

wants to merge 3 commits into from

Conversation

nathanosdev
Copy link
Member

@nathanosdev nathanosdev commented Feb 12, 2025

This has been tested with a TS script as follows:

import { HttpAgent } from '@dfinity/agent';
import { CnsClient } from './cns-client';

async function main(): Promise<void> {
  const agent = HttpAgent.createSync({ host: 'http://localhost:8000' });
  await agent.fetchRootKey();

  const cnsClient = new CnsClient({
    agent,
    cnsRoot: 'bkyz2-fmaaa-aaaaa-qaaaq-cai',
  });

  const icpNamingCanister =
    await cnsClient.lookupNamingCanister('nathanos.icp');
  console.log('.icp naming canister:', icpNamingCanister.toText());

  const nathanosIcpCanister = await cnsClient.lookupCanisterId('nathanos.icp');
  console.log('nathanos.icp canister id:', nathanosIcpCanister.toText());
}

main().catch(console.error);

The CNS Root and TLD Operator were setup and configured with a TLD and domain manually.

dfx.json Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main file where the client logic lives, every other file is just boilerplate and less important.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add a test for this functionality to CI? (potentially in a follow-up PR)

lib/cns-js/src/cns-root/cns-root-client.ts Show resolved Hide resolved
}
}

function normalizeDomain(domain: string): string {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to make these normalization functions pretty resilient, so you can lookup NC record for any of the following:

  • .foo.icp.
  • foo.icp.
  • foo.icp
  • .icp.
  • .icp
  • icp.

Sub domains like foo.bar.icp should also work with the same variations of . pre/post fixes.

And similarly you can lookup the CID record for any of the following:

  • .sub.icp.
  • sub.icp.
  • sub.icp

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
nits:

  • maybe this function should be a part of utils.ts?
  • would it be possible to add some unit tests for this function? (a follow-up PR would be also fine)

public async lookupCanisterId(domain: string): Promise<Principal> {
const normalizedDomain = normalizeDomain(domain);
const existingCanisterId = this.#canisterIds.get(normalizedDomain);
if (isNotNil(existingCanisterId)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a very simple cache that doesn't persist between page loads.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just some utils

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the rest is boilerplate nonsense

@nathanosdev nathanosdev marked this pull request as ready for review February 12, 2025 16:21
@nathanosdev nathanosdev requested a review from a team as a code owner February 12, 2025 16:21
Copy link
Contributor

@przydatek przydatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add a test for this functionality to CI? (potentially in a follow-up PR)

}
}

function normalizeDomain(domain: string): string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
nits:

  • maybe this function should be a part of utils.ts?
  • would it be possible to add some unit tests for this function? (a follow-up PR would be also fine)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants