Generic Position Swapper by Debugger022 · Pull Request #13 · VenusProtocol/venus-periphery

Debugger022 · 2025-10-08T11:49:21Z

This PR contains generic position swapper contract with swap logic.

To run the tests use npm pack to pack the package (VenusProtocol/venus-protocol#648) and then import it in package.json file. Ex: "@venusprotocol/venus-protocol": "file:../venus-protocol/venusprotocol-venus-protocol-9.8.0-dev.19.tgz",

Debugger022 · 2025-10-31T11:17:28Z

    /// @notice Emitted when an approved pair is updated.
    event ApprovedPairUpdated(address marketFrom, address marketTo, address helper, bool oldStatus, bool newStatus);


This event is no longer needed.

Debugger022 · 2025-10-31T11:19:26Z

Not used anywhere in the contract.

Same for BorrowFailed, NoVTokenBalance, NoUnderlyingReceived and AccrueInterestFailed.

Debugger022 · 2025-10-31T11:24:44Z

+    /// @custom:error NoEnoughBalance
+    error NoEnoughBalance();


Suggested change

/// @custom:error NoEnoughBalance

error NoEnoughBalance();

/// @custom:error NotEnoughBalance

error NotEnoughBalance();

Debugger022 · 2025-10-31T11:27:41Z

+        if (address(_comptroller) == address(0)) revert ZeroAddress();
+        if (address(_swapHelper) == address(0)) revert ZeroAddress();
+        if (address(_wrappedNative) == address(0)) revert ZeroAddress();
+        if (address(_nativeVToken) == address(0)) revert ZeroAddress();


We can use ensureNonzeroAddress from "@venusprotocol/solidity-utilities/contracts/validators.sol".

Debugger022 · 2025-10-31T11:29:22Z

+    /**
+     * @notice Swap user's entire native collateral (e.g., vBNB) into wrapped collateral (e.g., vWBNB).
+     * @param user Address of the user.
+     */


Please include events and errors in the natspec too.

Debugger022 · 2025-10-31T11:29:38Z

+     * @param user Address of the user.
+     */


Please include events and errors in the natspec too.

Debugger022 · 2025-10-31T11:31:14Z

+    function _validateAndEnterMarket(address user, IVToken marketFrom, IVToken marketTo) internal {
+        if (COMPTROLLER.checkMembership(user, marketFrom) && !COMPTROLLER.checkMembership(user, marketTo)) {


We should create Natspec comments for all the internal functions to ensure consistency.

Debugger022 · 2025-10-31T11:32:20Z

+     * @notice Flash loan callback entrypoint called by Comptroller.
+     * @param vTokens Array with the borrowed vToken market (single element)
+     * @param amounts Array with the borrowed underlying amount (single element)
+     * @param premiums Array with the flash loan fee amount (single element)


Suggested change

* @param premiums Array with the flash loan fee amount (single element)

* @param premiums Array with the flash loan fee amount (single element)

* @param /*initiator The address that initiated the flash loan (unused)

Debugger022 · 2025-10-31T11:33:30Z

+    ) internal returns (uint256 borrowedAssetAmountToRepay) {
+        uint256 err;
+        // For vBNB to vWBNB we take flashLoan of WBNB as vBNB does not support flashLoan
+        if (borrowMarket != WRAPPED_NATIVE_MARKET) revert InvalidFlashLoanBorrowedAsset();


IMO, this validation is unnecessary since we are hardcoding the value in the internal function.

Debugger022 · 2025-10-31T11:34:25Z

+        // supply new collateral
+        IERC20Upgradeable(address(WRAPPED_NATIVE)).forceApprove(
+            address(transientMarketTo),
+            borrowedAssetAmount - borrowedAssetFees


Should we store it in a variable mintAmount = borrowedAssetAmount - borrowedAssetFee?

Debugger022 · 2025-10-31T11:35:19Z

+        if (nativeBalanceAfter < borrowedAssetAmount) {
+            revert InsufficientFundsToRepayFlashloan();
        }


Suggested change

if (nativeBalanceAfter < borrowedAssetAmount) {

revert InsufficientFundsToRepayFlashloan();

}

if (nativeBalanceAfter < borrowedAssetAmountToRepay) {

revert InsufficientFundsToRepayFlashloan();

}

Debugger022 · 2025-10-31T11:36:43Z

+            revert InsufficientFundsToRepayFlashloan();
        }
+        WRAPPED_NATIVE.deposit{ value: nativeBalanceAfter }();
+        IERC20Upgradeable(address(WRAPPED_NATIVE)).approve(address(COMPTROLLER), borrowedAssetAmountToRepay);


Suggested change

IERC20Upgradeable(address(WRAPPED_NATIVE)).approve(address(COMPTROLLER), borrowedAssetAmountToRepay);

IERC20Upgradeable(address(WRAPPED_NATIVE)).approve(address(WRAPPED_NATIVE_MARKET), borrowedAssetAmountToRepay);

The vWBNB will take the repayment.

Debugger022 · 2025-10-31T11:38:35Z

+        if (nativeBalanceAfter > borrowedAssetAmount) {
+            IERC20Upgradeable(address(WRAPPED_NATIVE)).safeTransfer(onBehalf, nativeBalanceAfter - borrowedAssetAmount);


We can create a transferDust function for this purpose, allowing for future reuse.

Debugger022 · 2025-10-31T11:39:51Z

+            address(transientMarketTo),
+            borrowedAssetAmount - borrowedAssetFees
+        );
+        err = WRAPPED_NATIVE_MARKET.mintBehalf(onBehalf, borrowedAssetAmount - borrowedAssetFees);


Are we considering that the user collateral minted on vWBNB will be slightly less than the actual vBNB collateral?

Debugger022 · 2025-10-31T11:43:20Z

+            uint256 amountToRepay = _executeSwapDebtNativeToWrapped(onBehalf, vTokens[0], amounts[0], premiums[0]);
+            repayAmounts[0] = amountToRepay;
+        } else {
+            revert ExecuteOperationNotCalledCorrectly();


Suggested change

revert ExecuteOperationNotCalledCorrectly();

revert InvalidExecuteOperation();

?

fred-venus · 2025-11-20T07:14:31Z

@@ -123,316 +87,764 @@ contract PositionSwapper is Ownable2StepUpgradeable, ReentrancyGuardUpgradeable
     */
    receive() external payable {}


should we add limitation here so that only WBNB could send fund ?

fred-venus · 2025-11-20T07:24:29Z

+     * @custom:error EnterMarketFailed When Comptroller.enterMarketBehalf returns a non-zero error code
+     */
+    function _validateAndEnterMarket(address user, IVToken marketFrom, IVToken marketTo) internal {
+        if (COMPTROLLER.checkMembership(user, marketFrom) && !COMPTROLLER.checkMembership(user, marketTo)) {


is it possible COMPTROLLER.checkMembership(user, marketFrom) -> false

Yes, users can supply an asset without enabling it as collateral by choosing not to enter the market.

and in this case, the position swapper will still be functioning 🤔

fred-venus · 2025-11-20T09:07:59Z

+        // feeRate should be expressed as a scaled number, e.g. 9e14 for 0.09% (since 1e18 = 100%)
+        //  FA = x / (1 - r)
+        uint256 denominator = MANTISSA_ONE - feeRate;
+        flashLoanAmount = (requiredAmount * MANTISSA_ONE) / denominator;


division here by default will round down, will it make the flashLoanAmount slightly smaller than we expect ?

I think the rounding effect is minimal and safe. If flashLoanAmount is a bit lower due to truncation, the fee becomes proportionally smaller too (as this final amount is what we are taking as flashLoan), so the final received amount remains ≥ requiredAmount after subtracting the fee.

I dont think fee will be truncated, this is how fee is calculated

let's say feeRate = 9e14 i.e. 0.09%
borrowedAmt = 1 ether = 1e18
then fee = 1e18 * 9e14 / 1e18 = 9e14 ------- no truncation

But back here

XXX / (1e18 - 9e14) -------- truncation

it could be a issue here

True, but in our case the fee won’t be calculated on 1e18. It’s calculated on the value returned by calculateFlashLoanAmount.

required = 1e18 (1 ether)
feeRate = 9e14 (0.09%)
flashLoanAmount = (1e36) / (1e18 - 9e14)
= 1000900810729656691 // truncated

This truncated value is then used for the fee:
fee = flashLoanAmount * feeRate / 1e18
1000900810729656691 * 9e14 / 1e18 // truncated

So truncation happens in both steps.
Because of that, flashLoanAmount - fee still ends up ≥ required. (1000000000000000000) here
I don’t see any case where this fails, but I can switch to a ceil version for stricter validation and to avoid unnecessary reverts.

fred-venus · 2025-11-20T13:16:17Z

+        if (toUnderlying.balanceOf(address(this)) < borrowedAssetAmount) revert InvalidFlashLoanAmountReceived();
+
+        // Perform swap using SwapHelper
+        _performSwap(


we should have return value i.e. the actual swap output amount here

when repayBorrow, we should pass in the actual swap output amount because it could be greater than we expect, this also adheres with the naming here i.e. minDebtAmountToSwap

transientDebtRepaymentAmount may hold the user’s full current debt in the case of a full repayment. We don’t want to repay more than that amount, and even if the swap returns more than expected, any excess will be sent back to the user using _refundDustToUser.

[VPD-306]: Position Swapper V2 Quantstamp Audit

[VPD-306]: Position Swapper V2 Cantina Audit

github-actions · 2026-02-05T07:26:09Z

Package	Line Rate	Branch Rate	Health
contracts	100%	100%	✔
contracts.LeverageManager	94%	80%	✔
contracts.PositionSwapper	93%	56%	✔
contracts.SwapHelper	100%	100%	✔
Summary	94% (456 / 484)	71% (187 / 264)	✔

Debugger022 added 4 commits October 8, 2025 17:14

feat: add generic swapper contract

33f00ab

feat: add SwapHelper contract

7792b57

refactor: update imports

988136d

feat: update interface

d81840d

Debugger022 marked this pull request as draft October 8, 2025 11:50

chechu assigned Debugger022 Oct 8, 2025

refactor: update PositionSwapper to intergrate Swaphelper

989dea3

Debugger022 force-pushed the feat/generic-position-swapper branch from 3c4004b to 989dea3 Compare October 14, 2025 11:48

Debugger022 and others added 5 commits October 14, 2025 19:13

fix: minor fixes

486d1cf

refactor: update unit tests of position swapper

5ae419f

feat: add position swapper unit tests for non native markets

234b16c

fix: clean up approved pair logic after moving to a unified SwapHelper

300dfbb

feat: update PositionSwapper to use flashLoan

28c916a

Debugger022 commented Oct 31, 2025

View reviewed changes

GitGuru7 added 2 commits October 31, 2025 17:46

feat: add IPositionSwapper and refactor PositionSwapper

f65aad5

refactor: refactor and cleanup unused code

1572520

GitGuru7 added 4 commits November 10, 2025 17:03

fix: update unit tests for swapHelper changes

ac4402a

refactor: minor cleanups

5add8bb

test: add position-swapper fork test

6c26ccd

test: add more fork tests for position-swapper

9dd9335

GitGuru7 marked this pull request as ready for review November 12, 2025 07:12

refactor: drop unnecessary payable and add mantissa-one constant

3259c8c

fred-venus reviewed Nov 20, 2025

View reviewed changes

fix: add sender validation in receive

972dd09

fred-venus reviewed Nov 20, 2025

View reviewed changes

GitGuru7 added 2 commits November 20, 2025 19:34

refactor: combine full and partial swap functions

893033e

fix: use ceil in calculateFlashLoanAmount

bd3458d

Base automatically changed from feat/VEN-1193 to develop December 2, 2025 12:53

Exef and others added 15 commits December 15, 2025 12:55

Merge branch 'develop' into feat/generic-position-swapper-develop-merge

d6362b4

Merge branch 'develop' into feat/generic-position-swapper

10eab6c

fix: formatting

08628a2

fix: tests

df1afeb

fix: tests ci check

15badf5

fix: tests ci check

88f8d71

fix: [VEN-1] Missing Initiator Validation in Flashloan Callback

9a3e27c

fix: [S1] No Validation that marketFrom and marketTo Are Different

b3b5517

fix: [S2] Return Data From SWAP_HELPER.call() Is Ignored

063c9f9

fix: [S3] Native Swap Functions Are Not Generic

5f3a8d8

fix: [1] Code Clarity

340b49f

docs: add quantstamp audit report for position-swapper-v2

e90fd7a

Merge pull request #46 from VenusProtocol/feat/vpd-306-quantstamp-audit

5beef24

[VPD-306]: Position Swapper V2 Quantstamp Audit

docs: add cantina audit report for position-swapper-v2

3ed9fb3

Merge pull request #48 from VenusProtocol/feat/vpd-306-cantina-audit

d99a896

[VPD-306]: Position Swapper V2 Cantina Audit

		/// @notice Emitted when an approved pair is updated.
		event ApprovedPairUpdated(address marketFrom, address marketTo, address helper, bool oldStatus, bool newStatus);

		function _validateAndEnterMarket(address user, IVToken marketFrom, IVToken marketTo) internal {
		if (COMPTROLLER.checkMembership(user, marketFrom) && !COMPTROLLER.checkMembership(user, marketTo)) {

	* @param premiums Array with the flash loan fee amount (single element)
	* @param premiums Array with the flash loan fee amount (single element)
	* @param /*initiator The address that initiated the flash loan (unused)

	IERC20Upgradeable(address(WRAPPED_NATIVE)).approve(address(COMPTROLLER), borrowedAssetAmountToRepay);
	IERC20Upgradeable(address(WRAPPED_NATIVE)).approve(address(WRAPPED_NATIVE_MARKET), borrowedAssetAmountToRepay);

		if (nativeBalanceAfter > borrowedAssetAmount) {
		IERC20Upgradeable(address(WRAPPED_NATIVE)).safeTransfer(onBehalf, nativeBalanceAfter - borrowedAssetAmount);

	revert ExecuteOperationNotCalledCorrectly();
	revert InvalidExecuteOperation();

		@@ -123,316 +87,764 @@ contract PositionSwapper is Ownable2StepUpgradeable, ReentrancyGuardUpgradeable
		*/
		receive() external payable {}

Uh oh!

Conversation

Debugger022 commented Oct 8, 2025 • edited by GitGuru7 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GitGuru7 Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fred-venus Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fred-venus Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fred-venus Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GitGuru7 Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fred-venus Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Feb 5, 2026

Uh oh!

Reviewers

Debugger022 commented Oct 8, 2025 •

edited by GitGuru7

Loading

GitGuru7 Nov 21, 2025 •

edited

Loading

fred-venus Nov 20, 2025 •

edited

Loading

fred-venus Nov 20, 2025 •

edited

Loading

fred-venus Nov 20, 2025 •

edited

Loading

GitGuru7 Nov 21, 2025 •

edited

Loading

fred-venus Nov 20, 2025 •

edited

Loading