Able to initialize Semaphore with groupId starting from non-zero value

[jimmychu0807]

Describe the improvement you're thinking about

Semaphore starts with groupId = 0. Being able to have groupId starts with 1 (or non-zero value) maybe more convenient in certain cases, so checking an object with groupId == 0 mean the object/entity doesn't belong to any valid group.

Currently there is no straightforward way to initialize the groupId to start from values other than 0.

Describe alternatives you've considered

We can always have another variable to indicate if an object/entity belongs to a valid group.

[jimmychu0807]

I tried to give this a shot this morning, did something as follows:

contract Semaphore is ISemaphore, SemaphoreGroups {
    ISemaphoreVerifier public verifier;

    /// @dev Gets a group id and returns the group parameters.
    mapping(uint256 => Group) public groups;

    /// @dev Counter to assign an incremental id to the groups.
    /// This counter is used to keep track of the number of groups created.
    uint256 public groupCounter;

    /// @dev With this variable allow groupId to start with non-zero, while keeping
    /// groupCounter to be the total number of groups.
    uint256 private groupIdOffset;

    /// @dev Initializes the Semaphore verifier used to verify the user's ZK proofs.
    /// @param _verifier: Semaphore verifier addresse.
    constructor(ISemaphoreVerifier _verifier) {
        constructor(_verifier, 0);
    }

    constructor(ISemaphoreVerifier _verifier, uint256 _groupIdOffset) {
        verifier = _verifier;
        groupIdOffset = _groupIdOffset;
    }

    function nextGroupId() internal returns (uint256 groupId) {
        groupId = groupIdOffset + groupCounter;
        groupCounter += 1;
    }

    /// @dev See {SemaphoreGroups-_createGroup}.
    function createGroup() external override returns (uint256 groupId) {
        groupId = nextGroupId();
        _createGroup(groupId, msg.sender);

        groups[groupId].merkleTreeDuration = 1 hours;
    }
  //...
}

And then realized Solidity doesn't support constructor overloading. I don't have a good way of adding the groupIdOffset parameter without changing the constructor interface. And that would be an API change, a major version bump.

If we want to solve this issue, I guess the best way is to pick this up in the next major release.