Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b67d1f9
Added new GitHub action, check conventions and guidelines
purriza Feb 6, 2026
88e7e4c
Removed GitHub action to check conventions
purriza Feb 9, 2026
ef1fcfc
test: Add Solidity contract violations for CodeRabbit testing
purriza Feb 9, 2026
afb68fc
refactor: organize CodeRabbit violations by category and add deployme…
purriza Feb 10, 2026
ebf06ab
fix: move deployment script violations to script/deploy/ for CodeRabb…
purriza Feb 10, 2026
a6bc51f
fix: move Solidity violations to src/ for CodeRabbit detection
purriza Feb 10, 2026
e16776c
fix: resolve security warnings in violation examples
purriza Feb 10, 2026
1a20495
feat: add TypeScript violation examples for CodeRabbit validation
purriza Feb 10, 2026
72d67b8
fix: improve TypeScript violation examples for better CodeRabbit dete…
purriza Feb 10, 2026
f4d1c89
fix: update BadCodeQuality and BadImportOrder with improved violations
purriza Feb 10, 2026
bde5903
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza Feb 10, 2026
21b3949
Removed TypeScript violation examples. Added bash examples
purriza Feb 11, 2026
61729ac
Added Solidity violation file
purriza Feb 11, 2026
f958a82
Removed solidity violation file
purriza Feb 11, 2026
8f34c9a
Added violation test files
purriza Feb 12, 2026
1e74227
Removed scripts
purriza Feb 12, 2026
949c271
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza Feb 12, 2026
94bd7a8
Added workflow violation file
purriza Feb 12, 2026
5b3b1af
Added .sh violation files
purriza Feb 12, 2026
b6c1b02
Added .cursorrules for Coderabbit
purriza Feb 17, 2026
c0dbe42
Removed script violations and added .sol
purriza Feb 17, 2026
1ea00ba
Merge branch 'main' into SMAR-49-Create-AI-conventions-checker-git-ac…
purriza Feb 19, 2026
cf8c767
Solidity violation files moved
purriza Feb 19, 2026
fad4d82
Removed .sol and added .t.sol violations
purriza Feb 19, 2026
acc9bd9
Updated test to force violations
purriza Feb 19, 2026
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
40 changes: 40 additions & 0 deletions test/codeRabbitViolations/deploymentScripts/BadDeployPattern.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

import { DeployScriptBase } from "script/deploy/facets/utils/DeployScriptBase.sol";
import { SomeFacet } from "lifi/Facets/SomeFacet.sol";

/**
* Violation: Deployment script that does NOT use stdJson for configuration.
* Instead, it hardcodes addresses and values directly in the code.
*
* Convention violation: Deployment scripts MUST use JSON config via stdJson.
* Pattern: Use Foundry Deploy*.s.sol/Update*.s.sol with JSON config via stdJson.
*/
contract DeployScript is DeployScriptBase {
// Violation: Should use stdJson to read from config files, not hardcode values
constructor() DeployScriptBase("SomeFacet") {}

function run()
public
returns (SomeFacet deployed, bytes memory constructorArgs)
{
// Violation: Hardcoded address instead of reading from config JSON
address router = 0x1234567890123456789012345678901234567890;

// Violation: Hardcoded value instead of reading from config
uint256 someValue = 1000;

constructorArgs = abi.encode(router, someValue);

deployed = SomeFacet(deploy(type(SomeFacet).creationCode));
}

// Violation: Should override getConstructorArgs() and use stdJson to read from config
// function getConstructorArgs() internal override returns (bytes memory) {
// string memory path = string.concat(root, "/config/someFacet.json");
// string memory json = vm.readFile(path);
// address router = json.readAddress(string.concat(".", network, ".router"));
// return abi.encode(router);
// }
}
42 changes: 42 additions & 0 deletions test/codeRabbitViolations/deploymentScripts/BadNamingPattern.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

import { DeployScriptBase } from "script/deploy/facets/utils/DeployScriptBase.sol";
import { stdJson } from "forge-std/Script.sol";
import { SomeFacet } from "lifi/Facets/SomeFacet.sol";

