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

Fix: gas custom error new require feature #613

Merged
merged 3 commits into from
Dec 20, 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
24 changes: 12 additions & 12 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ title: "Rule Index of Solhint"

## Gas Consumption Rules

| Rule Id | Error | Recommended | Deprecated |
| ----------------------------------------------------------------------------- | ------------------------------------------------------------------------------------ | ------------ | ---------- |
| [gas-calldata-parameters](./rules/gas-consumption/gas-calldata-parameters.md) | Suggest calldata keyword on function arguments when read only | | |
| [gas-custom-errors](./rules/gas-consumption/gas-custom-errors.md) | Enforces the use of Custom Errors over Require and Revert statements | $~~~~~~~~$✔️ | |
| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md) | Suggest increments by one, like this ++i instead of other type | | |
| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | |
| [gas-length-in-loops](./rules/gas-consumption/gas-length-in-loops.md) | Suggest replacing object.length in a loop condition to avoid calculation on each lap | | |
| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | |
| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named | | |
| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md) | Keep strings smaller than 32 bytes | | |
| [gas-strict-inequalities](./rules/gas-consumption/gas-strict-inequalities.md) | Suggest Strict Inequalities over non Strict ones | | |
| [gas-struct-packing](./rules/gas-consumption/gas-struct-packing.md) | Suggest to re-arrange struct packing order when it is inefficient | | |
| Rule Id | Error | Recommended | Deprecated |
| ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ------------ | ---------- |
| [gas-calldata-parameters](./rules/gas-consumption/gas-calldata-parameters.md) | Suggest calldata keyword on function arguments when read only | | |
| [gas-custom-errors](./rules/gas-consumption/gas-custom-errors.md) | Enforces the use of Custom Errors over Require with strings error and Revert statements | $~~~~~~~~$✔️ | |
| [gas-increment-by-one](./rules/gas-consumption/gas-increment-by-one.md) | Suggest increments by one, like this ++i instead of other type | | |
| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | |
| [gas-length-in-loops](./rules/gas-consumption/gas-length-in-loops.md) | Suggest replacing object.length in a loop condition to avoid calculation on each lap | | |
| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | |
| [gas-named-return-values](./rules/gas-consumption/gas-named-return-values.md) | Enforce the return values of a function to be named | | |
| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md) | Keep strings smaller than 32 bytes | | |
| [gas-strict-inequalities](./rules/gas-consumption/gas-strict-inequalities.md) | Suggest Strict Inequalities over non Strict ones | | |
| [gas-struct-packing](./rules/gas-consumption/gas-struct-packing.md) | Suggest to re-arrange struct packing order when it is inefficient | | |


## Miscellaneous
Expand Down
20 changes: 19 additions & 1 deletion docs/rules/gas-consumption/gas-custom-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ title: "gas-custom-errors | Solhint"


## Description
Enforces the use of Custom Errors over Require and Revert statements
Enforces the use of Custom Errors over Require with strings error and Revert statements

