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 indexed events and gas custom errors #573

Merged
merged 4 commits into from
Apr 10, 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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## [4.5.3] - 2024-04-10
### Fixed
- `gas-custom-errors` improved logic to ranged pragma versions [#568](https://github.com/protofire/solhint/pull/568)
- `gas-indexed-events` [#568](https://github.com/protofire/solhint/pull/568)


## [4.5.2] - 2024-03-15

Expand Down
2 changes: 1 addition & 1 deletion docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
FROM node:20-alpine
LABEL maintainer="[email protected]"
ENV VERSION=4.5.2
ENV VERSION=4.5.3

RUN npm install -g solhint@"$VERSION"
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-calldata-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-calldata-parameters.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-custom-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ revert("Insufficient Balance");
```

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-custom-errors.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-increment-by-one.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-increment-by-one.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-indexed-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-indexed-events.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-length-in-loops.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-length-in-loops.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-multitoken1155.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-multitoken1155.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-named-return-values.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ function checkBalance(address wallet) external view returns(uint256) {}
```

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-named-return-values.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-small-strings.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-small-strings.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-strict-inequalities.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-strict-inequalities.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/gas-consumption/gas-struct-packing.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
This rule does not have examples.

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-struct-packing.js)
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/naming/interface-starts-with-i.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ interface Foo { function foo () external; }
```

## Version
This rule is introduced in the latest version.
This rule was introduced in [Solhint 4.5.0](https://github.com/protofire/solhint/tree/v4.5.0)

## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/interface-starts-with-i.js)
Expand Down
32 changes: 4 additions & 28 deletions lib/rules/gas-consumption/gas-custom-errors.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
const semver = require('semver')
const BaseChecker = require('../base-checker')
const { severityDescription } = require('../../doc/utils')

const DEFAULT_SEVERITY = 'warn'
const BASE_VERSION = [0, 8, 4]
const BASE_VERSION = '>=0.8.4'
const ruleId = 'gas-custom-errors'
const meta = {
type: 'gas-consumption',
Expand Down Expand Up @@ -62,33 +63,8 @@ class GasCustomErrorsChecker extends BaseChecker {
this.solidityVersion = 0
}

parseVersion(version) {
// Remove any leading ^ or ~ and other non-numeric characters before the first digit
const cleanVersion = version.replace(/^[^\d]+/, '')
return cleanVersion.split('.').map((num) => parseInt(num, 10))
}

isVersionGreater(node) {
const currentVersion = this.parseVersion(this.solidityVersion)
if (currentVersion.length !== 3) {
this.error(node, `GC: Invalid Solidity version`)
return
}

for (let i = 0; i < BASE_VERSION.length; i++) {
if (currentVersion[i] < BASE_VERSION[i]) {
// If any part of the current version is less than the base version, return false
return false
} else if (currentVersion[i] > BASE_VERSION[i]) {
// If any part of the current version is greater, no need to check further, return true
return true
}
// If parts are equal, continue to the next part
}

// Reaching here means all parts compared are equal, or the base version parts have been exhausted,
// so the current version is at least equal, return true
return true
isVersionGreater() {
return semver.intersects(this.solidityVersion, BASE_VERSION)
}

PragmaDirective(node) {
Expand Down
2 changes: 1 addition & 1 deletion lib/rules/gas-consumption/gas-indexed-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class GasIndexedEvents extends BaseChecker {

// compare if the type is one of the possible suggestion types
for (const type of suggestForTypes) {
if (argumentType.startsWith(type)) {
if (argumentType.startsWith(type) && !node.parameters[i].isIndexed) {
// push the position into an array to come back later if there's room for another indexed argument
positionsOfPossibleSuggestions.push(i)
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "solhint",
"version": "4.5.2",
"version": "4.5.3",
"description": "Solidity Code Linter",
"main": "lib/index.js",
"keywords": [
Expand Down
60 changes: 58 additions & 2 deletions test/rules/gas-consumption/gas-custom-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe('Linter - gas-custom-errors', () => {
assertErrorCount(report, 0)
})

it('should NOT raise error for lower versions 0.8.3', () => {
it('should raise error for higher versions 0.8', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '0.8')

Expand All @@ -152,6 +152,62 @@ describe('Linter - gas-custom-errors', () => {
})

assertErrorCount(report, 1)
assertErrorMessage(report, 'GC: Invalid Solidity version')
assert.equal(report.reports[0].message, 'GC: Use Custom Errors instead of revert statements')
})

it('should raise error for higher versions 0.9', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '0.9')

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

assertErrorCount(report, 1)
assert.equal(report.reports[0].message, 'GC: Use Custom Errors instead of revert statements')
})

it('should NOT raise error for lower versions 0.7', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '0.7')

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

assertErrorCount(report, 0)
})

it('should NOT raise error for range versions lower than 0.8.4', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '>= 0.8.1 <= 0.8.3')

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

assertErrorCount(report, 0)
})

it('should raise error for range versions higher than 0.8.4', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '> 0.8.4 <= 0.9')

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

assertErrorCount(report, 1)
})

it('should raise error for range versions containing 0.8.4', () => {
let code = funcWith(`revert();`)
code = replaceSolidityVersion(code, '> 0.8.1 <= 0.8.6')

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

assertErrorCount(report, 1)
})
})
20 changes: 20 additions & 0 deletions test/rules/gas-consumption/gas-indexed-events.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,24 @@ describe('Linter - gas-indexed-events', () => {

assert.equal(report.errorCount, 0)
})

it('should NOT raise error, all variables are indexed', () => {
const code = contractWith('event Increment (address indexed sender, uint256 indexed value);')

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

assert.equal(report.errorCount, 0)
})

it('should NOT raise error, all variables are indexed or not for suggestion', () => {
const code = contractWith('event Increment (address indexed sender, string whatever);')

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

assert.equal(report.errorCount, 0)
})
})
Loading