/**
* Violation: Script file does NOT follow the naming pattern Deploy*.s.sol or Update*.s.sol.
*
* Convention violation: Deployment scripts MUST follow the pattern:
* - Deploy*.s.sol for deployment scripts
* - Update*.s.sol for update scripts
*
* This file is named BadNamingPattern.s.sol instead of DeploySomeFacet.s.sol,
* which violates the naming convention.
*/
contract DeployScript is DeployScriptBase {
using stdJson for string;

constructor() DeployScriptBase("SomeFacet") {}

function run()
public
returns (SomeFacet deployed, bytes memory constructorArgs)
{
constructorArgs = getConstructorArgs();
deployed = SomeFacet(deploy(type(SomeFacet).creationCode));
}

function getConstructorArgs() internal override returns (bytes memory) {
string memory path = string.concat(root, "/config/someFacet.json");
string memory json = vm.readFile(path);

address router = _getConfigContractAddress(
path,
string.concat(".", network, ".router")
);

return abi.encode(router);
}
}
38 changes: 38 additions & 0 deletions test/codeRabbitViolations/deploymentScripts/BadUpdateScript.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

import { UpdateScriptBase } from "script/deploy/facets/utils/UpdateScriptBase.sol";
import { stdJson } from "forge-std/StdJson.sol";

/**
* Violation: Update script that does NOT override getExcludes() even though
* it should exclude certain selectors from the diamond cut.
*
* Convention violation: Update scripts MUST override getExcludes() for selectors
* that shouldn't be in the diamond cut.
*
* Example scenario: This facet has deprecated selectors that should be excluded
* from updates, but the script doesn't override getExcludes() to exclude them.
*/
contract DeployScript is UpdateScriptBase {
using stdJson for string;

function run()
public
returns (address[] memory facets, bytes memory cutData)
{
return update("SomeFacet");
}

// Violation: Missing getExcludes() override
// Should be:
// function getExcludes() internal pure override returns (bytes4[] memory) {
// bytes4[] memory excludes = new bytes4[](2);
// excludes[0] = SomeFacet.deprecatedFunction1.selector;
// excludes[1] = SomeFacet.deprecatedFunction2.selector;
// return excludes;
// }

// This script will include ALL selectors from SomeFacet, including deprecated ones
// that should be excluded from the diamond cut.
}
57 changes: 57 additions & 0 deletions test/codeRabbitViolations/deploymentScripts/BadZkSyncSync.s.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

import { DeployScriptBase } from "script/deploy/facets/utils/DeployScriptBase.sol";
import { stdJson } from "forge-std/Script.sol";
import { SomeFacet } from "lifi/Facets/SomeFacet.sol";

