Skip to content

Commit fdedf52

Browse files
ExefExef
committed
fix: after review fixes
1 parent 09d4144 commit fdedf52

File tree

7 files changed

+383
-73
lines changed

7 files changed

+383
-73
lines changed

‎contracts/SwapHelper/SwapHelper.sol‎

Lines changed: 142 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,127 +1,216 @@
11
// SPDX-License-Identifier: BSD-3-Clause
2-
pragma solidity 0.8.25;
2+
pragma solidity 0.8.28;
33

44
import { SafeERC20Upgradeable, IERC20Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeable.sol";
55
import { AddressUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
66
import { EIP712 } from "@openzeppelin/contracts/utils/cryptography/EIP712.sol";
77
import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
8+
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
9+
import { ReentrancyGuard } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
810

911
import { IWBNB } from "../Interfaces.sol";
1012

11-
contract SwapHelper is EIP712 {
13+
/**
14+
* @title SwapHelper
15+
* @author Venus Protocol
16+
* @notice Helper contract for executing multiple token operations atomically
17+
* @dev This contract provides utilities for wrapping native tokens, managing approvals,
18+
* and executing arbitrary calls in a single transaction. It supports optional
19+
* signature verification using EIP-712 for backend-authorized operations.
20+
* All functions except multicall are designed to be called internally via multicall.
21+
* @custom:security-contact security@venus.io
22+
*/
23+
contract SwapHelper is EIP712, Ownable, ReentrancyGuard {
1224
using SafeERC20Upgradeable for IERC20Upgradeable;
1325
using AddressUpgradeable for address;
1426

15-
uint256 internal constant REENTRANCY_LOCK_UNLOCKED = 1;
16-
uint256 internal constant REENTRANCY_LOCK_LOCKED = 2;
17-
bytes32 internal constant MULTICALL_TYPEHASH = keccak256("Multicall(bytes[] calls,uint256 deadline)");
27+
/// @notice EIP-712 typehash for Multicall struct used in signature verification
28+
/// @dev keccak256("Multicall(bytes[] calls,uint256 deadline,bytes32 salt)")
29+
bytes32 internal constant MULTICALL_TYPEHASH = keccak256("Multicall(bytes[] calls,uint256 deadline,bytes32 salt)");
1830

19-
/// @notice Wrapped native asset
31+
/// @notice Wrapped native asset contract (e.g., WBNB, WETH)
2032
IWBNB public immutable WRAPPED_NATIVE;
2133

22-
/// @notice Venus backend signer
23-
address public immutable BACKEND_SIGNER;
34+
/// @notice Address authorized to sign multicall operations
35+
/// @dev Can be updated by contract owner via setBackendSigner
36+
address public BACKEND_SIGNER;
2437

25-
/// @dev Reentrancy lock to prevent reentrancy attacks
26-
uint256 private reentrancyLock;
38+
/// @notice Mapping to track used salts for replay protection
39+
/// @dev Maps salt => bool to prevent reuse of same salt
40+
mapping(bytes32 => bool) public usedSalts;
2741

28-
/// @notice Error thrown when reentrancy is detected
29-
error Reentrancy();
30-
31-
/// @notice Error thrown when deadline is reached
42+
/// @notice Error thrown when transaction deadline has passed
43+
/// @dev Emitted when block.timestamp > deadline in multicall
3244
error DeadlineReached();
3345

34-
/// @notice Error thrown when caller is not the authorized backend signer
46+
/// @notice Error thrown when signature verification fails
47+
/// @dev Emitted when recovered signer doesn't match BACKEND_SIGNER
3548
error Unauthorized();
3649

37-
/// @notice In the locked state, allow contract to call itself, but block all external calls
38-
modifier externalLock() {
39-
bool isExternal = msg.sender != address(this);
50+
/// @notice Error thrown when zero address is provided as parameter
51+
/// @dev Used in constructor and setBackendSigner validation
52+
error ZeroAddress();
4053

41-
if (isExternal) {
42-
if (reentrancyLock == REENTRANCY_LOCK_LOCKED) revert Reentrancy();
43-
reentrancyLock = REENTRANCY_LOCK_LOCKED;
44-
}
54+
/// @notice Error thrown when salt has already been used
55+
/// @dev Prevents replay attacks by ensuring each salt is used only once
56+
error SaltAlreadyUsed();
4557

46-
_;
58+
/// @notice Event emitted when backend signer is updated
59+
/// @param oldSigner Previous backend signer address
60+
/// @param newSigner New backend signer address
61+
event BackendSignerUpdated(address indexed oldSigner, address indexed newSigner);
4762

48-
if (isExternal) reentrancyLock = REENTRANCY_LOCK_UNLOCKED;
49-
}
63+
/// @notice Event emitted when multicall is successfully executed
64+
/// @param caller Address that initiated the multicall
65+
/// @param callsCount Number of calls executed in the batch
66+
/// @param deadline Deadline timestamp used for the operation
67+
/// @param signatureVerified Whether signature verification was performed
68+
event MulticallExecuted(address indexed caller, uint256 callsCount, uint256 deadline, bool signatureVerified);
5069

5170
/// @notice Constructor
52-
/// @param wrappedNative_ Address of the wrapped native asset
53-
/// @param backendSigner_ Address of the backend signer
71+
/// @param wrappedNative_ Address of the wrapped native asset contract
72+
/// @param backendSigner_ Address authorized to sign multicall operations
73+
/// @dev Initializes EIP-712 domain with name "VenusSwap" and version "1"
74+
/// @dev Transfers ownership to msg.sender
75+
/// @dev Reverts with ZeroAddress if either parameter is address(0)
76+
/// @custom:error ZeroAddress if wrappedNative_ is address(0)
77+
/// @custom:error ZeroAddress if backendSigner_ is address(0)
5478
constructor(address wrappedNative_, address backendSigner_) EIP712("VenusSwap", "1") {
79+
if (wrappedNative_ == address(0) || backendSigner_ == address(0)) {
80+
revert ZeroAddress();
81+
}
82+
5583
WRAPPED_NATIVE = IWBNB(wrappedNative_);
5684
BACKEND_SIGNER = backendSigner_;
85+
86+
_transferOwnership(msg.sender);
5787
}
5888

5989
/// @notice Multicall function to execute multiple calls in a single transaction
60-
/// @param calls Array of calldata to execute
61-
/// @param deadline Deadline for the transaction
62-
/// @param signature Backend signature
90+
/// @param calls Array of encoded function calls to execute on this contract
91+
/// @param deadline Unix timestamp after which the transaction will revert
92+
/// @param salt Unique value to ensure this exact multicall can only be executed once
93+
/// @param signature Optional EIP-712 signature from backend signer (empty bytes to skip verification)
94+
/// @dev All calls are executed atomically - if any call fails, entire transaction reverts
95+
/// @dev Calls must be to functions on this contract (address(this))
96+
/// @dev Signature verification is only performed if signature.length != 0
97+
/// @dev Protected by nonReentrant modifier to prevent reentrancy attacks
98+
/// @dev Emits MulticallExecuted event upon successful execution
99+
/// @custom:security Only the contract itself can call wrap, sweep, approveMax, and genericCall
100+
/// @custom:error DeadlineReached if block.timestamp > deadline
101+
/// @custom:error SaltAlreadyUsed if salt has been used before
102+
/// @custom:error Unauthorized if signature verification fails
63103
function multicall(
64104
bytes[] calldata calls,
65105
uint256 deadline,
106+
bytes32 salt,
66107
bytes calldata signature
67-
) external payable externalLock {
108+
) external payable nonReentrant {
68109
if (block.timestamp > deadline) {
69110
revert DeadlineReached();
70111
}
71112

113+
if (usedSalts[salt]) {
114+
revert SaltAlreadyUsed();
115+
}
116+
usedSalts[salt] = true;
117+
118+
bool signatureVerified = false;
72119
if (signature.length != 0) {
73-
bytes32 digest = _hashMulticall(calls, deadline);
120+
bytes32 digest = _hashMulticall(calls, deadline, salt);
74121
address signer = ECDSA.recover(digest, signature);
75122
if (signer != BACKEND_SIGNER) {
76123
revert Unauthorized();
77124
}
125+
signatureVerified = true;
78126
}
79127

80128
for (uint256 i = 0; i < calls.length; i++) {
81-
address(this).functionCall(calls[i]);
129+
(bool success, bytes memory returnData) = address(this).call(calls[i]);
130+
if (!success) {
131+
assembly {
132+
revert(add(returnData, 0x20), mload(returnData))
133+
}
134+
}
82135
}
136+
137+
emit MulticallExecuted(msg.sender, calls.length, deadline, signatureVerified);
83138
}
84139

85140
/// @notice Generic call function to execute a call to an arbitrary address
86-
/// @param target Address to call
87-
/// @param data Calldata to execute
88-
function genericCall(address target, bytes calldata data) external externalLock {
141+
/// @param target Address of the contract to call
142+
/// @param data Encoded function call data
143+
/// @dev This function can interact with any external contract
144+
/// @dev Should only be called via multicall for safety
145+
/// @custom:security Use with extreme caution - can call any contract with any data
146+
/// @custom:security Ensure proper validation of target and data in off-chain systems
147+
function genericCall(address target, bytes calldata data) external payable {
89148
target.functionCall(data);
90149
}
91150

92-
/// @notice Wraps native asset into an ERC-20 token
93-
/// @param amount Amount of native asset to wrap
94-
function wrap(uint256 amount) external externalLock {
151+
/// @notice Wraps native asset into an ERC-20 wrapped token
152+
/// @param amount Amount of native asset to wrap (must match msg.value)
153+
/// @dev Calls deposit() on WRAPPED_NATIVE contract with msg.value
154+
/// @dev Wrapped tokens remain in this contract until swept
155+
/// @dev Should only be called via multicall
156+
/// @custom:security Ensure msg.value matches amount parameter
157+
function wrap(uint256 amount) external payable {
95158
WRAPPED_NATIVE.deposit{ value: amount }();
96159
}
97160

98-
/// @notice Sweeps an ERC-20 token to a specified address
99-
/// @param token ERC-20 token to sweep
100-
/// @param to Address to send the token to
101-
function sweep(IERC20Upgradeable token, address to) external externalLock {
161+
/// @notice Sweeps entire balance of an ERC-20 token to a specified address
162+
/// @param token ERC-20 token contract to sweep
163+
/// @param to Recipient address for the swept tokens
164+
/// @dev Transfers the entire balance of token held by this contract
165+
/// @dev Uses SafeERC20 for safe transfer operations
166+
/// @dev Should only be called via multicall
167+
function sweep(IERC20Upgradeable token, address to) external {
102168
token.safeTransfer(to, token.balanceOf(address(this)));
103169
}
104170

105-
/// @notice Approves the maximum amount of an ERC-20 token to a specified address
106-
/// @param token ERC-20 token to approve
107-
/// @param spender Address to approve the token to
108-
function approveMax(IERC20Upgradeable token, address spender) external externalLock {
109-
token.forceApprove(spender, 0);
171+
/// @notice Approves maximum amount of an ERC-20 token to a specified spender
172+
/// @param token ERC-20 token contract to approve
173+
/// @param spender Address to grant approval to
174+
/// @dev Sets approval to type(uint256).max for unlimited spending
175+
/// @dev Uses forceApprove to handle tokens that require 0 approval first
176+
/// @dev Should only be called via multicall
177+
/// @custom:security Grants unlimited approval - ensure spender is trusted
178+
function approveMax(IERC20Upgradeable token, address spender) external {
110179
token.forceApprove(spender, type(uint256).max);
111180
}
112181

182+
/// @notice Updates the backend signer address
183+
/// @param newSigner New backend signer address
184+
/// @dev Only callable by contract owner
185+
/// @dev Reverts with ZeroAddress if newSigner is address(0)
186+
/// @dev Emits BackendSignerUpdated event
187+
/// @custom:error ZeroAddress if newSigner is address(0)
188+
/// @custom:error Ownable: caller is not the owner (from OpenZeppelin Ownable)
189+
function setBackendSigner(address newSigner) external onlyOwner {
190+
if (newSigner == address(0)) {
191+
revert ZeroAddress();
192+
}
193+
address oldSigner = BACKEND_SIGNER;
194+
BACKEND_SIGNER = newSigner;
195+
196+
emit BackendSignerUpdated(oldSigner, newSigner);
197+
}
198+
113199
/// @notice Produces an EIP-712 digest of the multicall data
114-
/// @param calls Array of calldata to execute
115-
/// @param deadline Deadline for the transaction
116-
/// @return Digest of the multicall data
117-
function _hashMulticall(bytes[] calldata calls, uint256 deadline) internal view returns (bytes32) {
200+
/// @param calls Array of encoded function calls
201+
/// @param deadline Unix timestamp deadline
202+
/// @param salt Unique value to ensure replay protection
203+
/// @return EIP-712 typed data hash for signature verification
204+
/// @dev Hashes each call individually, then encodes with MULTICALL_TYPEHASH, deadline, and salt
205+
/// @dev Uses EIP-712 _hashTypedDataV4 for domain-separated hashing
206+
function _hashMulticall(bytes[] calldata calls, uint256 deadline, bytes32 salt) internal view returns (bytes32) {
118207
bytes32[] memory callHashes = new bytes32[](calls.length);
119208
for (uint256 i = 0; i < calls.length; i++) {
120209
callHashes[i] = keccak256(calls[i]);
121210
}
122211
return
123212
_hashTypedDataV4(
124-
keccak256(abi.encode(MULTICALL_TYPEHASH, keccak256(abi.encodePacked(callHashes)), deadline))
213+
keccak256(abi.encode(MULTICALL_TYPEHASH, keccak256(abi.encodePacked(callHashes)), deadline, salt))
125214
);
126215
}
127216
}

‎contracts/test/Imports-legacy.sol‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// SPDX-License-Identifier: BSD-3-Clause
2+
pragma solidity 0.5.16;
3+
4+
import { InterestRateModelHarness } from "@venusprotocol/venus-protocol/contracts/test/InterestRateModelHarness.sol";
5+
import { FaucetToken } from "@venusprotocol/venus-protocol/contracts/test/FaucetToken.sol";

‎contracts/test/Imports.sol‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
// SPDX-License-Identifier: BSD-3-Clause
2-
pragma solidity 0.5.16;
2+
pragma solidity 0.8.25;
33

44
import { ComptrollerMock } from "@venusprotocol/venus-protocol/contracts/test/ComptrollerMock.sol";
55
import { VBNB } from "@venusprotocol/venus-protocol/contracts/Tokens/VTokens/VBNB.sol";
66
import { Diamond } from "@venusprotocol/venus-protocol/contracts/Comptroller/Diamond/Diamond.sol";
7-
import { InterestRateModelHarness } from "@venusprotocol/venus-protocol/contracts/test/InterestRateModelHarness.sol";
87
import { MockVBNB } from "@venusprotocol/venus-protocol/contracts/test/MockVBNB.sol";
98
import { VBep20Harness } from "@venusprotocol/venus-protocol/contracts/test/VBep20Harness.sol";
109
import { ComptrollerLens } from "@venusprotocol/venus-protocol/contracts/Lens/ComptrollerLens.sol";
11-
import { FaucetToken } from "@venusprotocol/venus-protocol/contracts/test/FaucetToken.sol";

‎hardhat.config.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,23 @@ task("accounts", "Prints the list of accounts", async (taskArgs, hre) => {
7373
const config: HardhatUserConfig = {
7474
solidity: {
7575
compilers: [
76+
{
77+
version: "0.8.28",
78+
settings: {
79+
optimizer: {
80+
enabled: true,
81+
details: {
82+
yul: !process.env.CI,
83+
},
84+
},
85+
evmVersion: "cancun",
86+
outputSelection: {
87+
"*": {
88+
"*": ["storageLayout"],
89+
},
90+
},
91+
},
92+
},
7693
{
7794
version: "0.8.25",
7895
settings: {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
"@venusprotocol/isolated-pools": "^4.3.0",
8383
"@venusprotocol/oracle": "^2.13.0",
8484
"@venusprotocol/protocol-reserve": "^3.3.0",
85-
"@venusprotocol/venus-protocol": "file:../venus-protocol/venusprotocol-venus-protocol-9.8.0-dev.20.tgz",
85+
"@venusprotocol/venus-protocol": "10.0.0",
8686
"bignumber.js": "9.0.0",
8787
"chai": "^4.3.6",
8888
"dotenv": "^10.0.0",

0 commit comments

Comments
 (0)