Write contract tests

[nintynick]

I've been working on writing acceptance criteria / descriptive functional tests. Here's how I've been thinking about it:

Four sources of things to test:

• Errors
• Hat struct
• Functions
• Events

From the Hat struct, there are several variables we will have to try different combinations of:

• maxSupply: 1, 2+, and some other large number (say 300)
• admin: EOA, DAO, and maybe as a contract?
• oracle: EOA, DAO, contract (e.g. time bound, holding of other tokens, moloch DAO params, a number of tx related to role such as 3 multisig tx signed, something off-chain)
• conditions: same as oracle

Roughly speaking, combinations of these variables will give us a list of Hats to test. Then we can run the same set of tests over each Hat, potentially with some additions.

My hope is that these will evolve into a test suite that includes unit tests as well as a manual testing flow.

List of Hats:

• [ ] Hat A: maxSupply = 1, admin = EOA, oracle = EOA, conditions = EOA
• [ ] Hat B: maxSupply = 2, admin = EOA, oracle = EOA, conditions = EOA
• [ ] Hat C: maxSupply = 300, admin = EOA, oracle = EOA, conditions = EOA
• [ ] Hat D: maxSupply = 1, admin = DAO, oracle = DAO, conditions = DAO
• [ ] Hat E: maxSupply = 2, admin = DAO, oracle = DAO, conditions = DAO
• [ ] Hat F: maxSupply = 1, admin = EOA, oracle = contract, conditions = contract
• [ ] Hat G: maxSupply = 2, admin = EOA, oracle = contract, conditions = contract

List of tests:

1. Create

• [ ] 1-1. instantiate Hats with createTopHatAndHat - isTopHat(TopHat.hatId) == true && isWearerOfHat(admin, TopHat.hatId) == true && isActive(Hat.hatId) == true && isAdminOfHat(admin, Hat.hatId) == true

2. Mint

• [ ] 2-1. attempt to mint from wearer / other wallet - error NotAdmin()
• [ ] 2-2. mint hat from admin to wearer wallet - isWearerOfHat(wearer, Hat.hatId) == true && check HatCreated event
• [ ] 2-3. attempt to mint hat to a second wallet - error AllHatsWorn()
• [ ] 2-4. check Oracle - isInGoodStanding(wearer, Hat.hatId) == true

3. View -> Public View Functions

• [ ] 3-1. view Hat data - viewHat displays params (especially details) as expected
• [ ] what other functions need to be added here?

4. Transfer

• [ ] 4-1. transfer from wearer / other wallet - error OnlyAdminsCanTransfer()
• [ ] 4-2. transfer from admin - isWearerOfHat(wearer2, Hat.hatId) == true && isWearerOfHat(wearer, Hat.hatId) == false

5. Oracle / Revoke

• [ ] 5-1. check standing with checkHatWearerStanding from wearer2 / other wallet / admin - isWearerOfHat(wearer2, Hat.hatId) == true && check Ruling event
• [ ] 5-2. [change param 1st time]
• [ ] 5-3. check standing with checkHatWearerStanding from any wallet - isWearerOfHat(wearer2, Hat.hatId) == false && check HatRevoked event && check Ruling event
• [ ] 5-4. [change param 2nd time]
• [ ] 5-4. mint hat again from admin to wearer2 wallet - isWearerOfHat(wearer2, Hat.hatId) == true
• [ ] 5-5. attempt to rule on standing with ruleOnHatWearerStanding from wearer / other wallet / admin - error NotHatOracle()
• [ ] 5-6. rule on standing with ruleOnHatWearerStanding from conditions - isWearerOfHat(wearer2, Hat.hatId) == true && check Ruling event
• [ ] 5-7. [change param 3rd time]
• [ ] 5-8. rule on standing with ruleOnHatWearerStanding from conditions - isWearerOfHat(wearer2, Hat.hatId) == false && check HatRevoked event && check Ruling event
• [ ] 5-9. [change param 4th time]
• [ ] 5-10. (extra coverage) attempt to mint from wearer / other wallet - error NotAdmin()
• [ ] 5-11. mint hat again from admin to wearer2 wallet - isWearerOfHat(wearer2, Hat.hatId) == true

6. Renounce

• [ ] 6-1. attempt to renounce from admin / other wallet - error NotHatWearer()
• [ ] 6-2. renounce hat from wearer2 - isWearerOfHat(wearer2, Hat.hatId) == false && check HatRenounced event