## Options
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Defaults to warn.
Expand Down Expand Up @@ -44,6 +44,24 @@ revert CustomErrorFunction();
revert CustomErrorFunction({ msg: "Insufficient Balance" });
```

#### Use of Require with Custom Error with arguments

```solidity
require(success, CustomErrorFunction({ msg: "Insufficient Balance" });
```

#### Use of Require with function call and Custom Error

```solidity
require(isAuthorized(account), CustomErrorFunction();
```

#### Use of Require with binary comparison and Custom Error

```solidity
require(a > b, CustomErrorFunction();
```

### 👎 Examples of **incorrect** code for this rule

#### Use of require statement
Expand Down
19 changes: 16 additions & 3 deletions lib/rules/gas-consumption/gas-custom-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const meta = {
type: 'gas-consumption',

docs: {
description: 'Enforces the use of Custom Errors over Require and Revert statements',
description:
'Enforces the use of Custom Errors over Require with strings error and Revert statements',
category: 'Gas Consumption Rules',
options: [
{
Expand All @@ -32,6 +33,18 @@ const meta = {
description: 'Use of Custom Errors with arguments',
code: 'revert CustomErrorFunction({ msg: "Insufficient Balance" });',
},
{
description: 'Use of Require with Custom Error with arguments',
code: 'require(success, CustomErrorFunction({ msg: "Insufficient Balance" });',
},
{
description: 'Use of Require with function call and Custom Error',
code: 'require(isAuthorized(account), CustomErrorFunction();',
},
{
description: 'Use of Require with binary comparison and Custom Error',
code: 'require(a > b, CustomErrorFunction();',
},
],
bad: [
{
Expand Down Expand Up @@ -75,9 +88,9 @@ class GasCustomErrorsChecker extends BaseChecker {

FunctionCall(node) {
let errorStr = ''

if (this.isVersionGreater(node)) {
if (node.expression.name === 'require') {
// added second part of conditional to be able to use require with Custom Errors
if (node.expression.name === 'require' && node.arguments[1].type !== 'FunctionCall') {
errorStr = 'require'
} else if (
node.expression.name === 'revert' &&
Expand Down
70 changes: 67 additions & 3 deletions test/rules/gas-consumption/gas-custom-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,70 @@ function replaceSolidityVersion(code, newVersion) {
}

describe('Linter - gas-custom-errors', () => {
it('should NOT raise error for require with comparison and custom error()', () => {
let code = funcWith(`require(a > b, CustomErrorEmitted());`)
code = replaceSolidityVersion(code, '^0.8.4')

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
})

assertNoWarnings(report)
assertNoErrors(report)
})

it('should NOT raise error for require with function call and comparison along with custom error', () => {
let code = funcWith(`require(isAok(a) > b, CustomErrorEmitted(param1));`)
code = replaceSolidityVersion(code, '^0.8.4')

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
})

assertNoWarnings(report)
assertNoErrors(report)
})

it('should NOT raise error for require with boolean check and custom error call', () => {
let code = funcWith(`require(success, CustomErrorEmitted(param1, param2));`)
code = replaceSolidityVersion(code, '^0.8.4')

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
})

assertNoWarnings(report)
assertNoErrors(report)
})

it('should NOT raise error for require with function call and custom error call', () => {
let code = funcWith(
`require(isSuccess(param1) == true && value > 10, CustomErrorEmitted(param1, param2));`
)
code = replaceSolidityVersion(code, '^0.8.4')

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
})

assertNoWarnings(report)
assertNoErrors(report)
})

it('should NOT raise error for require and mapping access with boolean value check and custom error call', () => {
let code = funcWith(
`require(users[msg.sender].isRegistered, CustomErrorEmitted(param1, param2));`
)
code = replaceSolidityVersion(code, '^0.8.4')

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
})

assertNoWarnings(report)
assertNoErrors(report)
})

it('should raise error for revert()', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '^0.8.4')
Expand All @@ -30,7 +94,7 @@ describe('Linter - gas-custom-errors', () => {
})

it('should raise error for revert([string])', () => {
let code = funcWith(`revert("Insufficent funds");`)
let code = funcWith(`revert("Insufficient funds");`)
code = replaceSolidityVersion(code, '0.8.4')

const report = linter.processStr(code, {
Expand All @@ -54,7 +118,7 @@ describe('Linter - gas-custom-errors', () => {
})

it('should NOT raise error for revert ErrorFunction() with arguments', () => {
let code = funcWith(`revert ErrorFunction({ msg: "Insufficent funds msg" });`)
let code = funcWith(`revert ErrorFunction({ msg: "Insufficient funds msg" });`)
code = replaceSolidityVersion(code, '^0.8.5')

const report = linter.processStr(code, {
Expand Down Expand Up @@ -134,7 +198,7 @@ describe('Linter - gas-custom-errors', () => {
})

it('should NOT raise error for lower versions 0.4.4', () => {
const code = funcWith(`revert("Insufficent funds");`)
const code = funcWith(`revert("Insufficient funds");`)

const report = linter.processStr(code, {
rules: { 'gas-custom-errors': 'error' },
Expand Down
Loading