diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index e9b7ecad..79c9a8ee 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -25,6 +25,7 @@ module.exports = Object.freeze({ }, ], 'constructor-syntax': 'warn', + 'gas-indexed-events': 'warn', 'gas-multitoken1155': 'warn', 'gas-small-strings': 'warn', 'comprehensive-interface': 'warn', diff --git a/docs/rules.md b/docs/rules.md index b0458c23..c12bdf54 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -27,10 +27,11 @@ title: "Rule Index of Solhint" ## Gas Consumption Rules -| Rule Id | Error | Recommended | Deprecated | -| ------------------------------------------------------------------- | --------------------------------------------------- | ----------- | ---------- | -| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | | -| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md) | Keep strings smaller than 32 bytes | | | +| Rule Id | Error | Recommended | Deprecated | +| ------------------------------------------------------------------- | -------------------------------------------------------------- | ----------- | ---------- | +| [gas-indexed-events](./rules/gas-consumption/gas-indexed-events.md) | Suggest indexed arguments on events for uint, bool and address | | | +| [gas-multitoken1155](./rules/gas-consumption/gas-multitoken1155.md) | ERC1155 is a cheaper non-fungible token than ERC721 | | | +| [gas-small-strings](./rules/gas-consumption/gas-small-strings.md) | Keep strings smaller than 32 bytes | | | ## Miscellaneous diff --git a/docs/rules/gas-consumption/gas-indexed-events.md b/docs/rules/gas-consumption/gas-indexed-events.md new file mode 100644 index 00000000..62dd17dc --- /dev/null +++ b/docs/rules/gas-consumption/gas-indexed-events.md @@ -0,0 +1,38 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "gas-indexed-events | Solhint" +--- + +# gas-indexed-events +![Category Badge](https://img.shields.io/badge/-Gas%20Consumption%20Rules-informational) +![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) + +## Description +Suggest indexed arguments on events for uint, bool and address + +## Options +This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. + +### Example Config +```json +{ + "rules": { + "gas-indexed-events": "warn" + } +} +``` + +### Notes +- [source](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiciative (see Indexed Events) + +## Examples +This rule does not have examples. + +## Version +This rule is introduced in the latest version. + +## Resources +- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/gas-consumption/gas-indexed-events.js) +- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-indexed-events.md) +- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-indexed-events.js) diff --git a/lib/rules/gas-consumption/gas-indexed-events.js b/lib/rules/gas-consumption/gas-indexed-events.js new file mode 100644 index 00000000..ca07d54a --- /dev/null +++ b/lib/rules/gas-consumption/gas-indexed-events.js @@ -0,0 +1,68 @@ +const BaseChecker = require('../base-checker') + +const ruleId = 'gas-indexed-events' +const suggestForTypes = ['uint', 'int', 'bool', 'ufixed', 'fixed', 'address'] +const meta = { + type: 'gas-consumption', + + docs: { + description: 'Suggest indexed arguments on events for uint, bool and address', + category: 'Gas Consumption Rules', + notes: [ + { + note: '[source](https://coinsbench.com/comprehensive-guide-tips-and-tricks-for-gas-optimization-in-solidity-5380db734404) of the rule initiciative (see Indexed Events)', + }, + ], + }, + + isDefault: false, + recommended: false, + defaultSetup: 'warn', + + schema: null, +} + +class GasIndexedEvents extends BaseChecker { + constructor(reporter) { + super(reporter, ruleId, meta) + } + + EventDefinition(node) { + const qtyArguments = node.parameters.length + const positionsOfPossibleSuggestions = [] + let qtyOfIndexedKetwords = 0 + let argumentType = '' + + // first check if the indexed keyword is used three times to left room for suggestion + for (let i = 0; i < qtyArguments; i++) { + // put the type in the variable for better code reading + argumentType = node.parameters[i].typeName.name + + // compare if the type is one of the possible suggestion types + for (const type of suggestForTypes) { + if (argumentType.startsWith(type)) { + // push the position into an array to come back later if there's room for another indexed argument + positionsOfPossibleSuggestions.push(i) + } + } + + // count the indexed arguments + if (node.parameters[i].isIndexed) qtyOfIndexedKetwords++ + } + + // if there's room for more indexed arguments + if (qtyOfIndexedKetwords < 3 && positionsOfPossibleSuggestions.length > 0) { + this.reportError(node, positionsOfPossibleSuggestions) + } + } + + reportError(node, positionsOfPossibleSuggestions) { + let parameterName = '' + for (let i = 0; i < positionsOfPossibleSuggestions.length; i++) { + parameterName = node.parameters[positionsOfPossibleSuggestions[i]].name + this.error(node, `GC: [${parameterName}] on Event [${node.name}] could be Indexed`) + } + } +} + +module.exports = GasIndexedEvents diff --git a/lib/rules/gas-consumption/index.js b/lib/rules/gas-consumption/index.js index 804838e2..89ea5c7b 100644 --- a/lib/rules/gas-consumption/index.js +++ b/lib/rules/gas-consumption/index.js @@ -1,12 +1,12 @@ const GasMultitoken1155 = require('./gas-multitoken1155') const GasSmallStrings = require('./gas-small-strings') -// const GasIndexedEvents = require('./gas-indexed-events') +const GasIndexedEvents = require('./gas-indexed-events') // module.exports = function checkers(reporter, config, tokens) { module.exports = function checkers(reporter, config) { return [ new GasMultitoken1155(reporter, config), new GasSmallStrings(reporter, config), - // new GasIndexedEvents(reporter, config), + new GasIndexedEvents(reporter, config), ] } diff --git a/test/rules/gas-consumption/gas-indexed-events.js b/test/rules/gas-consumption/gas-indexed-events.js new file mode 100644 index 00000000..20be78e8 --- /dev/null +++ b/test/rules/gas-consumption/gas-indexed-events.js @@ -0,0 +1,82 @@ +const assert = require('assert') +const linter = require('../../../lib/index') +const { contractWith } = require('../../common/contract-builder') + +const replaceErrorMsg = (variableName, eventName) => { + return `GC: [${variableName}] on Event [${eventName}] could be Indexed` +} + +describe('Linter - gas-indexed-events', () => { + it('should raise error on amount and address', () => { + const code = contractWith( + 'event LogEvent1(string message, bytes whatever, uint128 amount, address account);' + ) + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 2) + assert.equal(report.messages[0].message, replaceErrorMsg('amount', 'LogEvent1')) + assert.equal(report.messages[1].message, replaceErrorMsg('account', 'LogEvent1')) + }) + + it('should raise error on account', () => { + const code = contractWith('event LogEvent2(ufixed account);') + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, replaceErrorMsg('account', 'LogEvent2')) + }) + + it('should raise error on amount and account', () => { + const code = contractWith( + 'event LogEvent4(string indexed message, bytes indexed whatever, bool active, address account);' + ) + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 2) + assert.equal(report.messages[0].message, replaceErrorMsg('active', 'LogEvent4')) + assert.equal(report.messages[1].message, replaceErrorMsg('account', 'LogEvent4')) + }) + + it('should NOT raise error, all three indexed keyword are used', () => { + const code = contractWith( + 'event LogEvent3(string indexed message, bytes indexed whatever, uint256 indexed amount, address account);' + ) + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should NOT raise error, all types are not for suggestion', () => { + const code = contractWith( + 'event LogEvent3(string indexed message, bytes whatever, string message2);' + ) + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should NOT raise error, there are no arguments', () => { + const code = contractWith('event LogEvent5();') + + const report = linter.processStr(code, { + rules: { 'gas-indexed-events': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) +})