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

add workflow to check formatting #10

Merged
merged 8 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/fmt.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name: format check

on:
pull_request:
branches:
- main

jobs:
style:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: dprint/check@v2.2
32 changes: 26 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function validate(UserOperation calldata userOp) external returns (bytes memory

If a function or set of functions could reasonably be defined as its own contract or as a part of a larger contract, prefer defining it as part of a larger contract. This makes the code easier to understand and audit.

Note this _does not_ mean that we should avoid inheritance, in general. Inheritance is useful at times, most especially when building on existing, trusted contracts. For example, _do not_ reimplement `Ownable` functionality to avoid inheritance. Inherit `Ownable` from a trusted vendor, such as [OpenZeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/) or [Solady](https://github.com/Vectorized/solady).
Note this _does not_ mean that we should avoid inheritance, in general. Inheritance is useful at times, most especially when building on existing, trusted contracts. For example, _do not_ reimplement `Ownable` functionality to avoid inheritance. Inherit `Ownable` from a trusted vendor, such as [OpenZeppelin](https://github.com/OpenZeppelin/openzeppelin-contracts/) or [Solady](https://github.com/Vectorized/solady).

#### 7. Avoid writing interfaces.

Expand All @@ -138,6 +138,7 @@ pragma solidity ^0.8.0;
##### B. If a struct or error is used across many files, with no interface, contract, or library reasonably being the "owner," then define them in their own file. Multiple structs and errors can be defined together in one file.

#### 10. Imports

##### A. Use named imports.

Named imports help readers understand what exactly is being used and where it is originally declared.
Expand All @@ -157,25 +158,32 @@ import {Contract} from "./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:

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

YES:

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

##### C. Group imports by external and local with a new line in between.

For example

```solidity
import {Math} from '/solady/Math.sol'

import {MyHelper} from './MyHelper.sol'
```

In test files, imports from `/test` should be their own group, as well.
In test files, imports from `/test` should be their own group, as well.

```solidity
import {Math} from '/solady/Math.sol'
Expand Down Expand Up @@ -329,22 +337,30 @@ function test_transferFrom_creditsTo(uint amount) {
assertEq(balanceOf(to), amount);
}
```

### C. Project Setup
#### 1. Avoid custom remappings.
[Remappings](https://book.getfoundry.sh/projects/dependencies?#remapping-dependencies) help Forge find dependencies based on import statements. Forge will automatically deduce some remappings, for example

#### 1. Avoid custom remappings.

[Remappings](https://book.getfoundry.sh/projects/dependencies?#remapping-dependencies) help Forge find dependencies based on import statements. Forge will automatically deduce some remappings, for example

```rust
forge-std/=lib/forge-std/src/
solmate/=lib/solmate/src/
```

We should avoid adding to these or defining any remappings explicitly, as it makes our project harder for others to use as a dependency. For example, if our project depends on Solmate and so does theirs, we want to avoid our project having some irregular import naming, resolved with a custom remapping, which will conflict with their import naming.

### D. Upgradability

#### 1. Prefer [ERC-7201](https://eips.ethereum.org/EIPS/eip-7201) "Namespaced Storage Layout" convention to avoid storage collisions.

### E. Structs

#### 1. Where possible, struct values should be packed to minimize SLOADs and SSTOREs.
#### 2. Timestamp fields in a struct should be at least uint32 and ideally be uint40.

#### 2. Timestamp fields in a struct should be at least uint32 and ideally be uint40.

`uint32` will give the contract ~82 years of validity `(2^32 / (60*60*24*365)) - (2024 - 1970)`. If space allows, uint40 is the preferred size.

## 3. NatSpec
Expand Down Expand Up @@ -376,6 +392,7 @@ struct Position {
For easier reading, add a new line between tag types, when multiple are present and there are three or more lines.

NO:

```solidity
/// @notice ...
/// @dev ...
Expand All @@ -386,6 +403,7 @@ NO:
```

YES:

```solidity
/// @notice ...
///
Expand All @@ -400,8 +418,10 @@ YES:

#### 4. Author should be Coinbase.

If you using the `@author` tag, it should be
If you using the `@author` tag, it should be

```solidity
/// @author Coinbase
```

Optionally followed by a link to the public Github repository.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
{
"dependencies": { "dprint": "^0.45.0", "simple-git-hooks": "^2.11.0" },
"scripts": {
"format": "dprint fmt"
"format": "dprint fmt",
"format-check": "dprint check"
},
"simple-git-hooks": {
"pre-commit": "bun run format"
"pre-commit": "bun run format-check"
}
}
Loading