Skip to content

Commit

Permalink
Bump solhint & npm cleanup (#12565)
Browse files Browse the repository at this point in the history
* fix lock file version

* cleanup npm deps

* rm not needed solhint ignores

* bump solhint and fix rename custom-errors -> gas-custom-errors

* turn on struct packing & interface-starts-with-i solhint rule

* rm changeset from CODEOWNERS to not ping the root folder owner

* add changelog entry
  • Loading branch information
RensR authored Mar 25, 2024
1 parent 1242f1e commit b673505
Show file tree
Hide file tree
Showing 68 changed files with 2,297 additions and 2,942 deletions.
2 changes: 2 additions & 0 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ core/scripts/gateway @smartcontractkit/functions

# At the end, match any files missed by the patterns above
/contracts/scripts/native_solc_compile_all_events_mock @smartcontractkit/functions
# Remove changeset files from the codeowners
/contracts/.changeset


# Tests
Expand Down
5 changes: 5 additions & 0 deletions contracts/.changeset/weak-kangaroos-heal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@chainlink/contracts": patch
---

bump solhint and address issues, remove unused imports
2 changes: 2 additions & 0 deletions contracts/.solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"no-inline-assembly": "off",
"contract-name-camelcase": "off",
"no-unused-import": "error",
"gas-struct-packing": "warn",
"interface-starts-with-i": "warn",
"func-visibility": [
"error",
{
Expand Down
13 changes: 8 additions & 5 deletions contracts/STYLE_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ uint256 networkFeeUSDCents; // good

### Structs

- All structs should be packed to have the lowest memory footprint to reduce gas usage. Even structs that will never be written to storage should be packed.
- A contract can be considered a struct; it should also be packed to reduce gas cost.
- Structs should contain struct packing comments to clearly indicate the storage slot layout
- Using the exact characters from the example below will ensure visually appealing struct packing comments.
- Notice there is no line on the unpacked last `fee` item.
Expand Down Expand Up @@ -378,17 +376,22 @@ function getNum() external view returns (uint64 num) {

Use [custom errors](https://blog.soliditylang.org/2021/04/21/custom-errors/) instead of emitting strings. This saves contract code size and simultaneously provides more informative error messages.

rule: `custom-errors`
rule: `gas-custom-errors`

## Interfaces

Interfaces should be named `IFoo` instead of `FooInterface`. This follows the patterns of popular [libraries like OpenZeppelin’s](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/IERC20.sol#L9).

rule: `tbd`
rule: `interface-starts-with-i`

## Structs

Structs should be constructed with named arguments. This prevents accidental assignment to the wrong field and makes the code more readable.
- All structs should be packed to have the lowest memory footprint to reduce gas usage. Even structs that will never be written to storage should be packed.
- A contract can be considered a struct; it should also be packed to reduce gas cost.

rule: `gas-struct-packing`

- Structs should be constructed with named arguments. This prevents accidental assignment to the wrong field and makes the code more readable.

```solidity
// Good
Expand Down
9 changes: 3 additions & 6 deletions contracts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"prepublishOnly": "pnpm compile && ./scripts/prepublish_generate_abi_folder",
"publish-beta": "pnpm publish --tag beta",
"publish-prod": "npm dist-tag add @chainlink/[email protected] latest",
"solhint": "solhint --max-warnings 20 \"./src/v0.8/**/*.sol\""
"solhint": "solhint --max-warnings 85 \"./src/v0.8/**/*.sol\""
},
"files": [
"src/v0.8",
Expand All @@ -37,13 +37,11 @@
"@ethersproject/bignumber": "~5.7.0",
"@ethersproject/contracts": "~5.7.0",
"@ethersproject/providers": "~5.7.2",
"@ethersproject/random": "~5.7.0",
"@nomicfoundation/hardhat-network-helpers": "^1.0.9",
"@nomiclabs/hardhat-ethers": "^2.2.3",
"@nomiclabs/hardhat-etherscan": "^3.1.7",
"@nomiclabs/hardhat-waffle": "2.0.6",
"@openzeppelin/hardhat-upgrades": "1.28.0",
"@openzeppelin/test-helpers": "^0.5.16",
"@typechain/ethers-v5": "^7.2.0",
"@typechain/hardhat": "^7.0.0",
"@types/cbor": "5.0.1",
Expand Down Expand Up @@ -74,14 +72,13 @@
"prettier": "^3.2.5",
"prettier-plugin-solidity": "1.3.1",
"rlp": "^2.2.7",
"solhint": "^4.1.1",
"solhint": "^4.5.2",
"solhint-plugin-chainlink-solidity": "git+https://github.com/smartcontractkit/chainlink-solhint-rules.git#v1.2.1",
"solhint-plugin-prettier": "^0.1.0",
"solidity-coverage": "^0.8.5",
"ts-node": "^10.9.2",
"tslib": "^2.6.2",
"typechain": "^8.2.1",
"typescript": "^5.3.3"
"typescript": "^5.4.3"
},
"dependencies": {
"@eth-optimism/contracts": "0.6.0",
Expand Down
Loading

0 comments on commit b673505

Please sign in to comment.