add Mappings tests

- add tests for Mappings.Mapping
- remove SetMap.test.js (no longer used)
- rename getManyCount to getValueCount

contracts/Marketplace.sol

@@ -57,7 +57,7 @@ contract Marketplace is Collateral, Proofs {
     returns (SlotId[] memory)
   {
     uint256 counter = 0;
-    uint256 totalSlots = activeRequestSlots.getManyCount(); // set this bigger than our possible filtered list size
+    uint256 totalSlots = activeRequestSlots.getValueCount(); // set this bigger than our possible filtered list size
     if (totalSlots == 0) {
       return new SlotId[](0);
     }
@@ -220,7 +220,7 @@ contract Marketplace is Collateral, Proofs {
     Slot storage slot = _slot(slotId);
     require(!slot.hostPaid, "Already paid");
     activeRequestSlots.deleteValue(_toValueId(slotId));
-    if (activeRequestSlots.getManyCount() == 0) {
+    if (activeRequestSlots.getValueCount() == 0) {
       activeRequestSlots.deleteKey(_toKeyId(requestId));
       activeHostRequests.deleteValue(valueId);
     }

contracts/TestMappings.sol

@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import "./libs/Mappings.sol";
+
+// exposes public functions for testing
+contract TestMappings {
+  using Mappings for Mappings.Mapping;
+
+  event OperationResult(bool result);
+
+  Mappings.Mapping private _map;
+
+  function getTotalValueCount() public view returns (uint256) {
+    return _map.getValueCount();
+  }
+
+  function getValueCount(Mappings.KeyId keyId) public view returns (uint256) {
+    return _map.getValueCount(keyId);
+  }
+
+  function keyExists(Mappings.KeyId keyId) public view returns (bool) {
+    return _map.keyExists(keyId);
+  }
+
+  function valueExists(Mappings.ValueId valueId)
+    public
+    view
+    returns (bool)
+  {
+    return _map.valueExists(valueId);
+  }
+
+  function getKeyIds() public view returns (Mappings.KeyId[] memory) {
+    return _map.getKeyIds();
+  }
+
+  function getValueIds(Mappings.KeyId keyId)
+    public
+    view
+    returns (Mappings.ValueId[] memory)
+  {
+    return _map.getValueIds(keyId);
+  }
+
+  function insertKey(Mappings.KeyId keyId) public returns (bool success) {
+    success = _map.insertKey(keyId);
+    emit OperationResult(success);
+  }
+
+  function insertValue(Mappings.KeyId keyId, Mappings.ValueId valueId)
+    public
+    returns (bool success)
+  {
+    success = _map.insertValue(keyId, valueId);
+    emit OperationResult(success);
+  }
+
+  function insert(Mappings.KeyId keyId, Mappings.ValueId valueId)
+    public
+    returns (bool success)
+  {
+    success = _map.insert(keyId, valueId);
+    emit OperationResult(success);
+  }
+
+  function deleteKey(Mappings.KeyId keyId) public returns (bool success) {
+    success = _map.deleteKey(keyId);
+    emit OperationResult(success);
+  }
+
+  function deleteValue(Mappings.ValueId valueId)
+    public
+    returns (bool success)
+  {
+    success = _map.deleteValue(valueId);
+    emit OperationResult(success);
+  }
+
+  function clearValues(Mappings.KeyId keyId)
+    public
+    returns (bool success)
+  {
+    success = _map.clearValues(keyId);
+    emit OperationResult(success);
+  }
+}

contracts/libs/Debug.sol

@@ -63,7 +63,7 @@ library Debug {
     }
     console.log("|_________________________________________________________________________________________________________________________________________|");
     console.log("  Referenced values:   ", referencedValues);
-    uint256 totalValues = Mappings.getManyCount(db);
+    uint256 totalValues = Mappings.getValueCount(db);
     console.log("  Unreferenced values: ", totalValues - referencedValues, " (total values not deleted but are unused)");
     console.log("  TOTAL Values:        ", totalValues);
   }

contracts/libs/Mappings.sol

@@ -42,17 +42,16 @@ library Mappings {
     return db._keyIds.length;
   }
 
-  function getManyCount(Mapping storage db) internal view returns(uint256) {
+  function getValueCount(Mapping storage db) internal view returns(uint256) {
     return db._valueIds.length;
   }
 
-  function getManyCount(Mapping storage db, KeyId keyId)
+  function getValueCount(Mapping storage db, KeyId keyId)
     internal
     view
-    returns(uint256 manyCount)
+    returns(uint256)
   {
-    require(keyExists(db, keyId), "key does not exist");
+    return getValueIds(db, keyId).length;
-    return _getValueIds(db, keyId).length;
   }
 
   function keyExists(Mapping storage db, KeyId keyId)
@@ -69,20 +68,18 @@ library Mappings {
     view
     returns(bool)
   {
-    if(getManyCount(db) == 0) return false;
+    if (getValueCount(db) == 0) return false;
     uint256 row = db._values[valueId]._valueIdsIndex;
     bool retVal = equals(db._valueIds[row], valueId);
     return retVal;
   }
 
-  function _getValueIds(Mapping storage db,
+  function getKeyIds(Mapping storage db)
-                        KeyId keyId)
     internal
     view
-    returns(ValueId[] storage)
+    returns(KeyId[] storage)
   {
-    require(keyExists(db, keyId), "key does not exist");
+    return db._keyIds;
-    return db._keys[keyId]._valueIds;
   }
 
   function getValueIds(Mapping storage db,
@@ -92,7 +89,7 @@ library Mappings {
     returns(ValueId[] storage)
   {
     require(keyExists(db, keyId), "key does not exist");
-    return _getValueIds(db, keyId);
+    return db._keys[keyId]._valueIds;
   }
 
   // Insert
@@ -116,7 +113,7 @@ library Mappings {
 
     Value storage value = db._values[valueId];
     db._valueIds.push(valueId);
-    value._valueIdsIndex = getManyCount(db) - 1;
+    value._valueIdsIndex = getValueCount(db) - 1;
     value._keyId = keyId; // each many has exactly one "One", so this is mandatory
 
     // We also maintain a list of "Many" that refer to the "One", so ...
@@ -149,7 +146,7 @@ library Mappings {
     returns(bool)
   {
     require(keyExists(db, keyId), "key does not exist");
-    require(_getValueIds(db, keyId).length == 0, "references manys"); // this would break referential integrity
+    require(getValueIds(db, keyId).length == 0, "references values"); // this would break referential integrity
 
     uint256 rowToDelete = db._keys[keyId]._oneListPointer;
     KeyId keyToMove = db._keyIds[keyCount(db)-1];
@@ -169,7 +166,7 @@ library Mappings {
     // delete from the Many table
     uint256 toDeleteIndex = db._values[valueId]._valueIdsIndex;
 
-    uint256 lastIndex = getManyCount(db) - 1;
+    uint256 lastIndex = getValueCount(db) - 1;
 
     if (lastIndex != toDeleteIndex) {
       ValueId lastValue = db._valueIds[lastIndex];
@@ -196,6 +193,10 @@ library Mappings {
     oneRow._valueIds.pop();
     delete oneRow._valueIdsIndex[valueId];
     delete db._values[valueId];
+
+    if (getValueCount(db, keyId) == 0) {
+      deleteKey(db, keyId);
+    }
     return true;
   }
 

test/Mappings.test.js

@@ -0,0 +1,129 @@
+const { ethers } = require("hardhat")
+const { expect } = require("chai")
+const { hexlify, randomBytes } = ethers.utils
+const { exampleAddress } = require("./examples")
+
+describe("Mappings", function () {
+  let account
+  let key
+  let value
+  let contract
+
+  describe("Mapping", function () {
+    beforeEach(async function () {
+      let Mappings = await ethers.getContractFactory("TestMappings")
+      contract = await Mappings.deploy()
+      ;[account] = await ethers.getSigners()
+      key = randomBytes(32)
+      value = randomBytes(32)
+    })
+
+    it("starts empty", async function () {
+      await expect(await contract.keyExists(key)).to.be.false
+      await expect(await contract.valueExists(value)).to.be.false
+      await expect(await contract.getKeyIds()).to.deep.equal([])
+      await expect(await contract.getTotalValueCount()).to.equal(0)
+    })
+
+    it("adds a key and value", async function () {
+      await expect(contract.insert(key, value))
+        .to.emit(contract, "OperationResult")
+        .withArgs(true)
+      await expect(await contract.getValueIds(key)).to.deep.equal([
+        hexlify(value),
+      ])
+    })
+
+    it("removes a key", async function () {
+      await contract.insertKey(key)
+      await expect(contract.deleteKey(key))
+        .to.emit(contract, "OperationResult")
+        .withArgs(true)
+      await expect(await contract.keyExists(key)).to.be.false
+    })
+
+    it("removes a value for key", async function () {
+      let value1 = randomBytes(32)
+      await contract.insert(key, value)
+      await contract.insert(key, value1)
+      await expect(contract.deleteValue(value))
+        .to.emit(contract, "OperationResult")
+        .withArgs(true)
+      await expect(await contract.getKeyIds()).to.deep.equal([hexlify(key)])
+      await expect(await contract.getValueIds(key)).to.deep.equal([
+        hexlify(value1),
+      ])
+    })
+
+    // referential integrity
+    it("fails to insert a value when key does not exist", async function () {
+      let key1 = randomBytes(32)
+      await expect(contract.insertValue(key1, value)).to.be.revertedWith(
+        "key does not exist"
+      )
+    })
+
+    it("fails to get value ids when key does not exist", async function () {
+      await expect(contract.getValueIds(key)).to.be.revertedWith(
+        "key does not exist"
+      )
+    })
+
+    it("fails to insert a value that already exists", async function () {
+      await contract.insert(key, value)
+      await expect(contract.insert(key, value))
+        .to.emit(contract, "OperationResult")
+        .withArgs(false)
+      await expect(contract.insertValue(key, value)).to.be.revertedWith(
+        "value already exists"
+      )
+    })
+
+    it("fails to remove a key when it has values", async function () {
+      let value1 = randomBytes(32)
+      await contract.insert(key, value)
+      await contract.insert(key, value1)
+      await expect(contract.deleteKey(key)).to.be.revertedWith(
+        "references values"
+      )
+    })
+
+    // counts / existence
+    it("reports correct counts and existence", async function () {
+      let value1 = randomBytes(32)
+      let value2 = randomBytes(32)
+      let value3 = randomBytes(32)
+      await contract.insert(key, value)
+      await expect(await contract.keyExists(key)).to.be.true
+      await expect(await contract.valueExists(value)).to.be.true
+      await expect(await contract.valueExists(value1)).to.be.false
+      await expect(await contract.getValueCount(key)).to.equal(1)
+      await expect(await contract.getTotalValueCount()).to.equal(1)
+
+      await contract.insert(key, value1)
+      await expect(await contract.valueExists(value1)).to.be.true
+      await expect(await contract.getValueCount(key)).to.equal(2)
+      await expect(await contract.getTotalValueCount()).to.equal(2)
+
+      await expect(contract.deleteValue(value1))
+      await expect(await contract.keyExists(key)).to.be.true
+      await expect(await contract.valueExists(value1)).to.be.false
+      await expect(await contract.getValueCount(key)).to.equal(1)
+      await expect(await contract.getTotalValueCount()).to.equal(1)
+
+      await contract.insert(key, value1)
+      await contract.insert(key, value2)
+      await contract.insert(key, value3)
+      await expect(contract.clearValues(key))
+      await expect(await contract.keyExists(key)).to.be.false
+      await expect(await contract.getKeyIds()).to.deep.equal([])
+
+      // TODO: handle unreferenced values, as visible here. Once handled, this value should be 1
+      await expect(await contract.getTotalValueCount()).to.equal(4)
+      // await expect(await contract.valueExists(value)).to.be.false
+      // await expect(await contract.valueExists(value1)).to.be.false
+      // await expect(await contract.valueExists(value2)).to.be.false
+      // await expect(await contract.valueExists(value3)).to.be.false
+    })
+  })
+})

test/SetMap.test.js

@@ -1,207 +0,0 @@
-const { ethers } = require("hardhat")
-const { expect } = require("chai")
-const { hexlify, randomBytes } = ethers.utils
-const { exampleAddress } = require("./examples")
-
-describe("SetMap", function () {
-  let account
-  let key
-  let value
-  let contract
-
-  describe("Bytes32SetMap", function () {
-    beforeEach(async function () {
-      let Bytes32SetMap = await ethers.getContractFactory("TestBytes32SetMap")
-      contract = await Bytes32SetMap.deploy()
-      ;[account] = await ethers.getSigners()
-      key = randomBytes(32)
-      value = randomBytes(32)
-    })
-
-    it("starts empty", async function () {
-      await expect(await contract.values(key, account.address)).to.deep.equal(
-        []
-      )
-    })
-
-    it("adds a key/address and value", async function () {
-      await expect(contract.add(key, account.address, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key, account.address)).to.deep.equal([
-        hexlify(value),
-      ])
-    })
-
-    it("removes a value for key/address", async function () {
-      let value1 = randomBytes(32)
-      await contract.add(key, account.address, value)
-      await contract.add(key, account.address, value1)
-      await expect(contract.remove(key, account.address, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key, account.address)).to.deep.equal([
-        hexlify(value1),
-      ])
-    })
-
-    it("clears all values for a key", async function () {
-      let key1 = randomBytes(32)
-      let value1 = randomBytes(32)
-      let value2 = randomBytes(32)
-      await contract.add(key, account.address, value)
-      await contract.add(key, account.address, value1)
-      await contract.add(key, account.address, value2)
-      await contract.add(key1, account.address, value)
-      await expect(contract.clear(key))
-      await expect(await contract.values(key, account.address)).to.deep.equal(
-        []
-      )
-      await expect(await contract.values(key1, account.address)).to.deep.equal([
-        hexlify(value),
-      ])
-    })
-
-    it("gets the length of values for a key/address", async function () {
-      let value1 = randomBytes(32)
-      let value2 = randomBytes(32)
-      await contract.add(key, account.address, value)
-      await contract.add(key, account.address, value1)
-      await contract.add(key, account.address, value2)
-      await expect(await contract.length(key, account.address)).to.equal(3)
-    })
-  })
-
-  describe("AddressBytes32SetMap", function () {
-    beforeEach(async function () {
-      let AddressBytes32SetMap = await ethers.getContractFactory(
-        "TestAddressBytes32SetMap"
-      )
-      contract = await AddressBytes32SetMap.deploy()
-      ;[account, account1] = await ethers.getSigners()
-      key = account.address
-      value = randomBytes(32)
-    })
-
-    it("starts empty", async function () {
-      await expect(await contract.values(key)).to.deep.equal([])
-    })
-
-    it("adds a key/address and value", async function () {
-      await expect(contract.add(key, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key)).to.deep.equal([hexlify(value)])
-    })
-
-    it("removes a value for key/address", async function () {
-      let value1 = randomBytes(32)
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await expect(contract.remove(key, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key)).to.deep.equal([hexlify(value1)])
-    })
-
-    it("clears all values for a key", async function () {
-      let key1 = account1.address
-      let value1 = randomBytes(32)
-      let value2 = randomBytes(32)
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await contract.add(key, value2)
-      await contract.add(key1, value)
-      await expect(contract.clear(key))
-      await expect(await contract.values(key)).to.deep.equal([])
-      await expect(await contract.values(key1)).to.deep.equal([hexlify(value)])
-    })
-  })
-
-  describe("Bytes32AddressSetMap", function () {
-    beforeEach(async function () {
-      let Bytes32AddressSetMap = await ethers.getContractFactory(
-        "TestBytes32AddressSetMap"
-      )
-      contract = await Bytes32AddressSetMap.deploy()
-      ;[account] = await ethers.getSigners()
-      key = randomBytes(32)
-      value = exampleAddress()
-    })
-
-    it("starts empty", async function () {
-      await expect(await contract.values(key)).to.deep.equal([])
-    })
-
-    it("adds a key/address and value", async function () {
-      await expect(contract.add(key, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key)).to.deep.equal([value])
-    })
-
-    it("returns list of keys", async function () {
-      let key1 = randomBytes(32)
-      let value1 = exampleAddress()
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await contract.add(key1, value)
-      await contract.add(key1, value1)
-      await expect(await contract.keys()).to.deep.equal([
-        hexlify(key),
-        hexlify(key1),
-      ])
-      await contract.remove(key1, value)
-      await expect(await contract.keys()).to.deep.equal([
-        hexlify(key),
-        hexlify(key1),
-      ])
-      await contract.remove(key1, value1)
-      await expect(await contract.keys()).to.deep.equal([hexlify(key)])
-      await contract.clear(key)
-      await expect(await contract.keys()).to.deep.equal([])
-    })
-
-    it("contains a key/value pair", async function () {
-      let key1 = randomBytes(32)
-      let value1 = exampleAddress()
-      await contract.add(key, value)
-      await contract.add(key1, value1)
-      await expect(await contract.contains(key, value)).to.equal(true)
-      await expect(await contract.contains(key1, value1)).to.equal(true)
-      await expect(await contract.contains(key1, value)).to.equal(false)
-    })
-
-    it("removes a value for key/address", async function () {
-      let value1 = exampleAddress()
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await expect(contract.remove(key, value))
-        .to.emit(contract, "OperationResult")
-        .withArgs(true)
-      await expect(await contract.values(key)).to.deep.equal([value1])
-    })
-
-    it("clears all values for a key", async function () {
-      let key1 = randomBytes(32)
-      let value1 = exampleAddress()
-      let value2 = exampleAddress()
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await contract.add(key, value2)
-      await contract.add(key1, value)
-      await expect(contract.clear(key))
-      await expect(await contract.values(key)).to.deep.equal([])
-      await expect(await contract.values(key1)).to.deep.equal([value])
-    })
-
-    it("gets the length of values for a key/address", async function () {
-      let value1 = exampleAddress()
-      let value2 = exampleAddress()
-      await contract.add(key, value)
-      await contract.add(key, value1)
-      await contract.add(key, value2)
-      await expect(await contract.length(key)).to.equal(3)
-    })
-  })
-})