Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refined coding standards and cascaded changes to the code base #366

Merged
merged 10 commits into from
Apr 29, 2021
Merged
14 changes: 13 additions & 1 deletion CODING_STANDARDS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,19 @@ We adhere to the `pep8` standard, using `flake8`, `pylint` and `mypy` to catch f

## Solidity

As of now, the solidity code does not follow consistent style conventions. Nevertheless, new solidity code must adhere to the [solidity coding standards](https://docs.soliditylang.org/en/develop/style-guide.html). Old code will progressively be updated to converge towards the recommended style.
Solidity code must adhere to the [solidity coding standards](https://docs.soliditylang.org/en/develop/style-guide.html), with the following amendments:
- Test functions must have a `test` prefix (e.g. `testReverseBytes`)
- Private/internal state variables and functions must have an underscore prefix (e.g. `_myInternalFunction`)
- The order in which contract members are written must driven by their scope (i.e. `public/external` functions must appear first in the contract code, `internal/private` functions must appear last). Additionally, `virtual` functions in abstract contracts must be written last in their visibility group.
- Function parameters must not be prefixed with an underscore
- Interface names must have a capital I prefix (e.g. `IERC20`)
AntoineRondelet marked this conversation as resolved.
Show resolved Hide resolved
- Library names must have a `Lib` prefix (e.g. `LibPairing`)
- Test contract names must have a `Test` prefix (e.g. `TestMyContract`)
- Abstract contract names must have an `Abstract` prefix (e.g. `AbstractMyContract`)
- Contract names may not be PascalCase if using PascalCase is introducing confusions in the name (e.g. `BLS12377.sol` vs `BLS12_377.sol`). PascalCase should be used whenever possible
- Event names must be prefixed by `Log` (e.g. `LogDeposit`)

**Note:** Some of the files of the current code base may not fully comply with the coding standards above. Old code will progressively be updated to converge towards the recommended style.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More precisely, the snake_case convention remains broadly used in function bodies (we still have mixed styles there). The function signatures and contract's state variables/constants however should be all fine now


## C++

Expand Down
4 changes: 2 additions & 2 deletions client/test_contracts/test_bls12_377_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ class TestBLS12_377Contract(TestCase):

@staticmethod
def setUpClass() -> None:
print("Deploying BLS12_377_test.sol")
print("Deploying TestBLS12_377.sol")
_web3, eth = mock.open_test_web3()
_bls12_interface, bls12_instance = mock.deploy_contract(
eth,
eth.accounts[0],
"BLS12_377_test",
"TestBLS12_377",
{})
global BLS12_INSTANCE # pylint: disable=global-statement
BLS12_INSTANCE = bls12_instance
Expand Down
4 changes: 2 additions & 2 deletions client/test_contracts/test_bw6_761_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ class TestBW6_761Contract(TestCase):

@staticmethod
def setUpClass() -> None:
print("Deploying BW6_761_test.sol")
print("Deploying TestBW6_761.sol")
_web3, eth = mock.open_test_web3()
_bw6_interface, bw6_instance = mock.deploy_contract(
eth,
eth.accounts[0],
"BW6_761_test",
"TestBW6_761",
{})
global BW6_INSTANCE # pylint: disable=global-statement
BW6_INSTANCE = bw6_instance
Expand Down
265 changes: 0 additions & 265 deletions client/test_contracts/test_groth16_altbn128_mixer_base.py

This file was deleted.

6 changes: 3 additions & 3 deletions client/test_contracts/test_groth16_bls12_377_contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ def setUpClass() -> None:
contracts_dir = get_contracts_dir()
contract_instance_desc = InstanceDescription.deploy(
web3,
join(contracts_dir, "Groth16BLS12_377_test.sol"),
"Groth16BLS12_377_test",
join(contracts_dir, "TestGroth16BLS12_377.sol"),
"TestGroth16BLS12_377",
web3.eth.accounts[0], # pylint: disable=no-member
None,
500000,
Expand All @@ -92,7 +92,7 @@ def _invoke_groth16_bls12_377_verify(
vk, BLS12_377_PAIRING)
proof_evm = Groth16.proof_to_contract_parameters(proof, BLS12_377_PAIRING)
inputs_evm = hex_list_to_uint256_list(inputs)
return CONTRACT_INSTANCE.functions.verifyTest(
return CONTRACT_INSTANCE.functions.testVerify(
vk_evm, proof_evm, inputs_evm).call()

def test_groth16_bls12_377_valid(self) -> None:
Expand Down
Loading