diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index f3b95779..e9b7ecad 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -26,6 +26,7 @@ module.exports = Object.freeze({ ], 'constructor-syntax': 'warn', 'gas-multitoken1155': 'warn', + 'gas-small-strings': 'warn', 'comprehensive-interface': 'warn', quotes: ['error', 'double'], 'const-name-snakecase': 'warn', diff --git a/docs/rules.md b/docs/rules.md index 707f45f3..b0458c23 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -30,6 +30,7 @@ title: "Rule Index of Solhint" | 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 | | | ## Miscellaneous diff --git a/docs/rules/gas-consumption/gas-multitoken1155.md b/docs/rules/gas-consumption/gas-multitoken1155.md index 7b0b6b95..535cd0b4 100644 --- a/docs/rules/gas-consumption/gas-multitoken1155.md +++ b/docs/rules/gas-consumption/gas-multitoken1155.md @@ -24,7 +24,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ``` ### Notes -- [source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b&utm_campaign=42ccb5d8-c2cc-4416-b661-8eec8368f72b&utm_source=so&utm_medium=mail&utm_content=40a3d3be-d07d-479e-af1d-6b2ef1b950da&cid=9619984a-b43c-4002-ba71-820fd72bb83a#viewer-8v8t9) of the rule initiciative +- [source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-8v8t9) of the rule initiciative ## Examples This rule does not have examples. diff --git a/docs/rules/gas-consumption/gas-small-strings.md b/docs/rules/gas-consumption/gas-small-strings.md new file mode 100644 index 00000000..2121d299 --- /dev/null +++ b/docs/rules/gas-consumption/gas-small-strings.md @@ -0,0 +1,38 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "gas-small-strings | Solhint" +--- + +# gas-small-strings +![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 +Keep strings smaller than 32 bytes + +## 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-small-strings": "warn" + } +} +``` + +### Notes +- [source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-ck1vq) of the rule initiciative + +## 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-small-strings.js) +- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/gas-consumption/gas-small-strings.md) +- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/gas-consumption/gas-small-strings.js) diff --git a/lib/rules/gas-consumption/gas-multitoken1155.js b/lib/rules/gas-consumption/gas-multitoken1155.js index bd1410b9..e137e661 100644 --- a/lib/rules/gas-consumption/gas-multitoken1155.js +++ b/lib/rules/gas-consumption/gas-multitoken1155.js @@ -9,7 +9,7 @@ const meta = { category: 'Gas Consumption Rules', notes: [ { - note: '[source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b&utm_campaign=42ccb5d8-c2cc-4416-b661-8eec8368f72b&utm_source=so&utm_medium=mail&utm_content=40a3d3be-d07d-479e-af1d-6b2ef1b950da&cid=9619984a-b43c-4002-ba71-820fd72bb83a#viewer-8v8t9) of the rule initiciative', + note: '[source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-8v8t9) of the rule initiciative', }, ], }, diff --git a/lib/rules/gas-consumption/gas-small-strings.js b/lib/rules/gas-consumption/gas-small-strings.js new file mode 100644 index 00000000..0fc24a38 --- /dev/null +++ b/lib/rules/gas-consumption/gas-small-strings.js @@ -0,0 +1,159 @@ +const BaseChecker = require('../base-checker') + +const ruleId = 'gas-small-strings' +const meta = { + type: 'gas-consumption', + + docs: { + description: 'Keep strings smaller than 32 bytes', + category: 'Gas Consumption Rules', + notes: [ + { + note: '[source](https://www.rareskills.io/post/gas-optimization?postId=c9db474a-ff97-4fa3-a51d-fe13ccb8fe3b#viewer-ck1vq) of the rule initiciative', + }, + ], + }, + + isDefault: false, + recommended: false, + defaultSetup: 'warn', + + schema: null, +} + +class GasSmallStrings extends BaseChecker { + constructor(reporter) { + super(reporter, ruleId, meta) + } + + FileLevelConstant(node) { + let isLarger = false + + if (node.typeName.name === 'string') { + isLarger = this.isStringLargerThan32Bytes(node.initialValue.value) + } + + if (isLarger) { + this.reportError(node) + } + } + + VariableDeclaration(node) { + let isLarger = false + + if (node.parent.type === 'StateVariableDeclaration' && node.typeName.name === 'string') { + isLarger = this.isStringLargerThan32Bytes(node.expression.value) + } + + if ( + node.parent.type === 'StateVariableDeclaration' && + node.typeName.type === 'ArrayTypeName' && + node.typeName.baseTypeName.name === 'string' + ) { + let isComponentLarger = false + const componentsSize = node.expression.components.length + + let i = 0 + while (i < componentsSize) { + isComponentLarger = this.isStringLargerThan32Bytes(node.expression.components[i].value) + i++ + if (isComponentLarger) { + // update the result + isLarger = true + // if there is one larger this loop can be exited + i = componentsSize + 1 + } + } + } + + if (node.parent.type === 'VariableDeclarationStatement' && node.typeName.name === 'string') { + isLarger = this.isStringLargerThan32Bytes(node.parent.initialValue.value) + } + + if (isLarger) { + this.reportError(node) + } + } + + BinaryOperation(node) { + let isLarger = false + + if (node.right.type === 'StringLiteral') { + isLarger = this.isStringLargerThan32Bytes(node.right.value) + } + + if (isLarger) { + this.reportError(node) + } + } + + FunctionCall(node) { + let isLarger = false + let isArgumentLarger = false + const argumentsSize = node.arguments.length + + let i = 0 + while (i < argumentsSize) { + if (node.arguments[i].type === 'StringLiteral') { + isArgumentLarger = this.isStringLargerThan32Bytes(node.arguments[i].value) + if (isArgumentLarger) { + // update the result + isLarger = true + // if there is one larger this loop can be exited + i = argumentsSize + } + } + i++ + } + + if (isLarger) { + this.reportError(node) + } + } + + ReturnStatement(node) { + let isLarger = false + let isComponentLarger = false + const componentsSize = node.expression.components.length + + let i = 0 + while (i < componentsSize) { + if ( + node.expression.components[i].type === 'StringLiteral' + // && + // node.parent.parent.returnParameters[i].typeName.name === 'string' + ) { + isComponentLarger = this.isStringLargerThan32Bytes(node.expression.components[i].value) + if (isComponentLarger) { + // update the result + isLarger = true + // if there is one larger this loop can be exited + i = componentsSize + } + } + i++ + } + + if (isLarger) { + this.reportError(node) + } + } + + isStringLargerThan32Bytes(inputString) { + if (inputString.startsWith('0x') && inputString.length === 42) { + // Check if the input is a valid Ethereum address + return false + } else { + // Convert the string to Buffer and get its byte length + const byteLength = Buffer.from(inputString, 'utf8').length + // Check if the byte length is greater than 32 + return byteLength > 32 + } + } + + reportError(node) { + this.error(node, 'GC: String exceeds 32 bytes') + } +} + +module.exports = GasSmallStrings diff --git a/lib/rules/gas-consumption/index.js b/lib/rules/gas-consumption/index.js index c117a725..804838e2 100644 --- a/lib/rules/gas-consumption/index.js +++ b/lib/rules/gas-consumption/index.js @@ -1,6 +1,12 @@ const GasMultitoken1155 = require('./gas-multitoken1155') +const GasSmallStrings = require('./gas-small-strings') +// const GasIndexedEvents = require('./gas-indexed-events') // module.exports = function checkers(reporter, config, tokens) { module.exports = function checkers(reporter, config) { - return [new GasMultitoken1155(reporter, config)] + return [ + new GasMultitoken1155(reporter, config), + new GasSmallStrings(reporter, config), + // new GasIndexedEvents(reporter, config), + ] } diff --git a/test/rules/gas-consumption/gas-small-strings.js b/test/rules/gas-consumption/gas-small-strings.js new file mode 100644 index 00000000..7a23d9ca --- /dev/null +++ b/test/rules/gas-consumption/gas-small-strings.js @@ -0,0 +1,322 @@ +const assert = require('assert') +const linter = require('../../../lib/index') + +const ERROR_MSG = 'GC: String exceeds 32 bytes' + +describe('Linter - gas-small-strings', () => { + it('should raise error on file level constant', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + import '@openzeppelin/asdadada/adadadadada/adadadadadad/adadadadad/adadadadadad/adadadadad/adadadadad/contracts/token/ERC721.sol'; + + string constant myStringConstantFile2 = 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 1.'; + + contract A {} + ` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on import nor address longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + import '@openzeppelin/asdadada/adadadadada/adadadadadad/adadadadad/adadadadadad/adadadadad/adadadadad/contracts/token/ERC721.sol'; + + address constant ownerConstantFile1 = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB000xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB000xA0b86991c6218b36c1d19D4a2e9Eb0cE3606'; + + contract A {} + ` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on state variable longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + string myStringState2 = + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 2.'; + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on state variable smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + string myStringState1 = 'Hello, World 3'; + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on state variable (array) with one element longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + string[] public myArrayState2 = [ + 'One', + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 4a', + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 4b' + ]; + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on state variable (array) with all elements smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + string[] public myArrayState1 = ['One', 'Two', 'Three']; + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on constructor variable longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + constructor() { + string myStringConstructor1 = 'Initialized in constructor'; + string myStringConstructor1 = 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 7'; + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on constructor variable smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + constructor() { + string myStringConstructor1 = 'Initialized in constructor'; + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on assignation of variable longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + myStringArray[0] = '1234'; + myStringArray[1] = + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 8'; + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error assignation of variable smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + myStringArray[0] = '1234'; + myStringArray[1] = '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB00'; + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on event with msg longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + emit LogEvent('Event message'); + emit LogEvent( + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 9' + ); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on event with msg smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + emit LogEvent('Event message'); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on require with msg longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + require(myBytesArray[0] == myBytesArray[1], 'Error message'); + require( + myBytesArray[0] == myBytesArray[1], + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 10' + ); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on require with msg smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external { + require(myBytesArray[0] == myBytesArray[1], 'Error message'); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + + it('should raise error on retrurn with at least one element longer than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external returns (address, string memory, string memory, uint256) { + return ( + '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB00', + 'This is a test string larger than 32 bytes! It contains more characters to ensure it exceeds the specified size 13', + 'Returned String2', + 1 + ); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 1) + assert.equal(report.messages[0].message, ERROR_MSG) + }) + + it('should NOT raise error on return with all elements smaller than 32 bytes', () => { + const code = ` + // SPDX-License-Identifier: Apache-2.0 + pragma solidity ^0.8.4; + + contract A { + function function1() external returns (address, string memory, string memory, uint256) { + return ( + '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB00', + 'Returned String1', + 'Returned String2', + 1 + ); + } + }` + + const report = linter.processStr(code, { + rules: { 'gas-small-strings': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) +})