/**
* Violation: Deployment script in script/deploy/ that was modified but does NOT
* have a corresponding update in script/deploy/zksync/.
*
* Convention violation: If modifying a script in script/deploy/, you MUST check
* and apply the same changes to script/deploy/zksync/.
*
* This file represents a scenario where:
* - A developer modified DeploySomeFacet.s.sol in script/deploy/facets/
* - Added new logic, changed constructor args, or updated config paths
* - But forgot to apply the same changes to script/deploy/zksync/DeploySomeFacet.zksync.s.sol
*
* This violates the ZkSync Synchronization rule.
*/
contract DeployScript is DeployScriptBase {
using stdJson for string;

constructor() DeployScriptBase("SomeFacet") {}

function run()
public
returns (SomeFacet deployed, bytes memory constructorArgs)
{
constructorArgs = getConstructorArgs();
deployed = SomeFacet(deploy(type(SomeFacet).creationCode));
}

function getConstructorArgs() internal override returns (bytes memory) {
string memory path = string.concat(root, "/config/someFacet.json");
string memory json = vm.readFile(path);

// Example: New logic added here that wasn't synced to zksync version
address router = _getConfigContractAddress(
path,
string.concat(".", network, ".router")
);

// Example: New config field added that zksync version doesn't have
address newConfigField = _getConfigContractAddress(
path,
string.concat(".", network, ".newField")
);

return abi.encode(router, newConfigField);
}

// Violation: The corresponding file script/deploy/zksync/DeploySomeFacet.zksync.s.sol
// was NOT updated with the same changes (newConfigField, etc.)
// This breaks the ZkSync Synchronization requirement.
}
23 changes: 23 additions & 0 deletions test/codeRabbitViolations/solidity/BadDRYExample.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// DRY violation: re-implements logic that already exists in LibAsset
// Should use LibAsset.transferFromNative instead of re-implementing native transfers
contract BadDRYContract {
function transferNative(address to, uint256 amount) public {
// Violation: re-implements native transfer logic that LibAsset already provides
(bool success, ) = to.call{value: amount}("");
require(success, "Transfer failed");
}

// Violation: re-implements validation that Validatable (or existing helpers) already provide
function validateAddress(address addr) public pure returns (bool) {
return addr != address(0);
// Should use Validatable or existing validation helpers instead
}

// Violation: re-implements swap logic that LibSwap already handles
function executeSwap(address token, uint256 amount) public {
// Swap logic that should delegate to LibSwap instead of being re-implemented
}
}
26 changes: 26 additions & 0 deletions test/codeRabbitViolations/solidity/BadFacet.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

import {LiFiDiamond} from "../src/LiFiDiamond.sol";

// Violation: Facet does not include the \"Facet\" suffix in its name
// Violation: Missing required internal _startBridge function
// Violation: Missing swapAndStartBridgeTokensVia{FacetName}
// Violation: Missing startBridgeTokensVia{FacetName}
// Violation: Missing nonReentrant modifier
// Violation: Missing refundExcessNative modifier
// Violation: Missing validateBridgeData modifier
contract BadBridgeFacet {
Comment thread
purriza marked this conversation as resolved.
Outdated
// Violation: receiverAddress is not the first parameter
struct BadBridgeData {
uint256 amount;
address receiverAddress; // Debería ser primero
}

// Violation: Does not use LibAsset/LibSwap/LibAllowList
// Violation: Does not delegate core logic to libraries
function bridgeTokens(BadBridgeData memory data) public {
// Inline logic instead of using shared libraries
// Violation: Does not emit LiFiTransferStarted at the end of _startBridge
}
}
9 changes: 9 additions & 0 deletions test/codeRabbitViolations/solidity/BadFilePlacement.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: File is placed in the wrong folder
// This contract should live in src/Facets/ but is intentionally placed in test/codeRabbitViolations/
// Violation: Should be generated using the standard plop templates for facets
contract MisplacedFacet {
// This is a facet-like contract but intentionally stored in the wrong location
}
16 changes: 16 additions & 0 deletions test/codeRabbitViolations/solidity/BadGasOptimization.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Inline assembly without documentation or justification
// Violation: Does not use existing optimized libraries (Solady/Solmate)
contract BadGasContract {
function transfer(address to, uint256 amount) public {
// Violation: Assembly block without explanation of why it is required
assembly {
// Assembly code without comments
}

// Violation: Re-implements logic that already exists in optimized libraries
// Should use SafeTransferLib from Solady (or similar) instead
}
}
17 changes: 17 additions & 0 deletions test/codeRabbitViolations/solidity/BadImportOrder.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Incorrect import order
// Should be: external libs → interfaces → libraries → contracts
import {SomeContract} from "../src/Facets/SomeFacet.sol" // contracts imported first (incorrect)
import {ILiFi} from "../src/Interfaces/ILiFi.sol" // interfaces imported after contracts (incorrect)
import {LibAsset} from "../src/Libraries/LibAsset.sol" // libraries imported after contracts (incorrect)
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol" // external libs imported last (incorrect)
Comment thread
purriza marked this conversation as resolved.
Outdated

