Skip to content

feat: Implement SafeERC20 for secure ERC20 token transfers #69

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.

Sign up for GitHub

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

Open
Nihallllll wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: Implement SafeERC20 for secure ERC20 token transfers #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
929e7b5
feat: implement SafeERC20 for secure token transfers
Nihallllll Jan 15, 2026
bba3e63
Merge branch 'main' into feat/implement-safeerc20
Nihallllll Mar 25, 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
3 changes: 3 additions & 0 deletions .gitmodules
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@
[submodule "contracts/lib/chainlink-brownie-contracts"]
path = contracts/lib/chainlink-brownie-contracts
url = https://github.com/smartcontractkit/chainlink-brownie-contracts
[submodule "contracts/lib/openzeppelin-contracts"]
path = contracts/lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
1 change: 0 additions & 1 deletion contracts/.env.example
View file
Open in desktop

This file was deleted.

6 changes: 6 additions & 0 deletions contracts/foundry.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
{
"lib/forge-std": {
"rev": "3b20d60d14b343ee4f908cb8079495c07f5e8981"
},
"lib\\openzeppelin-contracts": {
"tag": {
"name": "v5.5.0",
"rev": "fcbae5394ae8ad52d8e580a3477db99814b9d565"
}
}
}
3 changes: 2 additions & 1 deletion contracts/foundry.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
src = "src"
out = "out"
libs = ["lib"]
solc_version = '0.8.13'
solc_version = '0.8.20'
remappings = ['@openzeppelin/=lib/openzeppelin-contracts/']
# See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options
# forge create --rpc-url https://eth-sepolia.g.alchemy.com/v2/nnY0qPUQLYsUvb5BKJM5bh81sI6O0PQG --private-key fba7342ef6879df2c735644c734ea69c140f423d84eb2d53fbdfd53fd5d7c586 src/Token.sol:MyToken --legacy
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Remove or redact the private key from the comment.

Line 8 contains what appears to be a private key in a comment. Even if this is a test/example key, hardcoding private keys in version-controlled files is a serious security anti-pattern and can lead to accidental exposure of real credentials. Remove this line or replace the key with a placeholder like YOUR_PRIVATE_KEY.

Proposed fix 
-# forge create --rpc-url https://eth-sepolia.g.alchemy.com/v2/nnY0qPUQLYsUvb5BKJM5bh81sI6O0PQG   --private-key fba7342ef6879df2c735644c734ea69c140f423d84eb2d53fbdfd53fd5d7c586  src/Token.sol:MyToken --legacy
+# forge create --rpc-url <RPC_URL> --private-key <PRIVATE_KEY> src/Token.sol:MyToken --legacy
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# forge create --rpc-url https://eth-sepolia.g.alchemy.com/v2/nnY0qPUQLYsUvb5BKJM5bh81sI6O0PQG --private-key fba7342ef6879df2c735644c734ea69c140f423d84eb2d53fbdfd53fd5d7c586 src/Token.sol:MyToken --legacy
# forge create --rpc-url <RPC_URL> --private-key <PRIVATE_KEY> src/Token.sol:MyToken --legacy
🤖 Prompt for AI Agents 
In `@contracts/foundry.toml` at line 8, The comment in contracts/foundry.toml
contains a hardcoded private key in the forge create command; remove or redact
that secret by deleting the key or replacing it with a placeholder such as
YOUR_PRIVATE_KEY (and optionally reference an env var like PRIVATE_KEY), and
update the example command to instruct using environment variables or a secrets
manager rather than embedding keys in comments (look for the forge create
command line in the file and replace the literal key).

1 change: 1 addition & 0 deletions contracts/lib/openzeppelin-contracts
Open in desktop
Submodule openzeppelin-contracts added at fcbae5
31 changes: 9 additions & 22 deletions contracts/src/Chainvoice.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
// SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.13;
pragma solidity ^0.8.20;


import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

interface IERC20 {
function transferFrom(address sender, address recipient, uint256 amount) external returns (bool);
function balanceOf(address account) external view returns (uint256);
function allowance(address owner, address spender) external view returns (uint256);
}

