Implementation of the Stake table contract (register function)

[philippecamacho]

Part of #737.

Implements the register function of the Stake Table contract.
To test:

just sol-test

[alxiong]

General comment:
all variables/struct/event, all functions (including those defined in IStakeTable.sol, except constructor) should add code doc.
note that @notice is the general doc, @dev is dev-specific info, I noticed that sometimes we misuse the latter for the former. When in doubt, consult NatSpec.

[philippecamacho]

Two general comments:

• the order of tests should follow the logic of contract: if contract check StakeType first, then first few test functions should be about fail if it's wrong

Done in ce1ebbf .

• the order of the test contract (within the same StakeTable.t.sol file) should follow the order of their target function in the implementation: if in StakeTable.sol, the order is regiseter() then deposit() then in the test file, it should be contract StakeTable_register_Test and contract StakeTable_deposit_Test.

Should not we have a different file for each important Stake table contract function like register, deposit etc ...?

whenever there's external/public function with potential user-input, we should add testFuzz on these as much as possible. For example, we never add validity check on blsVK, if we use fuzzing, then it could be that user is passing in an invalid G2/G1 point. which we should also be able to handle.

Yes, added a TODO in the ticket #737.

[philippecamacho]

General comment: all variables/struct/event, all functions (including those defined in IStakeTable.sol, except constructor) should add code doc. note that @notice is the general doc, @dev is dev-specific info, I noticed that sometimes we misuse the latter for the former. When in doubt, consult NatSpec.

Done in 3c97955.

[alxiong]

The test code looks so much better and idiosyncratic! it's very easy to read them!! 👍

I left some comments to improve the quality and coverage of the logic that I believe can make our future contract auditor happier.

[alxiong]

the order of the test contract (within the same StakeTable.t.sol file) should follow the order of their target function in the implementation: if in StakeTable.sol, the order is regiseter() then deposit() then in the test file, it should be contract StakeTable_register_Test and contract StakeTable_deposit_Test.

Should not we have a different file for each important Stake table contract function like register, deposit etc ...?

this depends on preferences as the best practice guides (1st point here) says:

For testing MyContract.sol, the test file should be MyContract.t.sol. For testing MyScript.s.sol, the test file should be MyScript.t.sol.

• If the contract is big and you want to split it over multiple files, group them logically like MyContract.owner.t.sol, MyContract.deposits.t.sol, etc.

So I guess it depends, we can split the file when it's getting big. but for now, for easier location, just keep it in StakeTable.t.sol single file.

[philippecamacho]

the order of the test contract (within the same StakeTable.t.sol file) should follow the order of their target function in the implementation: if in StakeTable.sol, the order is regiseter() then deposit() then in the test file, it should be contract StakeTable_register_Test and contract StakeTable_deposit_Test.

Should not we have a different file for each important Stake table contract function like register, deposit etc ...?

this depends on preferences as the best practice guides (1st point here) says:

For testing MyContract.sol, the test file should be MyContract.t.sol. For testing MyScript.s.sol, the test file should be MyScript.t.sol.

• If the contract is big and you want to split it over multiple files, group them logically like MyContract.owner.t.sol, MyContract.deposits.t.sol, etc.

So I guess it depends, we can split the file when it's getting big. but for now, for easier location, just keep it in StakeTable.t.sol single file.

Sounds good, let us try with a single file for now.