Including optional time for isValid in server

[HughParry]

No description provided.

[pr-explainer-bot]

Pull Request Report

Hey there! I've generated a report for your pull request. Let's dive in!

Changes

1. Added an optional maxVerifiedTime parameter to the isVerified function in server.ts (line 111).
2. Added a comment to explain the purpose of the payload parameter (line 113).
3. Added a comment to explain the purpose of the maxVerifiedTime parameter (line 114).

Suggestions

1. Consider providing a more descriptive return type for the isVerified function (line 114).
2. Consider adding a comment to explain the expected format of the returns section in the JSDoc (line 114).

Bugs

1. Potential bug: In the isVerified function, the condition (maxVerifiedTime && blockNumber) should be updated to (maxVerifiedTime !== undefined && blockNumber !== undefined) to handle cases where maxVerifiedTime or blockNumber could be 0 (line 116).

Improvements

1. The isVerified function could benefit from better variable naming. Consider using more descriptive names for variables like user, dapp, providerUrl, commitmentId, and blockNumber (line 118).
2. The isVerified function could be refactored to improve readability. Here's a suggested code snippet:

public async isVerified(payload: ProcaptchaOutput, maxVerifiedTime?: number): Promise<boolean> {
    const { user, dapp, providerUrl, commitmentId, blockNumber } = payload;

    if (maxVerifiedTime !== undefined && blockNumber !== undefined) {
        const currentBlockNumber = (await this.getApi().rpc.chain.getBlock()).block.header.number.toNumber();
        const blockLength = this.getApi().consts.babe.expectedBlockTime.toNumber();

        if ((currentBlockNumber - blockNumber) * blockLength > maxVerifiedTime) {
            return false;
        }
    }

    const contractApi = await this.getContractApi();
    const block = (await this.getApi().rpc.chain.getBlockHash(blockNumber)) as BlockHash;
    const getRandomProviderResponse = await this.getContract().queryAtBlock<RandomProvider>(block, contractApi.query.getRandomProvider);

    // Rest of the code...
}

Rating

I would rate the code a 7 out of 10. The code has good readability and the added optional maxVerifiedTime parameter is a useful addition. However, there are a few areas for improvement, such as better variable naming and handling edge cases in the isVerified function.

That's it for the report! Let me know if you need any further assistance. Happy coding!

[forgetso]

Looks good, could you stick the time stuff in a separate function?

stale