contract Chainvoice {
// ========== Errors ==========
// Existing
using SafeERC20 for IERC20;
// Errors
error MixedTokenBatch();
error InvalidBatchSize();
error AlreadySettled();
Expand Down Expand Up @@ -103,16 +102,6 @@ contract Chainvoice {
// Constants
uint256 public constant MAX_BATCH = 50;

// ========== Internal Utils ==========
function _isERC20(address token) internal view returns (bool) {
if (token == address(0)) return false;
if (token.code.length == 0) return false;
(bool success, ) = token.staticcall(
abi.encodeWithSignature("balanceOf(address)", address(this))
);
return success;
}

// ========== Single-invoice create ==========
function createInvoice(
address to,
Expand Down Expand Up @@ -258,12 +247,11 @@ contract Chainvoice {
}
accumulatedFees += fee;

bool transferSuccess = IERC20(invoice.tokenAddress).transferFrom(
IERC20(invoice.tokenAddress).safeTransferFrom(
msg.sender,
invoice.from,
invoice.amountDue
);
if (!transferSuccess) revert TokenTransferFailed();
}

emit InvoicePaid(
Expand Down Expand Up @@ -331,8 +319,7 @@ contract Chainvoice {

for (uint256 i = 0; i < n; i++) {
InvoiceDetails storage inv = invoices[invoiceIds[i]];
bool ok = erc20.transferFrom(msg.sender, inv.from, inv.amountDue);
if (!ok) revert TokenTransferFailed();
erc20.safeTransferFrom(msg.sender, inv.from, inv.amountDue);
emit InvoicePaid(inv.id, inv.from, inv.to, inv.amountDue, token);
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/test/Chainvoice.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.13;
pragma solidity ^0.8.20;
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Solidity tests touching invoice payment paths ==="
rg -n -C2 --type sol \
  -g '**/*.t.sol' \
  -g '**/test/**/*.sol' \
  -g '**/tests/**/*.sol' \
  'createInvoice\(|createInvoicesBatch\(|payInvoice\(|payInvoicesBatch\(|InsufficientAllowance|IncorrectNativeValue|FeeMustBeNative|ERC20|IERC20|Mock.*Token'

Repository: StabilityNexus/Chainvoice

Length of output: 167

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Searching for Solidity test files ==="
find . -name "*.t.sol" -o -name "*Test.sol" | head -20

echo ""
echo "=== Searching for payInvoice and token-related keywords in test files ==="
rg -n -C2 'payInvoice|createInvoice|InsufficientAllowance|MockToken|ERC20|IERC20' \
  --glob='**/*.t.sol' \
  --glob='**/test/**/*.sol'

echo ""
echo "=== Checking for any ERC20 mock files ==="
find . -type f -name "*Mock*.sol" -o -name "*Token*.sol" | grep -E "(test|spec)" | head -10

Repository: StabilityNexus/Chainvoice

Length of output: 2948

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Looking for the main Chainvoice contract ==="
find . -name "Chainvoice.sol" -not -path "*/test/*"

echo ""
echo "=== Checking contract structure (first 100 lines) ==="
find . -name "Chainvoice.sol" -not -path "*/test/*" -exec head -100 {} \;

echo ""
echo "=== Searching for ERC20 imports and token handling ==="
find . -name "Chainvoice.sol" -not -path "*/test/*" -exec grep -n "ERC20\|IERC20\|token\|Token" {} +

echo ""
echo "=== Searching for payInvoicesBatch function ==="
find . -name "*.sol" -not -path "*/test/*" -exec grep -n "payInvoicesBatch\|createInvoicesBatch" {} +

Repository: StabilityNexus/Chainvoice

Length of output: 7073

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Viewing payInvoice function (around line 220-265) ==="
sed -n '220,265p' ./contracts/src/Chainvoice.sol

echo ""
echo "=== Viewing payInvoicesBatch function (around line 267-335) ==="
sed -n '267,335p' ./contracts/src/Chainvoice.sol

Repository: StabilityNexus/Chainvoice

Length of output: 4776

Add comprehensive test coverage for ERC20 payment flows and batch operations.

The payInvoice and payInvoicesBatch functions now handle token-denominated invoices with allowance checks and transfers, but the test suite covers only native (ETH) payments. This security-sensitive logic lacks coverage for:

  • ERC20 payment success paths (both single and batch)
  • InsufficientAllowance revert condition
  • Token transfer failures via safeTransferFrom
  • payInvoicesBatch operations entirely (native or token)

Add test cases covering successful token payments plus all revert paths, or point to existing coverage if present elsewhere.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/test/Chainvoice.t.sol` at line 2, Add tests in Chainvoice.t.sol to
cover ERC20-denominated flows: write a test that mints/approves a mock ERC20 and
calls payInvoice to succeed (verifying token balances and invoice paid state), a
batch success test that calls payInvoicesBatch with multiple token invoices and
verifies all transfers, a test that sets insufficient allowance and expects the
InsufficientAllowance revert from payInvoice/payInvoicesBatch, and a test where
the mock ERC20's transferFrom is forced to revert (or return false) to simulate
transfer failure and assert the call reverts; use the payInvoice,
payInvoicesBatch functions, the InsufficientAllowance error identifier, and a
controllable mock ERC20 (overriding safeTransferFrom/transferFrom behavior) to
trigger success and failure paths.


import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
Expand Down