add back code

[sarahmjohnson]

No description provided.

[GeraldHost]

Here we need to check that at least 3 members have vouched for this user. This is just checking that there are 3 members in the DAO which will always be true after the constructor is run. You need something like this:

    function register(address[] members) {
        require(members.length == 3, "!3 members");

        for (uint i = 0; i < members.length; i++) {
            // check account is vouched for by this member
            require(userManager.getVouchingAmount(members[i], msg.sender) > 0, "!vouching");
            // check address is member
            require(members[member], "!member");
        }

        // Mint the NFT (membership NFT)
        _mint(msg.sender, id);
        // add to members
        members[msg.sender] = true;
        // Guild vouches for this new member
        userManager.updateTrust(msg.sender, vouchAmount);
        // transfer membership fee
        dai.transferFrom(msg.sender, address(this), membershipFee);
    }

[GeraldHost]

you need to check the DAI balance as DAI is what is being staked into union:

uint bal = dai.balanceOf(address(this));
dai.approve(address(userManager), bal);
userManager.stake(bal);

[GeraldHost]

Here you want to check that the member is vouching for the contract. The signature is getVouchingAmount(address staker, address borrower) the staker is the member and the borrower is the contract (address(this))

[GeraldHost]

Syntax wise this is more common return result You can look at using this formatter to provide consistent formatting to your code: https://github.com/prettier-solidity/prettier-plugin-solidity

[GeraldHost]

Need to add this back in, in order to prevent transfers.

[GeraldHost]

You should also check these address are now members. You could write this test like this.

const memberAddresses = [MEMBER1.address, MEMBER2.address, MEMBER3.address];
const tx = await creditGuild.initialize(...memberAddresses);
const resp = await tx.wait();
const initialMembers = resp.events[resp.events.length - 1].args.initialMembers;
for (let i = 0; i < memberAddresses.length; i++) {
      expect(initialMembers[i]).eq(memberAddresses[i]);
      expect(await creditGuild.checkIsMember(memberAddresses[i])).eq(true);
}

[GeraldHost]

For both these checks you can just do

expect(await creditGuild.checkIsMember(potentialMember).eq(true);

[GeraldHost]

Rather than looking at the output event.

[GeraldHost]

There are a few other places below where you do this that can be updated to.

[GeraldHost]

Here you will also need to check the UserManager... in fact there is not much point testing this in a unit test.

[GeraldHost]

I would try and seperate out unhappy paths into a seperate it function ie:

describe("@method burnMembership()", () => {
  it("happy path: removes member from the DAO", async () => {
     // ....
  });

  it("happy path: burns the members NFT", async () => {
     // ....
  });

  it("unhappy path: cannot burnMembership for non member", async () => {
     // ....
  });
});

[GeraldHost]

In this you also want to include a test to check that the NFT has been burnt something like:

const bal = await creditGuild.balanceOf(member);
expect(bal).lte(0);