Skip to content

Commit

Permalink
Fix unused error false positives (#398)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexroan authored May 6, 2024
1 parent 499669a commit 0fdb644
Show file tree
Hide file tree
Showing 31 changed files with 82 additions and 37 deletions.
6 changes: 5 additions & 1 deletion aderyn_core/src/detect/low/useless_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ impl IssueDetector for UselessErrorDetector {
// extract the ids directly from the expression of the function call
if let Expression::Identifier(identifier) = &*revert_stmt.error_call.expression {
used_errors.insert(identifier.referenced_declaration);
} else if let Expression::MemberAccess(member_access) =
&*revert_stmt.error_call.expression
{
used_errors.insert(member_access.referenced_declaration.unwrap());
}
}

Expand Down Expand Up @@ -73,7 +77,7 @@ mod useless_error_tests {
let found = detector.detect(&context).unwrap();
assert!(found);
// Assert that the detector returns the correct number of instances
assert_eq!(detector.instances().len(), 1);
assert_eq!(detector.instances().len(), 2);
// Assert that the detector returns the correct severity
assert_eq!(
detector.severity(),
Expand Down
16 changes: 11 additions & 5 deletions judgeops/current/report.judge.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| Key | Value |
| --- | --- |
| .sol Files | 36 |
| Total nSLOC | 1002 |
| Total nSLOC | 1013 |


## Files Details
Expand Down Expand Up @@ -75,7 +75,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/T11sTranferer.sol | 8 |
| src/UnprotectedInitialize.sol | 25 |
| src/UnsafeERC721Mint.sol | 18 |
| src/UnusedError.sol | 8 |
| src/UnusedError.sol | 19 |
| src/WrongOrderOfLayout.sol | 13 |
| src/ZeroAddressCheck.sol | 41 |
| src/cloc/AnotherHeavilyCommentedContract.sol | 32 |
Expand All @@ -90,7 +90,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/parent_chain/ParentChainContract.sol | 29 |
| src/uniswap/UniswapV2Swapper.sol | 50 |
| src/uniswap/UniswapV3Swapper.sol | 150 |
| **Total** | **1002** |
| **Total** | **1013** |


## Issue Summary
Expand Down Expand Up @@ -1425,10 +1425,16 @@ it is recommended that the definition be removed when custom error is unused

### Responsible : useless-error

- Found in src/UnusedError.sol [Line: 7](../../tests/contract-playground/src/UnusedError.sol#L7)
- Found in src/UnusedError.sol [Line: 5](../../tests/contract-playground/src/UnusedError.sol#L5)

```solidity
error CannotRenounceWhilePaused(address account);
error UnusedLibraryError();
```

- Found in src/UnusedError.sol [Line: 13](../../tests/contract-playground/src/UnusedError.sol#L13)

```solidity
error UnusedError1(address account);
```

- Found in src/WrongOrderOfLayout.sol [Line: 13](../../tests/contract-playground/src/WrongOrderOfLayout.sol#L13)
Expand Down
13 changes: 9 additions & 4 deletions report.json
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
},
{
"file_path": "src/UnusedError.sol",
"n_sloc": 8
"n_sloc": 19
},
{
"file_path": "src/WrongOrderOfLayout.sol",
Expand Down Expand Up @@ -179,7 +179,7 @@
]
},
"files_summary": {
"total_sloc": 1002,
"total_sloc": 1013,
"total_source_units": 36
},
"high_issues": {
Expand Down Expand Up @@ -1253,8 +1253,13 @@
"instances": [
{
"contract_path": "src/UnusedError.sol",
"line_no": 7,
"src": "120:49"
"line_no": 5,
"src": "84:27"
},
{
"contract_path": "src/UnusedError.sol",
"line_no": 13,
"src": "258:36"
},
{
"contract_path": "src/WrongOrderOfLayout.sol",
Expand Down
16 changes: 11 additions & 5 deletions report.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| Key | Value |
| --- | --- |
| .sol Files | 36 |
| Total nSLOC | 1002 |
| Total nSLOC | 1013 |


## Files Details
Expand Down Expand Up @@ -75,7 +75,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/T11sTranferer.sol | 8 |
| src/UnprotectedInitialize.sol | 25 |
| src/UnsafeERC721Mint.sol | 18 |
| src/UnusedError.sol | 8 |
| src/UnusedError.sol | 19 |
| src/WrongOrderOfLayout.sol | 13 |
| src/ZeroAddressCheck.sol | 41 |
| src/cloc/AnotherHeavilyCommentedContract.sol | 32 |
Expand All @@ -90,7 +90,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/parent_chain/ParentChainContract.sol | 29 |
| src/uniswap/UniswapV2Swapper.sol | 50 |
| src/uniswap/UniswapV3Swapper.sol | 150 |
| **Total** | **1002** |
| **Total** | **1013** |


## Issue Summary
Expand Down Expand Up @@ -1313,10 +1313,16 @@ Consider keeping the naming convention consistent in a given contract

it is recommended that the definition be removed when custom error is unused

- Found in src/UnusedError.sol [Line: 7](tests/contract-playground/src/UnusedError.sol#L7)
- Found in src/UnusedError.sol [Line: 5](tests/contract-playground/src/UnusedError.sol#L5)

```solidity
error CannotRenounceWhilePaused(address account);
error UnusedLibraryError();
```

- Found in src/UnusedError.sol [Line: 13](tests/contract-playground/src/UnusedError.sol#L13)

```solidity
error UnusedError1(address account);
```

- Found in src/WrongOrderOfLayout.sol [Line: 13](tests/contract-playground/src/WrongOrderOfLayout.sol#L13)
Expand Down
16 changes: 11 additions & 5 deletions report.sample_profile.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| Key | Value |
| --- | --- |
| .sol Files | 36 |
| Total nSLOC | 1002 |
| Total nSLOC | 1013 |


## Files Details
Expand Down Expand Up @@ -75,7 +75,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/T11sTranferer.sol | 8 |
| src/UnprotectedInitialize.sol | 25 |
| src/UnsafeERC721Mint.sol | 18 |
| src/UnusedError.sol | 8 |
| src/UnusedError.sol | 19 |
| src/WrongOrderOfLayout.sol | 13 |
| src/ZeroAddressCheck.sol | 41 |
| src/cloc/AnotherHeavilyCommentedContract.sol | 32 |
Expand All @@ -90,7 +90,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| src/parent_chain/ParentChainContract.sol | 29 |
| src/uniswap/UniswapV2Swapper.sol | 50 |
| src/uniswap/UniswapV3Swapper.sol | 150 |
| **Total** | **1002** |
| **Total** | **1013** |


## Issue Summary
Expand Down Expand Up @@ -1313,10 +1313,16 @@ Consider keeping the naming convention consistent in a given contract

it is recommended that the definition be removed when custom error is unused

- Found in src/UnusedError.sol [Line: 7](tests/contract-playground/src/UnusedError.sol#L7)
- Found in src/UnusedError.sol [Line: 5](tests/contract-playground/src/UnusedError.sol#L5)

```solidity
error CannotRenounceWhilePaused(address account);
error UnusedLibraryError();
```

- Found in src/UnusedError.sol [Line: 13](tests/contract-playground/src/UnusedError.sol#L13)

```solidity
error UnusedError1(address account);
```

- Found in src/WrongOrderOfLayout.sol [Line: 13](tests/contract-playground/src/WrongOrderOfLayout.sol#L13)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"abi":[{"type":"error","name":"LibraryError","inputs":[]},{"type":"error","name":"UnusedLibraryError","inputs":[]}],"bytecode":{"object":"0x602d6037600b82828239805160001a607314602a57634e487b7160e01b600052600060045260246000fd5b30600052607381538281f3fe73000000000000000000000000000000000000000030146080604052600080fdfea164736f6c6343000813000a","sourceMap":"57:82:15:-:0;;;;;;;;;;;;;;;-1:-1:-1;;;57:82:15;;;;;;;;;;;;;;;;;","linkReferences":{}},"deployedBytecode":{"object":"0x73000000000000000000000000000000000000000030146080604052600080fdfea164736f6c6343000813000a","sourceMap":"57:82:15:-:0;;;;;;;;","linkReferences":{}},"methodIdentifiers":{},"rawMetadata":"{\"compiler\":{\"version\":\"0.8.19+commit.7dd6d404\"},\"language\":\"Solidity\",\"output\":{\"abi\":[{\"inputs\":[],\"name\":\"LibraryError\",\"type\":\"error\"},{\"inputs\":[],\"name\":\"UnusedLibraryError\",\"type\":\"error\"}],\"devdoc\":{\"kind\":\"dev\",\"methods\":{},\"version\":1},\"userdoc\":{\"kind\":\"user\",\"methods\":{},\"version\":1}},\"settings\":{\"compilationTarget\":{\"src/UnusedError.sol\":\"ErrorLibrary\"},\"evmVersion\":\"paris\",\"libraries\":{},\"metadata\":{\"bytecodeHash\":\"none\"},\"optimizer\":{\"enabled\":true,\"runs\":200},\"remappings\":[\":ds-test/=lib/forge-std/lib/ds-test/src/\",\":erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/\",\":forge-std/=lib/forge-std/src/\",\":openzeppelin-contracts/=lib/openzeppelin-contracts/\",\":openzeppelin/=lib/openzeppelin-contracts/contracts/\",\":solmate/=lib/solmate/src/\",\":v2-periphery/=lib/v2-periphery/contracts/\"]},\"sources\":{\"src/UnusedError.sol\":{\"keccak256\":\"0xa7728a2db61a840047367ba9b9d3133d68928b7cdc34f9b12d4d5d036249e19f\",\"license\":\"MIT\",\"urls\":[\"bzz-raw://159441553b7969f2ee3ef70b77a285f0f95ddb83b30df331d832536ab174bc54\",\"dweb:/ipfs/QmPQbyfUSjarFes16S1Jkymwh5rETSp9HdeihcxNfZY5q3\"]}},\"version\":1}","metadata":{"compiler":{"version":"0.8.19+commit.7dd6d404"},"language":"Solidity","output":{"abi":[{"inputs":[],"type":"error","name":"LibraryError"},{"inputs":[],"type":"error","name":"UnusedLibraryError"}],"devdoc":{"kind":"dev","methods":{},"version":1},"userdoc":{"kind":"user","methods":{},"version":1}},"settings":{"remappings":["ds-test/=lib/forge-std/lib/ds-test/src/","erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/","forge-std/=lib/forge-std/src/","openzeppelin-contracts/=lib/openzeppelin-contracts/","openzeppelin/=lib/openzeppelin-contracts/contracts/","solmate/=lib/solmate/src/","v2-periphery/=lib/v2-periphery/contracts/"],"optimizer":{"enabled":true,"runs":200},"metadata":{"bytecodeHash":"none"},"compilationTarget":{"src/UnusedError.sol":"ErrorLibrary"},"evmVersion":"paris","libraries":{}},"sources":{"src/UnusedError.sol":{"keccak256":"0xa7728a2db61a840047367ba9b9d3133d68928b7cdc34f9b12d4d5d036249e19f","urls":["bzz-raw://159441553b7969f2ee3ef70b77a285f0f95ddb83b30df331d832536ab174bc54","dweb:/ipfs/QmPQbyfUSjarFes16S1Jkymwh5rETSp9HdeihcxNfZY5q3"],"license":"MIT"}},"version":1},"ast":{"absolutePath":"src/UnusedError.sol","id":1770,"exportedSymbols":{"ErrorLibrary":[1732],"UnusedError":[1769]},"nodeType":"SourceUnit","src":"32:524:15","nodes":[{"id":1727,"nodeType":"PragmaDirective","src":"32:23:15","nodes":[],"literals":["solidity","0.8",".19"]},{"id":1732,"nodeType":"ContractDefinition","src":"57:82:15","nodes":[{"id":1729,"nodeType":"ErrorDefinition","src":"84:27:15","nodes":[],"errorSelector":"1d810984","name":"UnusedLibraryError","nameLocation":"90:18:15","parameters":{"id":1728,"nodeType":"ParameterList","parameters":[],"src":"108:2:15"}},{"id":1731,"nodeType":"ErrorDefinition","src":"116:21:15","nodes":[],"errorSelector":"161f7228","name":"LibraryError","nameLocation":"122:12:15","parameters":{"id":1730,"nodeType":"ParameterList","parameters":[],"src":"134:2:15"}}],"abstract":false,"baseContracts":[],"canonicalName":"ErrorLibrary","contractDependencies":[],"contractKind":"library","fullyImplemented":true,"linearizedBaseContracts":[1732],"name":"ErrorLibrary","nameLocation":"65:12:15","scope":1770,"usedErrors":[1729,1731]},{"id":1769,"nodeType":"ContractDefinition","src":"141:414:15","nodes":[{"id":1735,"nodeType":"VariableDeclaration","src":"168:25:15","nodes":[],"constant":false,"functionSelector":"8381f58a","mutability":"mutable","name":"number","nameLocation":"183:6:15","scope":1769,"stateVariable":true,"storageLocation":"default","typeDescriptions":{"typeIdentifier":"t_uint256","typeString":"uint256"},"typeName":{"id":1733,"name":"uint256","nodeType":"ElementaryTypeName","src":"168:7:15","typeDescriptions":{"typeIdentifier":"t_uint256","typeString":"uint256"}},"value":{"hexValue":"30","id":1734,"isConstant":false,"isLValue":false,"isPure":true,"kind":"number","lValueRequested":false,"nodeType":"Literal","src":"192:1:15","typeDescriptions":{"typeIdentifier":"t_rational_0_by_1","typeString":"int_const 0"},"value":"0"},"visibility":"public"},{"id":1739,"nodeType":"ErrorDefinition","src":"204:49:15","nodes":[],"errorSelector":"83911dd5","name":"CannotRenounceWhilePaused","nameLocation":"210:25:15","parameters":{"id":1738,"nodeType":"ParameterList","parameters":[{"constant":false,"id":1737,"mutability":"mutable","name":"account","nameLocation":"244:7:15","nodeType":"VariableDeclaration","scope":1739,"src":"236:15:15","stateVariable":false,"storageLocation":"default","typeDescriptions":{"typeIdentifier":"t_address","typeString":"address"},"typeName":{"id":1736,"name":"address","nodeType":"ElementaryTypeName","src":"236:7:15","stateMutability":"nonpayable","typeDescriptions":{"typeIdentifier":"t_address","typeString":"address"}},"visibility":"internal"}],"src":"235:17:15"}},{"id":1743,"nodeType":"ErrorDefinition","src":"258:36:15","nodes":[],"errorSelector":"764a9797","name":"UnusedError1","nameLocation":"264:12:15","parameters":{"id":1742,"nodeType":"ParameterList","parameters":[{"constant":false,"id":1741,"mutability":"mutable","name":"account","nameLocation":"285:7:15","nodeType":"VariableDeclaration","scope":1743,"src":"277:15:15","stateVariable":false,"storageLocation":"default","typeDescriptions":{"typeIdentifier":"t_address","typeString":"address"},"typeName":{"id":1740,"name":"address","nodeType":"ElementaryTypeName","src":"277:7:15","stateMutability":"nonpayable","typeDescriptions":{"typeIdentifier":"t_address","typeString":"address"}},"visibility":"internal"}],"src":"276:17:15"}},{"id":1750,"nodeType":"FunctionDefinition","src":"300:52:15","nodes":[],"body":{"id":1749,"nodeType":"Block","src":"328:24:15","nodes":[],"statements":[{"expression":{"id":1747,"isConstant":false,"isLValue":false,"isPure":false,"lValueRequested":false,"nodeType":"UnaryOperation","operator":"++","prefix":false,"src":"337:8:15","subExpression":{"id":1746,"name":"number","nodeType":"Identifier","overloadedDeclarations":[],"referencedDeclaration":1735,"src":"337:6:15","typeDescriptions":{"typeIdentifier":"t_uint256","typeString":"uint256"}},"typeDescriptions":{"typeIdentifier":"t_uint256","typeString":"uint256"}},"id":1748,"nodeType":"ExpressionStatement","src":"337:8:15"}]},"functionSelector":"b147f40c","implemented":true,"kind":"function","modifiers":[],"name":"perform","nameLocation":"309:7:15","parameters":{"id":1744,"nodeType":"ParameterList","parameters":[],"src":"316:2:15"},"returnParameters":{"id":1745,"nodeType":"ParameterList","parameters":[],"src":"328:0:15"},"scope":1769,"stateMutability":"nonpayable","virtual":false,"visibility":"external"},{"id":1759,"nodeType":"FunctionDefinition","src":"358:96:15","nodes":[],"body":{"id":1758,"nodeType":"Block","src":"393:61:15","nodes":[],"statements":[{"errorCall":{"arguments":[{"expression":{"id":1754,"name":"msg","nodeType":"Identifier","overloadedDeclarations":[],"referencedDeclaration":-15,"src":"436:3:15","typeDescriptions":{"typeIdentifier":"t_magic_message","typeString":"msg"}},"id":1755,"isConstant":false,"isLValue":false,"isPure":false,"lValueRequested":false,"memberLocation":"440:6:15","memberName":"sender","nodeType":"MemberAccess","src":"436:10:15","typeDescriptions":{"typeIdentifier":"t_address","typeString":"address"}}],"expression":{"argumentTypes":[{"typeIdentifier":"t_address","typeString":"address"}],"id":1753,"name":"CannotRenounceWhilePaused","nodeType":"Identifier","overloadedDeclarations":[],"referencedDeclaration":1739,"src":"410:25:15","typeDescriptions":{"typeIdentifier":"t_function_error_pure$_t_address_$returns$__$","typeString":"function (address) pure"}},"id":1756,"isConstant":false,"isLValue":false,"isPure":false,"kind":"functionCall","lValueRequested":false,"nameLocations":[],"names":[],"nodeType":"FunctionCall","src":"410:37:15","tryCall":false,"typeDescriptions":{"typeIdentifier":"t_tuple$__$","typeString":"tuple()"}},"id":1757,"nodeType":"RevertStatement","src":"403:44:15"}]},"functionSelector":"4798aa5d","implemented":true,"kind":"function","modifiers":[],"name":"goodError","nameLocation":"367:9:15","parameters":{"id":1751,"nodeType":"ParameterList","parameters":[],"src":"376:2:15"},"returnParameters":{"id":1752,"nodeType":"ParameterList","parameters":[],"src":"393:0:15"},"scope":1769,"stateMutability":"view","virtual":false,"visibility":"external"},{"id":1768,"nodeType":"FunctionDefinition","src":"460:93:15","nodes":[],"body":{"id":1767,"nodeType":"Block","src":"502:51:15","nodes":[],"statements":[{"errorCall":{"arguments":[],"expression":{"argumentTypes":[],"expression":{"id":1762,"name":"ErrorLibrary","nodeType":"Identifier","overloadedDeclarations":[],"referencedDeclaration":1732,"src":"519:12:15","typeDescriptions":{"typeIdentifier":"t_type$_t_contract$_ErrorLibrary_$1732_$","typeString":"type(library ErrorLibrary)"}},"id":1764,"isConstant":false,"isLValue":false,"isPure":false,"lValueRequested":false,"memberLocation":"532:12:15","memberName":"LibraryError","nodeType":"MemberAccess","referencedDeclaration":1731,"src":"519:25:15","typeDescriptions":{"typeIdentifier":"t_function_error_pure$__$returns$__$","typeString":"function () pure"}},"id":1765,"isConstant":false,"isLValue":false,"isPure":false,"kind":"functionCall","lValueRequested":false,"nameLocations":[],"names":[],"nodeType":"FunctionCall","src":"519:27:15","tryCall":false,"typeDescriptions":{"typeIdentifier":"t_tuple$__$","typeString":"tuple()"}},"id":1766,"nodeType":"RevertStatement","src":"512:34:15"}]},"functionSelector":"d3670f54","implemented":true,"kind":"function","modifiers":[],"name":"goodLibraryError","nameLocation":"469:16:15","parameters":{"id":1760,"nodeType":"ParameterList","parameters":[],"src":"485:2:15"},"returnParameters":{"id":1761,"nodeType":"ParameterList","parameters":[],"src":"502:0:15"},"scope":1769,"stateMutability":"pure","virtual":false,"visibility":"external"}],"abstract":false,"baseContracts":[],"canonicalName":"UnusedError","contractDependencies":[],"contractKind":"contract","fullyImplemented":true,"linearizedBaseContracts":[1769],"name":"UnusedError","nameLocation":"150:11:15","scope":1770,"usedErrors":[1731,1739,1743]}],"license":"MIT"},"id":15}
Loading

0 comments on commit 0fdb644

Please sign in to comment.