Merge pull request #543 from protofire/gc-indexed-events

GC: Indexed events

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',

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

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)

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

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),
   ]
 }

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)
+  })
+})