Skip to content

Validate fromToken and toToken on construction #322

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

Merged
arr00 merged 4 commits into from
Mar 10, 2026
Merged

Validate fromToken and toToken on construction #322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9abdbb
Validate `fromToken` on construction
arr00 Mar 9, 2026
1c348dc
use `ERC165Checker`
arr00 Mar 9, 2026
2e579a6
check toToken as well
arr00 Mar 10, 2026
b993a03
directly validate to and from token
arr00 Mar 10, 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
15 changes: 14 additions & 1 deletion contracts/finance/BatcherConfidential.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ pragma solidity ^0.8.27;
import {FHE, externalEuint64, euint64, ebool, euint128} from "@fhevm/solidity/lib/FHE.sol";
import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
import {ReentrancyGuardTransient} from "@openzeppelin/contracts/utils/ReentrancyGuardTransient.sol";
import {IERC7984Receiver} from "./../interfaces/IERC7984Receiver.sol";
import {ERC7984ERC20Wrapper} from "./../token/ERC7984/extensions/ERC7984ERC20Wrapper.sol";
import {ERC7984ERC20Wrapper, IERC7984ERC20Wrapper} from "./../token/ERC7984/extensions/ERC7984ERC20Wrapper.sol";
import {FHESafeMath} from "./../utils/FHESafeMath.sol";

