Skip to content

Fix node claims throughout tests/ #306

Description

@joshuakarp

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.

Activity

  1. CMCDragonkai commented on Jan 9, 2022

    @CMCDragonkai
    Member

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

  2. emmacasolin commented on Jan 10, 2022

    @emmacasolin
    Contributor

    Yep, this can be addressed in #308

  3. emmacasolin commented on Jan 20, 2022

    @emmacasolin
    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.

  4. CMCDragonkai commented on Jan 20, 2022

    @CMCDragonkai
    Member

    This was merged into master as c40b99d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions