Skip to content

Commit

Permalink
Yes before no
Browse files Browse the repository at this point in the history
  • Loading branch information
anikaraghu committed May 2, 2024
1 parent df88488 commit a99f6a0
Showing 1 changed file with 56 additions and 50 deletions.
106 changes: 56 additions & 50 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,20 +63,26 @@ Events should track things that _happened_ and so should be past tense. Using pa

We are aware this does not follow precedent from early ERCs, like [ERC-20](https://eips.ethereum.org/EIPS/eip-20). However it does align with some more recent high profile Solidity, e.g. [1](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/976a3d53624849ecaef1231019d2052a16a39ce4/contracts/access/Ownable.sol#L33), [2](https://github.com/aave/aave-v3-core/blob/724a9ef43adf139437ba87dcbab63462394d4601/contracts/interfaces/IAaveOracle.sol#L25-L31), [3](https://github.com/ProjectOpenSea/seaport/blob/1d12e33b71b6988cbbe955373ddbc40a87bd5b16/contracts/zones/interfaces/PausableZoneEventsAndErrors.sol#L25-L41).

YES:

```solidity
event OwnerUpdated(address newOwner);
```

NO:

```solidity
event OwnerUpdate(address newOwner);
```

##### B. Prefer `SubjectVerb` naming format.

YES:

```solidity
event OwnerUpdated(address newOwner);
```

##### B. Prefer `SubjectVerb` naming format.

NO:

```solidity
Expand Down Expand Up @@ -113,28 +119,28 @@ function validate(UserOperation calldata userOp) external returns (bytes memory

However, it is important to be explicit when returning early.

NO:
YES:

```solidity
function validate(UserOperation calldata userOp) external returns (bytes memory context, uint256 validationData) {
context = "";
validationData = 1;
if (condition) {
return;
return (context, validationData);
}
}
```

YES:
NO:

```solidity
function validate(UserOperation calldata userOp) external returns (bytes memory context, uint256 validationData) {
context = "";
validationData = 1;
if (condition) {
return (context, validationData);
return;
}
}
```
Expand Down Expand Up @@ -169,34 +175,34 @@ pragma solidity ^0.8.0;

Named imports help readers understand what exactly is being used and where it is originally declared.

NO:
YES:

```solidity
import "./contract.sol"
import {Contract} from "./contract.sol"
```

YES:
NO:

```solidity
import {Contract} from "./contract.sol"
import "./contract.sol"
```

For convenience, named imports do not have to be used in test files.

##### B. Order imports alphabetically (A to Z) by file name.

NO:
YES:

```solidity
import {B} from './B.sol'
import {A} from './A.sol'
import {B} from './B.sol'
```

YES:
NO:

```solidity
import {A} from './A.sol'
import {B} from './B.sol'
import {A} from './A.sol'
```

##### C. Group imports by external and local with a new line in between.
Expand Down Expand Up @@ -243,32 +249,32 @@ ASCII art is permitted in the space between the end of the Pragmas and the begin

Passing arguments to functions, events, and errors with explicit naming is helpful for clarity, especially when the name of the variable passed does not match the parameter name.

NO:
YES:

```
pow(x, y, v)
pow({base: x, exponent: y, scalar: v})
```

YES:
NO:

```
pow({base: x, exponent: y, scalar: v})
pow(x, y, v)
```

#### 14. Prefer named parameters in mapping types.

Explicit naming parameters in mapping types is helpful for clarity, especially when nesting is used.

NO:
YES:

```
mapping(uint256 => mapping(address => uint256)) public balances;
mapping(address account => mapping(address asset => uint256 amount)) public balances;
```

YES:
NO:

```
mapping(address account => mapping(address asset => uint256 amount)) public balances;
mapping(uint256 => mapping(address => uint256)) public balances;
```

## 2. Development
Expand Down Expand Up @@ -308,17 +314,6 @@ contract TransferFromTest {

This is generally good practice, but especially so because Forge does not give line numbers on assertion failures. This makes it hard to track down what, exactly, failed if a test has many assertions.

NO:

```solidity
function test_transferFrom_works() {
// debits correctly
// credits correctly
// emits correctly
// reverts correctly
}
```

YES:

```solidity
Expand All @@ -339,6 +334,17 @@ function test_transferFrom_reverts_whenAmountExceedsBalance() {
}
```

NO:

```solidity
function test_transferFrom_works() {
// debits correctly
// credits correctly
// emits correctly
// reverts correctly
}
```

Note, this does not mean a test should only ever have one assertion. Sometimes having multiple assertions is helpful for certainty on what is being tested.

```solidity
Expand All @@ -351,47 +357,47 @@ function test_transferFrom_creditsTo() {

#### 5. Use variables for important values in tests

NO:
YES:

```solidity
function test_transferFrom_creditsTo() {
assertEq(balanceOf(to), 0);
transferFrom(from, to, 10);
assertEq(balanceOf(to), 10);
uint amount = 10;
transferFrom(from, to, amount);
assertEq(balanceOf(to), amount);
}
```

YES:
NO:

```solidity
function test_transferFrom_creditsTo() {
assertEq(balanceOf(to), 0);
uint amount = 10;
transferFrom(from, to, amount);
assertEq(balanceOf(to), amount);
transferFrom(from, to, 10);
assertEq(balanceOf(to), 10);
}
```

#### 6. Prefer fuzz tests.

All else being equal, prefer fuzz tests.

NO:
YES:

```solidity
function test_transferFrom_creditsTo() {
function test_transferFrom_creditsTo(uint amount) {
assertEq(balanceOf(to), 0);
uint amount = 10;
transferFrom(from, to, amount);
assertEq(balanceOf(to), amount);
}
```

YES:
NO:

```solidity
function test_transferFrom_creditsTo(uint amount) {
function test_transferFrom_creditsTo() {
assertEq(balanceOf(to), 0);
uint amount = 10;
transferFrom(from, to, amount);
assertEq(balanceOf(to), amount);
}
Expand Down Expand Up @@ -450,28 +456,28 @@ struct Position {

For easier reading, add a new line between tag types, when multiple are present and there are three or more lines.

NO:
YES:

```solidity
/// @notice ...
///
/// @dev ...
/// @dev ...
///
/// @param ...
/// @param ...
///
/// @return
```

YES:
NO:

```solidity
/// @notice ...
///
/// @dev ...
/// @dev ...
///
/// @param ...
/// @param ...
///
/// @return
```

Expand Down

0 comments on commit a99f6a0

Please sign in to comment.