contract BadImportOrder {
// Correct order should be:
// 1. @openzeppelin/contracts/...
// 2. src/Interfaces/...
// 3. src/Libraries/...
// 4. src/Facets/...
}
19 changes: 19 additions & 0 deletions test/codeRabbitViolations/solidity/BadInterface.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Interface does not use the required I* prefix
// Violation: Incorrect location - should live under src/Interfaces/
// Violation: Includes many unused functions (should only declare what is actually used)
interface BadExternalProtocol {
function function1() external;
function function2() external;
function function3() external;
function function4() external;
function function5() external;
// Many functions that are never used
}

// Violation: Mixes interface and implementation in the same file
contract BadImplementation is BadExternalProtocol {
function function1() external override {}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
21 changes: 21 additions & 0 deletions test/codeRabbitViolations/solidity/BadNonEVMFacet.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Facet with non-EVM support but incorrect validation
contract BadNonEVMFacet {
struct BadBridgeData {
bytes32 receiverAddress; // Para non-EVM
}

function startBridge(BadBridgeData memory data) public {
// Violation: Does not validate that receiverAddress != bytes32(0) for non-EVM chains
// Should revert with InvalidNonEVMReceiver() if it is zero
if (data.receiverAddress == bytes32(0)) {
// Solo requiere, debería usar error específico
require(false, "Invalid receiver");
}
Comment thread
purriza marked this conversation as resolved.
Outdated

// Violation: Does not emit BridgeToNonEVMChainBytes32 for non-EVM chains
// Should emit with transactionId, destinationChainId, and non-EVM receiver
}
}
18 changes: 18 additions & 0 deletions test/codeRabbitViolations/solidity/BadPositiveSlippage.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Facet that does not handle positive slippage correctly
contract BadSlippageFacet {
function swapAndStartBridge(uint256 amount, uint256 minAmountOut) public {
// Violation: Does not update minAmountOut after _depositAndSwap
// Should adjust minAmountOut proportionally after
// _depositAndSwap updates _bridgeData.minAmount
// See AcrossFacetV4.sol lines 137-147 for a correct reference

// Simulate _depositAndSwap updating minAmount
uint256 updatedMinAmount = amount * 105 / 100; // +5% slippage positivo

// Violation: Does not adjust bridge minAmountOut to the updatedMinAmount
bridgeProtocol.bridge(amount, minAmountOut); // Uses original minAmountOut instead of updatedMinAmount
}
Comment thread
purriza marked this conversation as resolved.
Outdated
}
25 changes: 25 additions & 0 deletions test/codeRabbitViolations/solidity/BadReceiver.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Receiver does not inherit from ILiFi and WithdrawablePeriphery
contract BadReceiver {
// Violation: executor is not immutable
address public executor;

// Violation: constructor does not validate address(0)
constructor(address _executor) {
executor = _executor;
}

// Violation: emits LiFiTransferStarted (should only be emitted in bridge facets)
function handleMessage(bytes memory data) external {
emit LiFiTransferStarted(bytes32(0), address(0), address(0), 0, 0);
}

// Violation: emits LiFiTransferCompleted (should only be emitted in Executor)
function complete() external {
emit LiFiTransferCompleted(bytes32(0), address(0), address(0), 0);
}
Comment thread
purriza marked this conversation as resolved.
Outdated

// Violation: missing receive() external payable {}
}
20 changes: 20 additions & 0 deletions test/codeRabbitViolations/solidity/BadSecurity.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.17;

// Violation: Does not validate external inputs
// Violation: Governance bypass - direct admin function without timelock/safe/timelock controller
contract BadSecurityContract {
address public owner;

// Violation: Admin function that bypasses timelock/Safe governance
function emergencyUpgrade(address newContract) public {
require(msg.sender == owner, "Not owner");
// Direct upgrade without going through governance / Safe / timelock
}

// Violation: Does not validate parameters
function setConfig(uint256 value, address target) public {
// Missing checks for address(0) or invalid values
// Does not use existing validation helpers (e.g. Validatable / library helpers)
}
}
Loading
Loading