/**
Expand Down Expand Up @@ -97,7 +98,19 @@ abstract contract BatcherConfidential is ReentrancyGuardTransient, IERC7984Recei
/// @dev The caller is not authorized to call this function.
error Unauthorized();

/// @dev The given `token` does not support `IERC7984ERC20Wrapper` via `ERC165`.
error InvalidWrapperToken(address token);

constructor(ERC7984ERC20Wrapper fromToken_, ERC7984ERC20Wrapper toToken_) {
require(
ERC165Checker.supportsInterface(address(fromToken_), type(IERC7984ERC20Wrapper).interfaceId),
InvalidWrapperToken(address(fromToken_))
);
require(
ERC165Checker.supportsInterface(address(toToken_), type(IERC7984ERC20Wrapper).interfaceId),
InvalidWrapperToken(address(toToken_))
);

Comment on lines 104 to +113
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 9, 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:

# Find and examine the target file
fd -t f "BatcherConfidential.sol" | head -5

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 133

🏁 Script executed:

# Check repository structure and dependencies
ls -la | grep -E "(package.json|foundry.toml|remappings.txt|hardhat.config)"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 358

🏁 Script executed:

# Look for imports of ERC165Checker or ERC165 in the project
rg -i "ERC165Checker|ERC165" --type solidity | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 124

🏁 Script executed:

# Read the target contract around lines 103-105
cat -n contracts/finance/BatcherConfidential.sol | sed -n '95,115p'

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1197

🏁 Script executed:

# Check package.json for OpenZeppelin contracts dependency
cat package.json | grep -A 5 "@openzeppelin"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 659

🏁 Script executed:

# Search for ERC165 usage in the codebase
rg "ERC165" contracts/

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1665

🏁 Script executed:

# Check if ERC165Checker exists in the local openzeppelin-contracts
find lib/openzeppelin-contracts -name "*ERC165Checker*" -type f

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Read ERC165Checker implementation to understand its behavior
find lib/openzeppelin-contracts -name "ERC165Checker.sol" -exec cat -n {} \;

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Check the IERC165 interface to understand supportsInterface behavior
rg -A 10 "interface IERC165" lib/openzeppelin-contracts

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Check if toToken_ has similar validation or if there are tests for this
rg -B 5 -A 10 "supportsInterface" contracts/finance/BatcherConfidential.sol

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 782

🏁 Script executed:

# Search more broadly for ERC165Checker in openzeppelin-contracts
rg "ERC165Checker" lib/openzeppelin-contracts --type sol

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 119

🏁 Script executed:

# List the contents of the introspection directory
ls -la lib/openzeppelin-contracts/contracts/utils/introspection/ 2>/dev/null || echo "Directory not found"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 111

🏁 Script executed:

# Check the actual import statement to see what version/source is being used
head -30 contracts/finance/BatcherConfidential.sol | cat -n

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 2432

🏁 Script executed:

# Verify the type of fromToken_ parameter and how it's used elsewhere
rg "ERC7984ERC20Wrapper" contracts/finance/BatcherConfidential.sol

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 961

🏁 Script executed:

# Check if lib/openzeppelin-contracts directory exists and explore its structure
ls -la lib/ | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 358

🏁 Script executed:

# Look for ERC165Checker more broadly
find lib -name "*.sol" -type f 2>/dev/null | xargs grep -l "ERC165Checker" 2>/dev/null | head -5

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Check the openzeppelin-contracts version being used
cat lib/openzeppelin-contracts/package.json 2>/dev/null | grep -A 2 '"version"' || cat lib/openzeppelin-contracts/package.json 2>/dev/null | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Let's understand the actual behavior: check if there are any tests for BatcherConfidential
find . -name "*test*" -o -name "*spec*" | grep -i batcher

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 134

🏁 Script executed:

# Check what IERC7984ERC20Wrapper actually is
cat -n contracts/token/ERC7984/extensions/ERC7984ERC20Wrapper.sol | head -50

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 2852

🏁 Script executed:

# Check the test file to understand how BatcherConfidential is tested
cat -n test/finance/BatcherConfidential.test.ts | head -100

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 4039

🏁 Script executed:

# Let's check what's in lib/openzeppelin-contracts directly
ls -la lib/openzeppelin-contracts/ | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 198

🏁 Script executed:

# Search for ERC165Checker in the entire lib directory more broadly
find lib -type f -name "*.sol" 2>/dev/null | xargs grep -l "library ERC165Checker" 2>/dev/null

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 76

🏁 Script executed:

# Check the IERC165 standard to understand supportsInterface behavior
cat lib/openzeppelin-contracts/IERC165.sol 2>/dev/null || rg -A 5 "interface IERC165" lib/ 2>/dev/null | head -30

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Search web resources or documentation about ERC165Checker behavior
echo "Searching for ERC165Checker in OpenZeppelin v5 contracts"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 148

🏁 Script executed:

# Check node_modules for OpenZeppelin contracts
ls -la node_modules/@openzeppelin/contracts/utils/introspection/ 2>/dev/null | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Look for ERC165Checker in node_modules
find node_modules/@openzeppelin -name "ERC165Checker.sol" 2>/dev/null

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Check if tests have any coverage for invalid fromToken scenarios
grep -n "InvalidFromToken\|revert\|expect.*throw" test/finance/BatcherConfidential.test.ts

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 990

🏁 Script executed:

# Understand the actual runtime behavior: check what happens when you call a missing function
# Let's look at the IERC7984ERC20Wrapper interface to understand what's being checked
cat -n contracts/interfaces/IERC7984ERC20Wrapper.sol | head -50

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1458

🏁 Script executed:

# Check the actual test that triggers InvalidFromToken
grep -B 10 -A 5 "InvalidFromToken" test/finance/BatcherConfidential.test.ts | head -40

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 687

🏁 Script executed:

# Look for how the batcher is deployed in the test with invalid tokens
grep -B 5 -A 15 "deployContract.*BatcherConfidentialSwapMock" test/finance/BatcherConfidential.test.ts

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 2113

🏁 Script executed:

# Check if there are any tests that pass an invalid fromToken (non-ERC165)
grep -C 10 "expect.*InvalidFromToken\|should.*revert.*InvalidFromToken" test/finance/BatcherConfidential.test.ts

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Understand the behavioral difference: check if direct supportsInterface calls vs safe calls differ
# Let's look at the ERC7984.sol implementation to see how supportsInterface is implemented
cat -n contracts/token/ERC7984/ERC7984.sol | grep -A 10 "supportsInterface"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 639

🏁 Script executed:

# Check remappings to understand the OpenZeppelin import paths
cat remappings.txt

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 154

🏁 Script executed:

# Check if ERC165Checker is available in OpenZeppelin v5
web_search "ERC165Checker OpenZeppelin Contracts v5"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 142

🏁 Script executed:

# Let's look at what contracts are imported from OpenZeppelin to understand the version
grep -r "@openzeppelin/contracts" contracts/ | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 2573

🏁 Script executed:

# Check the package.json to see if there's version pinning for `@openzeppelin/contracts`
cat package.json | grep -A 10 "dependencies"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Let's verify the actual behavior: check if the test with ERC7984Mock (non-wrapper) truly expects InvalidFromToken
grep -B 5 -A 15 "should reject invalid fromToken" test/finance/BatcherConfidential.test.ts

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 804

🏁 Script executed:

# Check the lib/openzeppelin-contracts directory structure to understand what's available
find lib/openzeppelin-contracts/contracts/utils -type d | head -20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 169

🏁 Script executed:

# Look for introspection utilities in the available lib
find lib/openzeppelin-contracts/contracts/utils -name "*165*" -o -name "*introspection*"

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 169

🏁 Script executed:

# Let's check the package.json version info more carefully
cat lib/openzeppelin-contracts/package.json 2>/dev/null | grep -E '"version"|"name"' | head -5

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🏁 Script executed:

# Check what utilities are in the introspection folder
ls -la lib/openzeppelin-contracts/contracts/utils/introspection/ 2>/dev/null || echo "Checking alternative path" && find lib/openzeppelin-contracts -type f -path "*introspection*" 2>/dev/null

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 117

🏁 Script executed:

# Search for any safe interface checking patterns already used in the codebase
rg "supportsInterface|AddressChecker|ERC165" contracts/ --max-count 20

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 2815

🏁 Script executed:

# Let's check if the lib was properly set up or if it's a stub
ls -la lib/openzeppelin-contracts/ 2>/dev/null

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 198

🏁 Script executed:

# Check the actual contract that's being deployed in tests to understand the inheritance
grep -r "ERC7984Mock" contracts/mocks/ | head -5

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 676

🏁 Script executed:

# Look at what ERC7984Mock actually implements
cat contracts/mocks/token/ERC7984Mock.sol

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 3259

🏁 Script executed:

# Verify the test scenario more carefully - what happens when you pass a non-ERC165 contract?
# Search for how the test validates the error
grep -B 20 "should reject invalid fromToken" test/finance/BatcherConfidential.test.ts | tail -30

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 626

🏁 Script executed:

# Check if there are any alternative implementations or workarounds for ERC165Checker in this project
find contracts -name "*.sol" | xargs grep -l "staticcall\|try.*supportsInterface" 2>/dev/null

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 150

🏁 Script executed:

# Let's verify the actual behavior: check if the test is expecting the revert to be due to supportsInterface call
# or due to the interface check returning false
cat -n test/finance/BatcherConfidential.test.ts | grep -B 30 "should reject invalid fromToken" | tail -40

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1016

🏁 Script executed:

# Understand what happens when an ERC7984Mock is passed - check if it implements IERC7984ERC20Wrapper interface
cat contracts/mocks/token/ERC7984ERC20WrapperMock.sol | head -30

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1264

🏁 Script executed:

# Check if toToken_ also needs validation
grep -n "toToken_" contracts/finance/BatcherConfidential.sol | head -10

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 208

🏁 Script executed:

# Verify the exact error scenario: try to understand if passing a plain ERC20 would fail at the supportsInterface call
# Let's check if there are any tests with plain ERC20Mock
grep -n "ERC20Mock\|ZeroAddress" test/finance/BatcherConfidential.test.ts | head -10

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 617

🏁 Script executed:

# Let's verify the actual behavior by checking the IERC7984ERC20Wrapper interface
# and understanding what supportsInterface should return for ERC7984Mock
grep -A 5 "type(IERC7984ERC20Wrapper).interfaceId" contracts/

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 124

🏁 Script executed:

# Check what interface ID IERC7984ERC20Wrapper declares
cat contracts/interfaces/IERC7984ERC20Wrapper.sol

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1220

🏁 Script executed:

# Verify the inheritance chain and supportsInterface implementation in ERC7984
cat -n contracts/token/ERC7984/ERC7984.sol | sed -n '1,80p'

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 4040

🏁 Script executed:

# Check if toToken_ should also have the same validation
grep -C 10 "toToken_" contracts/finance/BatcherConfidential.sol | head -30

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 1217

🏁 Script executed:

# Final check: search for any web resources about ERC165Checker in OpenZeppelin
# Try a web search capability

Repository: OpenZeppelin/openzeppelin-confidential-contracts

Length of output: 74

🌐 Web query:

ERC165Checker OpenZeppelin Contracts documentation

💡 Result:

ERC165Checker is an OpenZeppelin Contracts library for querying whether an address (contract) supports an interface via ERC-165 (IERC165). It returns true/false (it does not revert if unsupported). [1]

Docs (v5.x API – Utilities → Introspection / ERC165Checker): [1]

Import:

import "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";

[1]

Key functions:

  • supportsERC165(address account) -> bool
  • supportsInterface(address account, bytes4 interfaceId) -> bool
  • getSupportedInterfaces(address account, bytes4[] interfaceIds) -> bool[]
  • supportsAllInterfaces(address account, bytes4[] interfaceIds) -> bool
  • supportsERC165InterfaceUnchecked(address account, bytes4 interfaceId) -> bool
    [1]

Use an ERC165-safe probe for fromToken_ and consider toToken_ validation.

Line 104 calls supportsInterface directly on an arbitrary constructor argument. If fromToken_ is a plain ERC20 or non-ERC165 contract, that call reverts before require can emit InvalidFromToken(), leaving a gap in error handling.

Suggested fix 
+import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
 import {ERC7984ERC20Wrapper, IERC7984ERC20Wrapper} from "./../token/ERC7984/extensions/ERC7984ERC20Wrapper.sol";

 constructor(ERC7984ERC20Wrapper fromToken_, ERC7984ERC20Wrapper toToken_) {
-    require(fromToken_.supportsInterface(type(IERC7984ERC20Wrapper).interfaceId), InvalidFromToken());
+    require(
+        ERC165Checker.supportsInterface(address(fromToken_), type(IERC7984ERC20Wrapper).interfaceId),
+        InvalidFromToken()
+    );

     _fromToken = fromToken_;
     _toToken = toToken_;

Tests currently only validate rejection of ERC7984Mock (which supports ERC165). Add regression tests with plain $ERC20Mock and ethers.ZeroAddress to ensure safe handling of non-ERC165 inputs. Also verify whether toToken_ requires similar validation.

📝 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
constructor(ERC7984ERC20Wrapper fromToken_, ERC7984ERC20Wrapper toToken_) {
require(fromToken_.supportsInterface(type(IERC7984ERC20Wrapper).interfaceId), InvalidFromToken());
import {ERC165Checker} from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";
import {ERC7984ERC20Wrapper, IERC7984ERC20Wrapper} from "./../token/ERC7984/extensions/ERC7984ERC20Wrapper.sol";
constructor(ERC7984ERC20Wrapper fromToken_, ERC7984ERC20Wrapper toToken_) {
require(
ERC165Checker.supportsInterface(address(fromToken_), type(IERC7984ERC20Wrapper).interfaceId),
InvalidFromToken()
);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/finance/BatcherConfidential.sol` around lines 103 - 105, The
constructor currently calls fromToken_.supportsInterface(...) directly which can
revert for non-ERC165 contracts; update the constructor in BatcherConfidential
to probe ERC165 safely (e.g. use ERC165Checker.supportsInterface or perform a
low-level/staticcall to supportsInterface and check the returned bool) before
requiring IERC7984ERC20Wrapper support and emit InvalidFromToken() on failure;
apply analogous validation for toToken_ if it should also implement
IERC7984ERC20Wrapper; add regression tests that pass a plain ERC20Mock and
address(0) to the constructor to assert safe rejections (and retain the existing
ERC7984Mock rejection test).

_fromToken = fromToken_;
_toToken = toToken_;
_currentBatchId = 1;
Expand Down
30 changes: 30 additions & 0 deletions test/finance/BatcherConfidential.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,36 @@ describe('BatcherConfidential', function () {
});
});

it('should reject invalid fromToken', async function () {
const confidentialToken = await ethers.deployContract('$ERC7984Mock', ['Mock Token', 'MTK', 'URI']);

await expect(
ethers.deployContract('$BatcherConfidentialSwapMock', [
confidentialToken,
this.toToken,
this.exchange,
this.operator,
]),
)
.to.be.revertedWithCustomError(this.batcher, 'InvalidWrapperToken')
.withArgs(confidentialToken.target);
});

it('should reject invalid toToken', async function () {
const confidentialToken = await ethers.deployContract('$ERC7984Mock', ['Mock Token', 'MTK', 'URI']);

await expect(
ethers.deployContract('$BatcherConfidentialSwapMock', [
this.fromToken,
confidentialToken,
this.exchange,
this.operator,
]),
)
.to.be.revertedWithCustomError(this.batcher, 'InvalidWrapperToken')
.withArgs(confidentialToken.target);
});

for (const viaCallback of [true, false]) {
describe(`join ${viaCallback ? 'via callback' : 'directly'}`, async function () {
const join = async function (
Expand Down
Loading