7. Deactivate

• [ ] 7-1. attempt to changeHatStatus hat from wearer / other wallet / admin - error NotHatConditions()
• [ ] 7-2. changeHatStatus in conditions - isActive(Hat.hatId) == false && check HatStatusChanged event
• [ ] 7-3. call various functions in deactivated state again as wearer / other wallet / admin - error HatNotActive()

8. Reactivate

• [ ] 8-1. attempt to changeHatStatus hat from wearer / other wallet / admin - error NotHatConditions()
• [ ] 8-2. changeHatStatus in conditions - isActive(Hat.hatId) == true && check HatStatusChanged event

TODO:

• [x] write example conditions contracts for Hats F and G
• [ ] write example oracle contracts for Hats F and G
• [ ] add tests for functions createHatsTree (and batchMintHats, batchTrasnferHats, and error BatchArrayLengthMismatch)
• [ ] fill in wallet params (e.g. admin / wearer / wearer2 / other wallet)
• [ ] remove redundancies from List of Hats X List of tests
• [ ] add Hat-specific tests (e.g. for Hat B as follows)
• attempt to mint hat with maxSupply 2 to the the same wallet twice - error AlreadyWearingHat
• mint hat with maxSupply 2 to two different wallets - can see token in each wallet on block explorer

ninty questions

• [x] - where do events show up?
• [x] - what's the connector between DAO and manually voting for yes / no on Oracle or Conditions?
• [x] - is it possible to use checkHatWearerStanding in place of isInGoodStanding? (redundant?) (see 2-4, 5.)
• [x] - basically ruleOnHatWearerStanding is proactive from the Oracle, and checkHatWearerStanding is reactive by anyone?
• [ ] - [TODO: note in docs] what happens to minted Hat(s) when a Hat's status is changed to inactive?
• [x] - [TODO: yes] should we test changeHatStatus while a hat is minted, as well as when no hats are minted?
• [x] - [TODO: yes] do we need to test errors SafeTransfersNotNecessary() or NoApprovalsNeeded()? yes, need to test that it blocks the function as intended
• [x] - [see #20] do we need test cases for if ORACLE doesn't have a checkWearerStanding() function?
• [x] - [see #20] do we need test cases for if CONDITIONS doesn't have a checkConditions() function?
• [ ] - [TODO: note in docs "default is that you are in good standing for every hat"] what happens when you call isInGoodStanding(non-wearer, hatId)? does each Oracle have to implement checkWearerStanding to handle this in a specific way?

Notes

• deactivateHat is now "changeHatStatus" but comments need to be updated e.g. line 43, 120
• wearar->wearer line 322
• invert 618: standing = !revocations[_hatId][_wearer];

[spengrah]

write example oracle / conditions contracts for Hats F and G @nintynick see #27 for example conditions contracts

[nintynick]

documenting progress tonight, as of https://github.com/Hats-Protocol/hats-protocol/commit/a4c408073f6a0c8815df4e0ccb06a58d84fd9428:

1. setUp() function abstracted into helper contract TestVariablesAndSetup for all the tests I wrote

2. 6. Renounce tests done

3. 5. Oracle / Revoke tests and 7. Deactivate & 8. Reactivate (i.e. Conditions) tests 50% done a. setHatStatus and setHatWearerStatus working great (except: testFailFunctionCallsOnDeactivatedHat(), see comment on line 453-454 (note we are not using HatNotActive() error in Hats.sol)) b. getHatStatus and getHatWearerStatus may have to hook into the sample Oracle and Conditions contracts for testing c. We might want to look at how getHatStatus and getHatWearerStatus fail if the function is not in the respective Oracle / Conditions contract (see lines 390, 494) (I think we talked about this but don't recall the verdict)

remaining:

• TODOs marked in getHatStatus and getHatWearerStatus
• 2. Mint (though we already have some coverage in CreateHatsTest)
• 3. View
• 4. Transfer

getting there - so close!

[nintynick]

• make sure that all public functions have test coverage

[nintynick]

• everything in the previous two comments now complete except for 2. Mint and things with TODOs in the code!

[nintynick]

closing this, as we now have full coverage of the list of tests mentioned above, merged into mvp branch in #34. all tests are passing and functions are working on the testnet deployment.

for future testing, there's an opportunity to do more varied and robust TestSetups (see list of hats in comment above), such that we test more edge cases.