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: payable fallback #561

Merged
merged 1 commit into from
Mar 12, 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
2 changes: 1 addition & 1 deletion conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = Object.freeze({
'code-complexity': ['warn', 7],
'explicit-types': ['warn', 'explicit'],
'function-max-lines': ['warn', 50],
'interface-starts-with-i': 'warning',
'interface-starts-with-i': 'warn',
'max-line-length': ['error', 120],
'max-states-count': ['warn', 15],
'no-console': 'error',
Expand Down
26 changes: 13 additions & 13 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ title: "Rule Index of Solhint"
| [no-unused-import](./rules/best-practises/no-unused-import.md) | Imported object name is not being used by the contract. | $~~~~~~~~$✔️ | |
| [no-unused-vars](./rules/best-practises/no-unused-vars.md) | Variable "name" is unused. | $~~~~~~~~$✔️ | |
| [one-contract-per-file](./rules/best-practises/one-contract-per-file.md) | Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names) | $~~~~~~~~$✔️ | |
| [payable-fallback](./rules/best-practises/payable-fallback.md) | When fallback is not payable you will not be able to receive ethers. | $~~~~~~~~$✔️ | |
| [payable-fallback](./rules/best-practises/payable-fallback.md) | When fallback is not payable and there is no receive function you will not be able to receive currency. | $~~~~~~~~$✔️ | |
| [reason-string](./rules/best-practises/reason-string.md) | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | $~~~~~~~~$✔️ | |
| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword. | | |

Expand Down Expand Up @@ -50,18 +50,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 incrementation 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-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 | | |
| [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 | | |
| 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 incrementation 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
29 changes: 14 additions & 15 deletions docs/rules/best-practises/payable-fallback.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ title: "payable-fallback | Solhint"


## Description
When fallback is not payable you will not be able to receive ethers.
When fallback is not payable and there is no receive function you will not be able to receive currency.

## Options
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
Expand All @@ -28,36 +28,35 @@ This rule accepts a string option of rule severity. Must be one of "error", "war

### Notes
- Solhint allows this rule to automatically fix the code with `--fix` option
- Instead of having a fallback function to receive native currency it is recommended to code a receive() function [[here]](https://docs.soliditylang.org/en/v0.8.24/contracts.html#fallback-function)

## Examples
### 👍 Examples of **correct** code for this rule

#### Fallback is payable

```solidity
function() public payable {}
```

#### Fallback is payable

pragma solidity 0.4.4;


contract A {
function () public payable {}
}

```solidity
fallback() external payable {}
```

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

#### Fallback is not payable

```solidity
function() {} function g() payable {}
```

#### Fallback is not payable

pragma solidity 0.4.4;


contract A {
function () public {}
}

```solidity
fallback() {} function g() payable {}
```

## Version
Expand Down
6 changes: 3 additions & 3 deletions docs/rules/naming/interface-starts-with-i.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,19 @@ title: "interface-starts-with-i | Solhint"

# interface-starts-with-i
![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
![Default Severity Badge warning](https://img.shields.io/badge/Default%20Severity-warning-undefined)
![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)

## Description
Solidity Interfaces names should start with an `I`

## Options
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warning.
This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.

### Example Config
```json
{
"rules": {
"interface-starts-with-i": "warning"
"interface-starts-with-i": "warn"
}
}
```
Expand Down
58 changes: 37 additions & 21 deletions e2e/08-autofix/payable-fallback/Foo1.sol
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;

contract Generic {

constructor() {}
contract FallbackNoReceive1 {
constructor() {}

function anyFunction() {}

//// fallback with receive
receive() public {}

receive() external onlyOwner {}

receive() external virtual {
uint256 anUintToFillSpace;
}

//// fallback with no name
function() public {}

function() {
Expand All @@ -29,22 +18,49 @@ contract Generic {
uint256 anUintToFillSpace;
}


//// fallback explicit
fallback() {}
fallback() external {}

fallback() {
fallback() external {
uint256 anUintToFillSpace;
}

fallback() external onlyOwner{
fallback() external onlyOwner {
uint256 anUintToFillSpace;
}

fallback() virtual {}

fallback() external virtual {}

fallback() external payable {}
function() external payable {}
receive() public virtual payable {}
function() external payable {}
}

contract FallbackWithReceive {
constructor() {}

function() {
uint256 anUintToFillSpace;
}

function() external onlyOwner {}

fallback() external {
uint256 anUintToFillSpace;
}

receive() external payable onlyOwner {}
}

contract FallbackNoReceive2 {
constructor() {}

function() {
uint256 anUintToFillSpace;
}

function() external onlyOwner {}

fallback() external {
uint256 anUintToFillSpace;
}
}
58 changes: 37 additions & 21 deletions e2e/08-autofix/payable-fallback/Foo1AfterFix.sol
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;

contract Generic {

constructor() {}
contract FallbackNoReceive1 {
constructor() {}

function anyFunction() {}

//// fallback with receive
receive() payable public {}

receive() payable external onlyOwner {}

receive() payable external virtual {
uint256 anUintToFillSpace;
}

//// fallback with no name
function() payable public {}

function() payable {
Expand All @@ -29,22 +18,49 @@ contract Generic {
uint256 anUintToFillSpace;
}


//// fallback explicit
fallback() payable {}
fallback() payable external {}

fallback() payable {
fallback() payable external {
uint256 anUintToFillSpace;
}

fallback() payable external onlyOwner{
fallback() payable external onlyOwner {
uint256 anUintToFillSpace;
}

fallback() payable virtual {}

fallback() payable external virtual {}

fallback() external payable {}
function() external payable {}
receive() public virtual payable {}
function() external payable {}
}

contract FallbackWithReceive {
constructor() {}

function() {
uint256 anUintToFillSpace;
}

function() external onlyOwner {}

fallback() external {
uint256 anUintToFillSpace;
}

receive() external payable onlyOwner {}
}

contract FallbackNoReceive2 {
constructor() {}

function() payable {
uint256 anUintToFillSpace;
}

function() payable external onlyOwner {}

fallback() payable external {
uint256 anUintToFillSpace;
}
}
58 changes: 37 additions & 21 deletions e2e/08-autofix/payable-fallback/Foo1BeforeFix.sol
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;

contract Generic {

constructor() {}
contract FallbackNoReceive1 {
constructor() {}

function anyFunction() {}

//// fallback with receive
receive() public {}

receive() external onlyOwner {}

receive() external virtual {
uint256 anUintToFillSpace;
}

//// fallback with no name
function() public {}

function() {
Expand All @@ -29,22 +18,49 @@ contract Generic {
uint256 anUintToFillSpace;
}


//// fallback explicit
fallback() {}
fallback() external {}

fallback() {
fallback() external {
uint256 anUintToFillSpace;
}

fallback() external onlyOwner{
fallback() external onlyOwner {
uint256 anUintToFillSpace;
}

fallback() virtual {}

fallback() external virtual {}

fallback() external payable {}
function() external payable {}
receive() public virtual payable {}
function() external payable {}
}

contract FallbackWithReceive {
constructor() {}

function() {
uint256 anUintToFillSpace;
}

function() external onlyOwner {}

fallback() external {
uint256 anUintToFillSpace;
}

receive() external payable onlyOwner {}
}

contract FallbackNoReceive2 {
constructor() {}

function() {
uint256 anUintToFillSpace;
}

function() external onlyOwner {}

fallback() external {
uint256 anUintToFillSpace;
}
}
2 changes: 1 addition & 1 deletion lib/rules/best-practises/interface-starts-with-i.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const meta = {
},
isDefault: false,
recommended: false,
defaultSetup: 'warning',
defaultSetup: 'warn',
schema: [],
}

Expand Down
Loading
Loading