fixed bug on linked list insert and add tests

contracts/RolesRegistry/SftRolesRegistry.sol

@@ -8,14 +8,12 @@ import { IERC1155 } from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol";
 import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol";
 import { ERC1155Holder, ERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/utils/ERC1155Holder.sol";
 import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
-import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import { LinkedLists } from "./libraries/LinkedLists.sol";
 
 // Semi-fungible token (SFT) roles registry
 contract SftRolesRegistry is IERCXXXX, ERC1155Holder {
     using LinkedLists for LinkedLists.Lists;
     using LinkedLists for LinkedLists.ListItem;
-    using EnumerableSet for EnumerableSet.UintSet;
 
     LinkedLists.Lists internal lists;
 

contracts/RolesRegistry/libraries/LinkedLists.sol

@@ -30,17 +30,31 @@ library LinkedLists {
 
         uint256 headerNonce = _self.headers[_headerKey];
         if (headerNonce == EMPTY) {
+            // if list is empty
             // insert as header
             _self.headers[_headerKey] = _nonce;
             _self.items[_nonce] = ListItem(_data, EMPTY, EMPTY);
-        } else {
-            // search where to insert
-            uint256 currentNonce = headerNonce;
-            while (_data.expirationDate < _self.items[currentNonce].data.expirationDate) {
-                currentNonce = _self.items[currentNonce].next;
-            }
-            insertAt(_self, currentNonce, _nonce, _data);
+            return;
+        }
+
+        if (_data.expirationDate > _self.items[headerNonce].data.expirationDate) {
+            // if expirationDate is greater than head's expirationDate
+            // update current head
+            _self.items[headerNonce].previous = _nonce;
+
+            // insert as header
+            _self.headers[_headerKey] = _nonce;
+            _self.items[_nonce] = ListItem(_data, EMPTY, headerNonce);
+            return;
         }
+
+        // search where to insert
+        uint256 currentNonce = headerNonce;
+        while (_data.expirationDate < _self.items[currentNonce].data.expirationDate && _self.items[currentNonce].next != EMPTY) {
+            currentNonce = _self.items[currentNonce].next;
+        }
+        insertAt(_self, currentNonce, _nonce, _data);
+
     }
 
     function insertAt(Lists storage _self, uint256 _previousNonce, uint256 _dataNonce, IERCXXXX.RoleData memory _data) internal {

contracts/mocks/MockLinkedLists.sol

@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: CC0-1.0
+
+pragma solidity 0.8.9;
+
+import { IERCXXXX } from "../RolesRegistry/interfaces/IERCXXXX.sol";
+import { LinkedLists } from "../RolesRegistry/libraries/LinkedLists.sol";
+
+contract MockLinkedLists {
+    using LinkedLists for LinkedLists.Lists;
+    using LinkedLists for LinkedLists.ListItem;
+
+    struct ListItem {
+        uint64 expirationDate;
+        uint256 previous;
+        uint256 next;
+    }
+
+    LinkedLists.Lists internal lists;
+
+    function insert(bytes32 _headKey, uint256 _nonce, uint64 _expirationDate) external {
+        // the only attribute that affects the list sorting is the expiration date
+        IERCXXXX.RoleData memory data = IERCXXXX.RoleData("", 1, _expirationDate, true, "");
+        lists.insert(_headKey, _nonce, data);
+    }
+
+    function remove(bytes32 _headKey, uint256 _nonce) external {
+        lists.remove(_headKey, _nonce);
+    }
+
+    function getHeadNonce(bytes32 _headKey) external view returns (uint256) {
+        return lists.headers[_headKey];
+    }
+
+    function getListItem(uint256 _nonce) public view returns (ListItem memory) {
+        LinkedLists.ListItem memory item = lists.items[_nonce];
+        return ListItem(item.data.expirationDate, item.previous, item.next);
+    }
+
+    function getListHead(bytes32 _headKey) external view returns (ListItem memory) {
+        uint256 nonce = lists.headers[_headKey];
+        return getListItem(nonce);
+    }
+}

test/LinkedLists.spec.ts

@@ -0,0 +1,82 @@
+import { Contract } from 'ethers'
+import { ethers } from 'hardhat'
+import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'
+import { expect } from 'chai'
+import { beforeEach } from 'mocha'
+import { generateRandomInt, assertListItem, assertList } from './helpers'
+const { HashZero } = ethers.constants
+
+describe('LinkedLists', async () => {
+  let LinkedLists: Contract
+
+  async function deployContracts() {
+    const MockLinkedListsFactory = await ethers.getContractFactory('MockLinkedLists')
+    LinkedLists = await MockLinkedListsFactory.deploy()
+    return { LinkedLists }
+  }
+
+  beforeEach(async () => {
+    await loadFixture(deployContracts)
+  })
+
+  describe('Insert Item', async () => {
+    it('when nonce is zero, should revert', async () => {
+      await expect(LinkedLists.insert(HashZero, 0, 1)).to.revertedWith('LinkedLists: invalid nonce')
+    })
+
+    it('when list is empty, insert item as head', async () => {
+      const nonce = generateRandomInt()
+      const expirationDate = 1
+      await expect(LinkedLists.insert(HashZero, nonce, expirationDate)).to.not.be.reverted
+      await assertListItem(LinkedLists, HashZero, nonce, expirationDate, 0)
+      await assertList(LinkedLists, HashZero, 1)
+    })
+
+    describe('List with one item', async () => {
+      let FirstItem: { nonce: number; expirationDate: number }
+
+      beforeEach(async () => {
+        FirstItem = { expirationDate: 10, nonce: generateRandomInt() }
+        await expect(LinkedLists.insert(HashZero, FirstItem.nonce, FirstItem.expirationDate)).to.not.be.reverted
+      })
+
+      it('when expiration date is greater, insert item as head', async () => {
+        const newNonce = generateRandomInt()
+        const newDate = 11
+        await expect(LinkedLists.insert(HashZero, newNonce, newDate)).to.not.be.reverted
+
+        // assert new item
+        await assertListItem(LinkedLists, HashZero, newNonce, newDate, 0)
+        // assert old item
+        await assertListItem(LinkedLists, HashZero, FirstItem.nonce, FirstItem.expirationDate, 1)
+        // assert list integrity
+        await assertList(LinkedLists, HashZero, 2)
+      })
+
+      it('when expiration date is lower, insert item as tail', async () => {
+        const newNonce = generateRandomInt()
+        const newDate = 9
+        await expect(LinkedLists.insert(HashZero, newNonce, newDate)).to.not.be.reverted
+
+        // assert new item
+        await assertListItem(LinkedLists, HashZero, newNonce, newDate, 1)
+        // assert old item
+        await assertListItem(LinkedLists, HashZero, FirstItem.nonce, FirstItem.expirationDate, 0)
+        // assert list integrity
+        await assertList(LinkedLists, HashZero, 2)
+      })
+
+      it('when expiration date is equal, insert item as tail', async () => {
+        const newNonce = generateRandomInt()
+        await expect(LinkedLists.insert(HashZero, newNonce, FirstItem.expirationDate)).to.not.be.reverted
+
+        // assert new item
+        await assertListItem(LinkedLists, HashZero, newNonce, FirstItem.expirationDate, 1)
+        // assert old item
+        await assertListItem(LinkedLists, HashZero, FirstItem.nonce, FirstItem.expirationDate, 0)
+        // assert list integrity
+        await assertList(LinkedLists, HashZero, 2)
+      })
+    })
+  })
+})

test/helpers.ts

@@ -0,0 +1,112 @@
+import { Contract } from 'ethers'
+import { expect } from 'chai'
+
+/**
+ * Validates the list length, order, head and tail
+ * @param LinkedLists The MockLinkedLists contract
+ * @param listId The bytes32 identifier of the list
+ * @param expectedLength The number of items expected in the list
+ */
+export async function assertList(LinkedLists: Contract, listId: string, expectedLength: number) {
+  const headNonce = await LinkedLists.getHeadNonce(listId)
+  if (headNonce.toNumber() === 0) {
+    return expect(expectedLength, 'List is empty, head should be zero').to.be.equal(0)
+  }
+
+  // assert head
+  let item = await LinkedLists.getListItem(headNonce)
+  expect(item.previous, 'Header previous should be zero').to.be.equal(0)
+
+  let previous = headNonce.toNumber()
+  let previousExpirationDate = item.expirationDate.toNumber()
+  let next = item.next.toNumber()
+  let listLength = 1
+  for (; next !== 0; listLength++) {
+    item = await LinkedLists.getListItem(next)
+
+    // assert previous
+    expect(item.previous, 'Wrong previous item').to.be.equal(previous)
+
+    // assert decreasing order for expiration date
+    expect(item.expirationDate.toNumber(), 'Wrong order for expiration date').to.be.lessThanOrEqual(
+      previousExpirationDate,
+    )
+
+    // update all references
+    previous = next
+    previousExpirationDate = item.expirationDate.toNumber()
+    next = item.next.toNumber()
+  }
+
+  // assert tail
+  expect(item.next, 'Tail next should be zero').to.be.equal(0)
+
+  // assert list length
+  expect(listLength, 'List does not have the expected length').to.be.equal(expectedLength)
+}
+
+/**
+ * Validates the item nonce, expiration date, and position in the list
+ * @param LinkedLists The MockLinkedLists contract
+ * @param listId The bytes32 identifier of the list
+ * @param itemNonce The nonce of the item
+ * @param itemExpirationDate The expiration date of the item
+ * @param expectedPosition The expected position of the item in the list
+ */
+export async function assertListItem(
+  LinkedLists: Contract,
+  listId: string,
+  itemNonce: number,
+  itemExpirationDate: number,
+  expectedPosition: number,
+) {
+  const { expirationDate, previous, next } = await LinkedLists.getListItem(itemNonce)
+  expect(expirationDate, `Item ${itemNonce} expiration date is not ${itemExpirationDate}`).to.be.equal(
+    itemExpirationDate,
+  )
+
+  if (expectedPosition === 0) {
+    // if item is the header
+    expect(await LinkedLists.getHeadNonce(listId), `Item ${itemNonce} should be the header`).to.equal(itemNonce)
+    return expect(previous, 'Header previous should be zero').to.be.equal(0)
+  }
+
+  // if item is not the header
+  expect(previous, 'Item previous should not be zero').to.not.be.equal(0)
+
+  // assert position
+  let position = 0
+  let item = await LinkedLists.getListHead(listId)
+  while (item.next.toNumber() !== 0) {
+    item = await LinkedLists.getListItem(item.next)
+    position += 1
+  }
+  expect(position, 'Item is not on expected position').to.be.equal(expectedPosition)
+}
+
+export async function printList(LinkedLists: Contract, listId: string) {
+  console.log('\n== List ==============================================')
+  const headNonce = (await LinkedLists.getHeadNonce(listId)).toNumber()
+  if (headNonce === 0) {
+    return console.log('\tList is empty!')
+  }
+
+  let position = 0
+  let currentNonce = headNonce
+  while (currentNonce !== 0) {
+    const currentItem = await LinkedLists.getListItem(currentNonce)
+    console.log(`\n\tItem ${position}:`)
+    console.log(`\t\tNonce: ${currentNonce}`)
+    console.log(`\t\tExpiration Date: ${currentItem.expirationDate}`)
+    console.log(`\t\tPrevious: ${currentItem.previous.toNumber()}`)
+    console.log(`\t\tNext: ${currentItem.next.toNumber()}`)
+    currentNonce = currentItem.next.toNumber()
+    position += 1
+  }
+
+  console.log('\n== End of List =======================================\n')
+}
+
+export function generateRandomInt() {
+  return Math.floor(Math.random() * 1000 * 1000) + 1
+}