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

Fix node claims throughout tests/ #306

Closed
joshuakarp opened this issue Dec 3, 2021 · 4 comments · Fixed by #311
Closed

Fix node claims throughout tests/ #306

joshuakarp opened this issue Dec 3, 2021 · 4 comments · Fixed by #311
Assignees
Labels
bug Something isn't working development Standard development r&d:polykey:core activity 3 Peer to Peer Federated Hierarchy

Comments

@joshuakarp
Copy link
Contributor

joshuakarp commented Dec 3, 2021

Specification

There are several occurrences of node claims that are being incorrectly set up throughout the testing suites.

Consider the situation where we have two nodes, B and C, with a cryptolink between them (i.e. part of the same gestalt). The cryptolink should be represented by a single claim on each node's sigchain (that's been signed by both B and C). That is, B will have a single claim signed by both itself and C, and C will have a single claim signed by both itself and B.

However, there are some tests that aren't setting this up correctly. For example, in tests/bin/identities.test.ts:

      // Adding sigchain details.
      const claimBtoC: ClaimLinkNode = {
        type: 'node',
        node1: nodeB.nodeManager.getNodeId(),
        node2: nodeC.nodeManager.getNodeId(),
      };
      const claimCtoB: ClaimLinkNode = {
        type: 'node',
        node1: nodeC.nodeManager.getNodeId(),
        node2: nodeB.nodeManager.getNodeId(),
      };
      await nodeB.sigchain.addClaim(claimBtoC);
      await nodeB.sigchain.addClaim(claimCtoB);
      await nodeC.sigchain.addClaim(claimCtoB);
      await nodeC.sigchain.addClaim(claimBtoC);

Here we're incorrectly adding 2 claims, that are singly signed by the owner of the respective sigchain the claim is added to. From the git blame, it seems like this was added prior to the refactoring of node claims though, so just something that's been missed.

Additional context

Tasks

  1. Create a tests utility function to create a node claim (a doubly signed claim, signed by both nodes' private keys).
  2. Find and fix all instances where the nodes claims are being incorrectly set up.
@joshuakarp joshuakarp added bug Something isn't working development Standard development labels Dec 3, 2021
@CMCDragonkai
Copy link
Member

@emmacasolin can you address this in #308 too? Or would it not be relevant to address there?

@emmacasolin
Copy link
Contributor

Yep, this can be addressed in #308

@emmacasolin
Copy link
Contributor

This has been fixed in 71acbff. Since this was really only affecting discovery tests I didn't think a utility function was needed, rather one node simply needs to call nodeManager.claimNode([other node's id]), so this is what the tests now do.

@CMCDragonkai
Copy link
Member

This was merged into master as c40b99d.

@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 3 Peer to Peer Federated Hierarchy label Jul 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working development Standard development r&d:polykey:core activity 3 Peer to Peer Federated Hierarchy
Development

Successfully merging a pull request may close this issue.

3 participants