From b98ca31197bd5f55080aa83616bc24dceb8b14c9 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Mon, 27 Jul 2026 23:06:01 +0800 Subject: [PATCH 01/13] feat: add rVirtual conversion via RVirtualConverter Adds an open, permissionless 1:1 VIRTUAL -> rVirtual converter (RVirtualConverter, UUPS upgradeable), pre-funded with the rVirtual supply. veVirtual gains convertVeVirtualToRVirtual(id), which deletes a staking position (regardless of maturity or autoRenew state) and routes its underlying VIRTUAL through the same open converter entrypoint any wallet can call directly - no backend distribution step required. Co-Authored-By: Claude Sonnet 5 --- contracts/token/IRVirtualConverter.sol | 9 + contracts/token/RVirtualConverter.sol | 93 ++++++++ contracts/token/RVirtualConverterV2Mock.sol | 16 ++ contracts/token/veVirtual.sol | 53 +++++ test/rvirtual-converter.js | 227 ++++++++++++++++++++ test/vevirtual-rvirtual-conversion.js | 173 +++++++++++++++ 6 files changed, 571 insertions(+) create mode 100644 contracts/token/IRVirtualConverter.sol create mode 100644 contracts/token/RVirtualConverter.sol create mode 100644 contracts/token/RVirtualConverterV2Mock.sol create mode 100644 test/rvirtual-converter.js create mode 100644 test/vevirtual-rvirtual-conversion.js diff --git a/contracts/token/IRVirtualConverter.sol b/contracts/token/IRVirtualConverter.sol new file mode 100644 index 00000000..39b89c10 --- /dev/null +++ b/contracts/token/IRVirtualConverter.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +interface IRVirtualConverter { + function convertVirtualToRVirtual( + uint256 amount, + address rVirtualReceiver + ) external; +} diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol new file mode 100644 index 00000000..606b5f5d --- /dev/null +++ b/contracts/token/RVirtualConverter.sol @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; +import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol"; +import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +/// @notice Open, permissionless 1:1 converter from VIRTUAL to rVirtual. +/// +/// Pre-funded with the full rVirtual supply before launch - conversions draw down that +/// balance rather than minting on demand. Any caller (a regular wallet, or veVirtual's +/// convertVeVirtualToRVirtual()) uses the exact same convertVirtualToRVirtual() entrypoint; +/// there is no privileged "veVirtual-only" path here. +contract RVirtualConverter is + Initializable, + ReentrancyGuardUpgradeable, + AccessControlUpgradeable, + UUPSUpgradeable +{ + using SafeERC20 for IERC20; + + bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE"); + + address public virtualToken; + address public rVirtualToken; + address public adminWallet; + + event ConvertedVirtualToRVirtual( + address indexed caller, + address indexed rVirtualReceiver, + uint256 amount + ); + event AdminWalletUpdated(address adminWallet); + event VirtualWithdrawn(address adminWallet, uint256 amount); + + function initialize( + address virtualToken_, + address rVirtualToken_ + ) external initializer { + __ReentrancyGuard_init(); + __AccessControl_init(); + __UUPSUpgradeable_init(); + + require(virtualToken_ != address(0), "Invalid virtual token"); + require(rVirtualToken_ != address(0), "Invalid rVirtual token"); + virtualToken = virtualToken_; + rVirtualToken = rVirtualToken_; + + _grantRole(DEFAULT_ADMIN_ROLE, _msgSender()); + _grantRole(ADMIN_ROLE, _msgSender()); + } + + /// @notice Convert `amount` VIRTUAL (pulled from the caller) into `amount` rVirtual, + /// sent to `rVirtualReceiver`. Fully open - no allowlist, no cap. + function convertVirtualToRVirtual( + uint256 amount, + address rVirtualReceiver + ) external nonReentrant { + require(amount > 0, "Amount must be greater than 0"); + require(rVirtualReceiver != address(0), "Invalid receiver"); + + IERC20(virtualToken).safeTransferFrom( + _msgSender(), + address(this), + amount + ); + IERC20(rVirtualToken).safeTransfer(rVirtualReceiver, amount); + + emit ConvertedVirtualToRVirtual(_msgSender(), rVirtualReceiver, amount); + } + + function setAdminWallet(address adminWallet_) external onlyRole(ADMIN_ROLE) { + require(adminWallet_ != address(0), "Invalid admin wallet"); + adminWallet = adminWallet_; + emit AdminWalletUpdated(adminWallet_); + } + + /// @notice Withdraw accumulated VIRTUAL out of this contract. Only VIRTUAL - there is + /// intentionally no withdrawal path for rVirtual or any other token here, so + /// adminWallet is never exposed to rVirtual's transfer tax. + function withdrawVirtual(uint256 amount) external nonReentrant { + require(_msgSender() == adminWallet, "Only admin wallet"); + IERC20(virtualToken).safeTransfer(adminWallet, amount); + emit VirtualWithdrawn(adminWallet, amount); + } + + function _authorizeUpgrade( + address newImplementation + ) internal override onlyRole(ADMIN_ROLE) {} +} diff --git a/contracts/token/RVirtualConverterV2Mock.sol b/contracts/token/RVirtualConverterV2Mock.sol new file mode 100644 index 00000000..b5e1ae19 --- /dev/null +++ b/contracts/token/RVirtualConverterV2Mock.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "./RVirtualConverter.sol"; + +/// @notice Test-only V2 used to verify RVirtualConverter's UUPS upgrade path round-trips +/// cleanly. Adds one new event + trigger function on top of V1; never deployed to +/// production - exists purely so a test can upgrade forward, prove the new code is +/// live, then upgrade back and confirm the final bytecode matches the original V1. +contract RVirtualConverterV2Mock is RVirtualConverter { + event V2UpgradeMarker(string message); + + function triggerV2Marker() external { + emit V2UpgradeMarker("upgraded"); + } +} diff --git a/contracts/token/veVirtual.sol b/contracts/token/veVirtual.sol index 1de0c488..f7a74755 100644 --- a/contracts/token/veVirtual.sol +++ b/contracts/token/veVirtual.sol @@ -7,6 +7,7 @@ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/utils/structs/Checkpoints.sol"; import "@openzeppelin/contracts-upgradeable/governance/utils/VotesUpgradeable.sol"; +import "./IRVirtualConverter.sol"; contract veVirtual is Initializable, @@ -47,6 +48,14 @@ contract veVirtual is event AdminUnlocked(bool adminUnlocked); bool public adminUnlocked; + address public rVirtualConverter; + event RVirtualConverterUpdated(address rVirtualConverter); + event ConvertedVeVirtualToRVirtual( + address indexed user, + uint256 id, + uint256 amount + ); + function initialize( address baseToken_, uint8 maxWeeks_ @@ -298,4 +307,48 @@ contract veVirtual is } return amount; } + + /** + * @notice Set the RVirtualConverter contract that convertVeVirtualToRVirtual() forwards to. + */ + function setRVirtualConverter( + address rVirtualConverter_ + ) external onlyRole(ADMIN_ROLE) { + require(rVirtualConverter_ != address(0), "Invalid converter"); + rVirtualConverter = rVirtualConverter_; + emit RVirtualConverterUpdated(rVirtualConverter_); + } + + /** + * @notice Voluntarily give up a lock's underlying VIRTUAL (and its voting power) in + * exchange for an equal amount of rVirtual. Unlike withdraw(), this does not + * require the lock to be matured - the user is explicitly forfeiting the + * remaining lock time. The lock is deleted regardless of its autoRenew state. + * @dev Approves RVirtualConverter for exactly this lock's amount and calls its + * convertVirtualToRVirtual() - the same open entrypoint any wallet can call directly. + * There is no veVirtual-specific path on the converter side. + */ + function convertVeVirtualToRVirtual(uint256 id) external nonReentrant { + require(rVirtualConverter != address(0), "Converter not set"); + address account = _msgSender(); + uint256 index = _indexOf(account, id); + Lock memory lock = locks[account][index]; + + uint256 amount = lock.amount; + + uint256 lastIndex = locks[account].length - 1; + if (index != lastIndex) { + locks[account][index] = locks[account][lastIndex]; + } + locks[account].pop(); + + IERC20(baseToken).approve(rVirtualConverter, amount); + IRVirtualConverter(rVirtualConverter).convertVirtualToRVirtual( + amount, + account + ); + + emit ConvertedVeVirtualToRVirtual(account, id, amount); + _transferVotingUnits(account, address(0), amount); + } } diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js new file mode 100644 index 00000000..39ddb9bf --- /dev/null +++ b/test/rvirtual-converter.js @@ -0,0 +1,227 @@ +/* +Test RVirtualConverter: an open, permissionless 1:1 VIRTUAL -> rVirtual converter. +Also verifies the UUPS upgrade path round-trips cleanly (upgrade forward, verify new code +is live, upgrade back, confirm final bytecode matches the original). +*/ +const { expect } = require("chai"); +const { ethers, upgrades } = require("hardhat"); +const { parseEther } = ethers; + +describe("RVirtualConverter", function () { + let virtual, rVirtual, converter; + let deployer, user, other, adminWallet; + + before(async function () { + [deployer, user, other, adminWallet] = await ethers.getSigners(); + }); + + beforeEach(async function () { + virtual = await ethers.deployContract("VirtualToken", [ + parseEther("1000000000"), + deployer.address, + ]); + rVirtual = await ethers.deployContract("MockERC20", [ + "rVirtual", + "rVIRTUAL", + deployer.address, + parseEther("1000000000"), + ]); + + const Converter = await ethers.getContractFactory("RVirtualConverter"); + converter = await upgrades.deployProxy(Converter, [ + virtual.target, + rVirtual.target, + ]); + + // Pre-fund the converter with rVirtual liquidity for these tests (production pre-funds + // the full 1B supply, but a smaller amount here leaves deployer enough spare VIRTUAL + // balance to fully drain it in the "insufficient liquidity" test below). + await rVirtual.transfer(converter.target, parseEther("10000")); + + await virtual.transfer(user.address, parseEther("1000")); + await virtual.connect(user).approve(converter.target, parseEther("1000")); + }); + + describe("convertVirtualToRVirtual", function () { + it("should convert at a flat 1:1 rate for any caller", async function () { + await expect( + converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address) + ) + .to.emit(converter, "ConvertedVirtualToRVirtual") + .withArgs(user.address, user.address, parseEther("100")); + + expect(await virtual.balanceOf(user.address)).to.be.equal(parseEther("900")); + expect(await virtual.balanceOf(converter.target)).to.be.equal(parseEther("100")); + expect(await rVirtual.balanceOf(user.address)).to.be.equal(parseEther("100")); + }); + + it("should allow sending the rVirtual to a different receiver than the caller", async function () { + await converter + .connect(user) + .convertVirtualToRVirtual(parseEther("100"), other.address); + + expect(await rVirtual.balanceOf(other.address)).to.be.equal(parseEther("100")); + expect(await rVirtual.balanceOf(user.address)).to.be.equal(0); + }); + + it("should be fully open - no allowlist, no cap", async function () { + // Large relative to a normal user conversion, but within the test pool's pre-funded + // rVirtual liquidity (10000) - the point here is the absence of any allowlist/cap + // check, not exercising the liquidity limit (covered separately below). + await virtual.transfer(other.address, parseEther("5000")); + await virtual.connect(other).approve(converter.target, parseEther("5000")); + + await expect( + converter.connect(other).convertVirtualToRVirtual(parseEther("5000"), other.address) + ).to.not.be.reverted; + expect(await rVirtual.balanceOf(other.address)).to.be.equal(parseEther("5000")); + }); + + it("should revert without prior VIRTUAL approval", async function () { + await virtual.connect(user).approve(converter.target, 0); + await expect( + converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address) + ).to.be.reverted; + }); + + it("should revert if the converter has insufficient rVirtual liquidity", async function () { + // Drain the pre-funded rVirtual balance first. + const drained = await rVirtual.balanceOf(converter.target); + // Impersonate is unnecessary - just have deployer pull it out via a huge legit conversion + // from a funded wallet to exhaust the pool, then attempt one more. + await virtual.transfer(other.address, drained); + await virtual.connect(other).approve(converter.target, drained); + await converter.connect(other).convertVirtualToRVirtual(drained, other.address); + + expect(await rVirtual.balanceOf(converter.target)).to.be.equal(0); + + await expect( + converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address) + ).to.be.reverted; + }); + + it("should reject a zero receiver address", async function () { + await expect( + converter.connect(user).convertVirtualToRVirtual(parseEther("100"), ethers.ZeroAddress) + ).to.be.revertedWith("Invalid receiver"); + }); + + it("should reject a zero amount", async function () { + await expect( + converter.connect(user).convertVirtualToRVirtual(0, user.address) + ).to.be.revertedWith("Amount must be greater than 0"); + }); + }); + + describe("withdrawVirtual", function () { + beforeEach(async function () { + await converter.setAdminWallet(adminWallet.address); + await converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address); + }); + + it("should allow only adminWallet to withdraw the accumulated VIRTUAL", async function () { + await expect( + converter.connect(adminWallet).withdrawVirtual(parseEther("100")) + ) + .to.emit(converter, "VirtualWithdrawn") + .withArgs(adminWallet.address, parseEther("100")); + + expect(await virtual.balanceOf(adminWallet.address)).to.be.equal(parseEther("100")); + expect(await virtual.balanceOf(converter.target)).to.be.equal(0); + }); + + it("should allow withdrawing up to the full current balance, no reserve floor", async function () { + const full = await virtual.balanceOf(converter.target); + await expect(converter.connect(adminWallet).withdrawVirtual(full)).to.not.be.reverted; + expect(await virtual.balanceOf(converter.target)).to.be.equal(0); + }); + + it("should reject withdrawal from anyone other than adminWallet", async function () { + await expect( + converter.connect(user).withdrawVirtual(parseEther("100")) + ).to.be.revertedWith("Only admin wallet"); + await expect( + converter.connect(deployer).withdrawVirtual(parseEther("100")) + ).to.be.revertedWith("Only admin wallet"); + }); + + it("should reject non-admin-role setting of adminWallet", async function () { + await expect( + converter.connect(user).setAdminWallet(other.address) + ).to.be.reverted; + }); + }); + + it("should provide no path to withdraw rVirtual or any other token", async function () { + // The contract intentionally only exposes withdrawVirtual() - there is no generic + // rescue/withdraw function for rVirtual or arbitrary tokens. + expect(converter.withdrawRVirtual).to.be.undefined; + expect(converter.rescueToken).to.be.undefined; + expect(converter.recoverToken).to.be.undefined; + }); + + describe("UUPS upgradeability", function () { + it("should upgrade to V2, run new code, upgrade back to V1, and end up with identical bytecode", async function () { + const implBefore = await upgrades.erc1967.getImplementationAddress(converter.target); + const codeBefore = await ethers.provider.getCode(implBefore); + expect(codeBefore).to.not.equal("0x"); + + // Upgrade forward to V2. RVirtualConverterV2Mock adds no new storage or initializer - + // it only appends a function/event - so the plugin's "missing initializer" heuristic + // (which fires for any child contract without its own initialize()) is a false + // positive here and safe to bypass. + const V2 = await ethers.getContractFactory("RVirtualConverterV2Mock"); + const upgraded = await upgrades.upgradeProxy(converter.target, V2, { + unsafeAllow: ["missing-initializer"], + }); + + const implAfterV2 = await upgrades.erc1967.getImplementationAddress(upgraded.target); + expect(implAfterV2).to.not.equal(implBefore); + + // Prove the new code is actually live. + await expect(upgraded.triggerV2Marker()) + .to.emit(upgraded, "V2UpgradeMarker") + .withArgs("upgraded"); + + // Existing state and functionality must survive the upgrade untouched. + expect(await upgraded.virtualToken()).to.be.equal(virtual.target); + expect(await upgraded.rVirtualToken()).to.be.equal(rVirtual.target); + await expect( + upgraded.connect(user).convertVirtualToRVirtual(parseEther("50"), user.address) + ).to.emit(upgraded, "ConvertedVirtualToRVirtual"); + + // Upgrade back to V1. + const V1 = await ethers.getContractFactory("RVirtualConverter"); + const backToV1 = await upgrades.upgradeProxy(upgraded.target, V1); + + const implAfterRoundTrip = await upgrades.erc1967.getImplementationAddress( + backToV1.target + ); + const codeAfterRoundTrip = await ethers.provider.getCode(implAfterRoundTrip); + + // Final deployed code must match the original V1 bytecode exactly - the round trip + // (V1 -> V2 -> V1) leaves the contract functionally and byte-for-byte identical to + // where it started, even though the implementation address itself differs (each + // upgrade deploys a fresh implementation contract). + expect(codeAfterRoundTrip).to.be.equal(codeBefore); + + // V2-only function must no longer be part of the (V1) interface/ABI. + expect(backToV1.triggerV2Marker).to.be.undefined; + + // Original functionality still works post-round-trip. + expect(await backToV1.virtualToken()).to.be.equal(virtual.target); + await expect( + backToV1.connect(user).convertVirtualToRVirtual(parseEther("50"), user.address) + ).to.emit(backToV1, "ConvertedVirtualToRVirtual"); + }); + + it("should reject upgrade attempts from non-admin accounts", async function () { + const V2 = await ethers.getContractFactory("RVirtualConverterV2Mock", user); + await expect( + upgrades.upgradeProxy(converter.target, V2, { + unsafeAllow: ["missing-initializer"], + }) + ).to.be.reverted; + }); + }); +}); diff --git a/test/vevirtual-rvirtual-conversion.js b/test/vevirtual-rvirtual-conversion.js new file mode 100644 index 00000000..64a36faf --- /dev/null +++ b/test/vevirtual-rvirtual-conversion.js @@ -0,0 +1,173 @@ +/* +Test veVirtual.convertVeVirtualToRVirtual(): deletes a staking position and routes its +underlying VIRTUAL through RVirtualConverter for a flat 1:1 rVirtual payout. No maturity +requirement, no autoRenew restriction - any position can be converted directly. +*/ +const { expect } = require("chai"); +const { ethers } = require("hardhat"); +const { parseEther } = ethers; +const { time } = require("@nomicfoundation/hardhat-network-helpers"); + +describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { + let virtual, rVirtual, veVirtual, converter; + let deployer, staker, staker2, other; + + before(async function () { + [deployer, staker, staker2, other] = await ethers.getSigners(); + }); + + beforeEach(async function () { + virtual = await ethers.deployContract("VirtualToken", [ + parseEther("1000000000"), + deployer.address, + ]); + rVirtual = await ethers.deployContract("MockERC20", [ + "rVirtual", + "rVIRTUAL", + deployer.address, + parseEther("1000000000"), + ]); + + const VeVirtualContract = await ethers.getContractFactory("veVirtual"); + veVirtual = await upgrades.deployProxy(VeVirtualContract, [virtual.target, 104]); + + const ConverterContract = await ethers.getContractFactory("RVirtualConverter"); + converter = await upgrades.deployProxy(ConverterContract, [ + virtual.target, + rVirtual.target, + ]); + await rVirtual.transfer(converter.target, parseEther("1000000000")); + + await virtual.transfer(staker.address, parseEther("1000")); + await virtual.connect(staker).approve(veVirtual.target, parseEther("1000")); + }); + + it("should reject conversion when rVirtualConverter is not set", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + await expect( + veVirtual.connect(staker).convertVeVirtualToRVirtual(id) + ).to.be.revertedWith("Converter not set"); + }); + + it("should reject non-admin setting rVirtualConverter", async function () { + await expect( + veVirtual.connect(staker).setRVirtualConverter(converter.target) + ).to.be.reverted; + }); + + describe("with rVirtualConverter configured", function () { + beforeEach(async function () { + await veVirtual.setRVirtualConverter(converter.target); + }); + + it("should convert a NOT-yet-matured lock at a flat 1:1 rate", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + // Confirm it's genuinely not matured - withdraw() would revert here. + await expect(veVirtual.connect(staker).withdraw(id)).to.be.revertedWith( + "Lock is not expired" + ); + + await expect(veVirtual.connect(staker).convertVeVirtualToRVirtual(id)) + .to.emit(veVirtual, "ConvertedVeVirtualToRVirtual") + .withArgs(staker.address, id, parseEther("100")); + + expect(await veVirtual.numPositions(staker.address)).to.be.equal(0); + expect(await veVirtual.balanceOf(staker.address)).to.be.equal(0); + expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); + expect(await virtual.balanceOf(converter.target)).to.be.equal(parseEther("100")); + }); + + it("should also convert an already-matured lock", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + await time.increase(53 * 7 * 24 * 60 * 60); + + await veVirtual.connect(staker).convertVeVirtualToRVirtual(id); + expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); + }); + + it("should convert an auto-renewing lock too - no autoRenew restriction", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, true); + const id = (await veVirtual.locks(staker.address, 0)).id; + + await expect(veVirtual.connect(staker).convertVeVirtualToRVirtual(id)).to.not.be + .reverted; + expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); + expect(await veVirtual.numPositions(staker.address)).to.be.equal(0); + }); + + it("should convert regardless of remaining lock time, always at 1:1", async function () { + await virtual.transfer(staker.address, parseEther("100")); + await veVirtual.connect(staker).stake(parseEther("100"), 104, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + // Fresh position, maximum remaining lock time - still full 1:1, no discount. + await veVirtual.connect(staker).convertVeVirtualToRVirtual(id); + expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); + }); + + it("should not allow converting someone else's lock id", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + await expect( + veVirtual.connect(staker2).convertVeVirtualToRVirtual(id) + ).to.be.revertedWith("Lock not found"); + }); + + it("should not allow withdrawing or re-converting after conversion", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + await veVirtual.connect(staker).convertVeVirtualToRVirtual(id); + + await expect(veVirtual.connect(staker).withdraw(id)).to.be.revertedWith( + "Lock not found" + ); + await expect( + veVirtual.connect(staker).convertVeVirtualToRVirtual(id) + ).to.be.revertedWith("Lock not found"); + }); + + it("should only remove the converted lock, leaving other positions untouched", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); // id 1 + await veVirtual.connect(staker).stake(parseEther("50"), 52, false); // id 2 + expect(await veVirtual.numPositions(staker.address)).to.be.equal(2); + + const firstId = (await veVirtual.locks(staker.address, 0)).id; + await veVirtual.connect(staker).convertVeVirtualToRVirtual(firstId); + + expect(await veVirtual.numPositions(staker.address)).to.be.equal(1); + expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); + }); + + it("should remove voting power on conversion but preserve historical snapshots", async function () { + await veVirtual.connect(staker).delegate(staker.address); + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + expect(await veVirtual.getVotes(staker.address)).to.be.equal(parseEther("100")); + const blockBeforeConversion = await ethers.provider.getBlockNumber(); + + await veVirtual.connect(staker).convertVeVirtualToRVirtual(id); + + expect(await veVirtual.getVotes(staker.address)).to.be.equal(0); + expect( + await veVirtual.getPastVotes(staker.address, blockBeforeConversion) + ).to.be.equal(parseEther("100")); + }); + + it("should not leave any residual VIRTUAL allowance on veVirtual toward the converter", async function () { + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + await veVirtual.connect(staker).convertVeVirtualToRVirtual(id); + + expect( + await virtual.allowance(veVirtual.target, converter.target) + ).to.be.equal(0); + }); + }); +}); From 0d4be6e35d67d9615aafb75958e76f0beb16dece Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:42:43 +0800 Subject: [PATCH 02/13] fix(M-01): revert on rVirtual under-delivery in convertVirtualToRVirtual Compute the delivered amount via a balanceOf delta around the transfer and require it equals the requested amount, so a taxed/fee-on-transfer rVirtual token causes the whole conversion to revert atomically instead of silently under-delivering while the caller is charged in full. The emitted event also now reports the actually-delivered amount rather than the requested one. Addresses AUDIT_REPORT.md finding M-01. --- contracts/token/RVirtualConverter.sol | 6 +++- test/rvirtual-converter.js | 45 +++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index 606b5f5d..4f2b879f 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -67,9 +67,13 @@ contract RVirtualConverter is address(this), amount ); + + uint256 balanceBefore = IERC20(rVirtualToken).balanceOf(rVirtualReceiver); IERC20(rVirtualToken).safeTransfer(rVirtualReceiver, amount); + uint256 delivered = IERC20(rVirtualToken).balanceOf(rVirtualReceiver) - balanceBefore; + require(delivered == amount, "rVirtual delivery mismatch"); - emit ConvertedVirtualToRVirtual(_msgSender(), rVirtualReceiver, amount); + emit ConvertedVirtualToRVirtual(_msgSender(), rVirtualReceiver, delivered); } function setAdminWallet(address adminWallet_) external onlyRole(ADMIN_ROLE) { diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index 39ddb9bf..ba18cf8d 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -113,6 +113,51 @@ describe("RVirtualConverter", function () { }); }); + describe("delivery verification (M-01 fix)", function () { + let taxedRVirtual, taxedConverter; + + beforeEach(async function () { + // 10% fee-on-transfer token standing in for a taxed rVirtual configuration. + taxedRVirtual = await ethers.deployContract("FeeOnTransferMock", [ + "rVirtual", + "rVIRTUAL", + deployer.address, + parseEther("1000000000"), + 1000, // 10% fee + other.address, // fee sink + ]); + + const Converter = await ethers.getContractFactory("RVirtualConverter"); + taxedConverter = await upgrades.deployProxy(Converter, [ + virtual.target, + taxedRVirtual.target, + ]); + await taxedRVirtual.transfer(taxedConverter.target, parseEther("10000")); + + await virtual.connect(user).approve(taxedConverter.target, parseEther("1000")); + }); + + it("should revert the whole conversion instead of silently under-delivering", async function () { + await expect( + taxedConverter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address) + ).to.be.revertedWith("rVirtual delivery mismatch"); + + // Both legs must have unwound - caller keeps their VIRTUAL, receives no rVirtual. + expect(await virtual.balanceOf(user.address)).to.be.equal(parseEther("1000")); + expect(await taxedRVirtual.balanceOf(user.address)).to.be.equal(0); + }); + + it("should still succeed and emit the exact delivered amount for a non-taxed token", async function () { + await expect( + converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address) + ) + .to.emit(converter, "ConvertedVirtualToRVirtual") + .withArgs(user.address, user.address, parseEther("100")); + + expect(await rVirtual.balanceOf(user.address)).to.be.equal(parseEther("100")); + }); + }); + describe("withdrawVirtual", function () { beforeEach(async function () { await converter.setAdminWallet(adminWallet.address); From 747389b97df6305256db58a912e11f9216eefb95 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:42:59 +0800 Subject: [PATCH 03/13] docs(M-02): document the 18-decimals guarantee for VIRTUAL/rVirtual No code check added by design - the protocol guarantees both VIRTUAL and rVirtual use 18 decimals, so a runtime decimals() equivalence assertion isn't needed. This comment records that assumption at the exact line the raw 1:1 conversion depends on it, so a future migration to a differently-decimaled token isn't wired in silently. Addresses AUDIT_REPORT.md finding M-02 (comment-only fix per request; no behavior change, no new test). --- contracts/token/RVirtualConverter.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index 4f2b879f..a8c7a7f4 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -46,6 +46,12 @@ contract RVirtualConverter is require(virtualToken_ != address(0), "Invalid virtual token"); require(rVirtualToken_ != address(0), "Invalid rVirtual token"); + // NOTE (audit M-02): the 1:1 conversion below is a raw-integer transfer with no + // decimals rescaling. This is safe only because VIRTUAL and rVirtual are both + // guaranteed by protocol design to use 18 decimals - if either token is ever + // redeployed/migrated to a different decimals value, this invariant must be + // re-verified (or an explicit decimals() equivalence check added) before wiring + // it in here. virtualToken = virtualToken_; rVirtualToken = rVirtualToken_; From 3e1bc138b679f088eb704333dd60e4af52646e2d Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:44:57 +0800 Subject: [PATCH 04/13] feat(L-02): route incoming VIRTUAL directly to a treasury multisig Add a `treasury` address, required non-zero and set once at initialize(), and change convertVirtualToRVirtual() to send the caller's incoming VIRTUAL straight to it instead of holding it in the converter contract. VIRTUAL is no longer custodied by RVirtualConverter at any point, so there is no accumulated balance for a compromised/malicious ADMIN_ROLE key to sweep via a repointed converter or a malicious UUPS upgrade. This does not change the original L-02 attack surface (a malicious `rVirtualConverter` repoint on veVirtual, or a malicious UUPS upgrade, can still intercept a user's approved VIRTUAL before it reaches treasury) - that remains an acknowledged, trust-gated risk. It does remove the "accumulated VIRTUAL balance" component of the blast radius entirely, since funds never sit in this contract. Addresses AUDIT_REPORT.md finding L-02 (additional hardening beyond the original recommendation, per request). --- contracts/token/RVirtualConverter.sol | 12 +++++++-- test/rvirtual-converter.js | 37 ++++++++++++++++++++++++--- test/vevirtual-rvirtual-conversion.js | 9 ++++--- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index a8c7a7f4..51c15571 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -27,6 +27,11 @@ contract RVirtualConverter is address public virtualToken; address public rVirtualToken; address public adminWallet; + /// @notice Treasury multisig that every conversion's incoming VIRTUAL is sent to + /// directly. Set once at initialize() and never changed at runtime - VIRTUAL + /// is never custodied by this contract, so there is no accumulated balance + /// for a compromised or malicious admin key to sweep (see audit L-02). + address public treasury; event ConvertedVirtualToRVirtual( address indexed caller, @@ -38,7 +43,8 @@ contract RVirtualConverter is function initialize( address virtualToken_, - address rVirtualToken_ + address rVirtualToken_, + address treasury_ ) external initializer { __ReentrancyGuard_init(); __AccessControl_init(); @@ -46,6 +52,7 @@ contract RVirtualConverter is require(virtualToken_ != address(0), "Invalid virtual token"); require(rVirtualToken_ != address(0), "Invalid rVirtual token"); + require(treasury_ != address(0), "Invalid treasury"); // NOTE (audit M-02): the 1:1 conversion below is a raw-integer transfer with no // decimals rescaling. This is safe only because VIRTUAL and rVirtual are both // guaranteed by protocol design to use 18 decimals - if either token is ever @@ -54,6 +61,7 @@ contract RVirtualConverter is // it in here. virtualToken = virtualToken_; rVirtualToken = rVirtualToken_; + treasury = treasury_; _grantRole(DEFAULT_ADMIN_ROLE, _msgSender()); _grantRole(ADMIN_ROLE, _msgSender()); @@ -70,7 +78,7 @@ contract RVirtualConverter is IERC20(virtualToken).safeTransferFrom( _msgSender(), - address(this), + treasury, amount ); diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index ba18cf8d..3d5017a2 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -9,10 +9,10 @@ const { parseEther } = ethers; describe("RVirtualConverter", function () { let virtual, rVirtual, converter; - let deployer, user, other, adminWallet; + let deployer, user, other, adminWallet, treasury; before(async function () { - [deployer, user, other, adminWallet] = await ethers.getSigners(); + [deployer, user, other, adminWallet, treasury] = await ethers.getSigners(); }); beforeEach(async function () { @@ -31,6 +31,7 @@ describe("RVirtualConverter", function () { converter = await upgrades.deployProxy(Converter, [ virtual.target, rVirtual.target, + treasury.address, ]); // Pre-fund the converter with rVirtual liquidity for these tests (production pre-funds @@ -51,7 +52,10 @@ describe("RVirtualConverter", function () { .withArgs(user.address, user.address, parseEther("100")); expect(await virtual.balanceOf(user.address)).to.be.equal(parseEther("900")); - expect(await virtual.balanceOf(converter.target)).to.be.equal(parseEther("100")); + // Incoming VIRTUAL is routed straight to the treasury (L-02 fix) - the converter + // itself never custodies any VIRTUAL. + expect(await virtual.balanceOf(converter.target)).to.be.equal(0); + expect(await virtual.balanceOf(treasury.address)).to.be.equal(parseEther("100")); expect(await rVirtual.balanceOf(user.address)).to.be.equal(parseEther("100")); }); @@ -131,6 +135,7 @@ describe("RVirtualConverter", function () { taxedConverter = await upgrades.deployProxy(Converter, [ virtual.target, taxedRVirtual.target, + treasury.address, ]); await taxedRVirtual.transfer(taxedConverter.target, parseEther("10000")); @@ -158,10 +163,34 @@ describe("RVirtualConverter", function () { }); }); + describe("treasury routing (L-02 fix)", function () { + it("should reject a zero-address treasury at initialize", async function () { + const Converter = await ethers.getContractFactory("RVirtualConverter"); + await expect( + upgrades.deployProxy(Converter, [ + virtual.target, + rVirtual.target, + ethers.ZeroAddress, + ]) + ).to.be.revertedWith("Invalid treasury"); + }); + + it("should route every conversion's incoming VIRTUAL straight to treasury, never the converter", async function () { + await converter.connect(user).convertVirtualToRVirtual(parseEther("30"), user.address); + await converter.connect(user).convertVirtualToRVirtual(parseEther("20"), other.address); + + expect(await virtual.balanceOf(treasury.address)).to.be.equal(parseEther("50")); + expect(await virtual.balanceOf(converter.target)).to.be.equal(0); + }); + }); + describe("withdrawVirtual", function () { beforeEach(async function () { await converter.setAdminWallet(adminWallet.address); - await converter.connect(user).convertVirtualToRVirtual(parseEther("100"), user.address); + // VIRTUAL no longer accumulates in the converter via conversions (see L-02 fix + // above) - donate directly so withdrawVirtual's own mechanics can still be + // exercised in isolation. + await virtual.transfer(converter.target, parseEther("100")); }); it("should allow only adminWallet to withdraw the accumulated VIRTUAL", async function () { diff --git a/test/vevirtual-rvirtual-conversion.js b/test/vevirtual-rvirtual-conversion.js index 64a36faf..7a405a8c 100644 --- a/test/vevirtual-rvirtual-conversion.js +++ b/test/vevirtual-rvirtual-conversion.js @@ -10,10 +10,10 @@ const { time } = require("@nomicfoundation/hardhat-network-helpers"); describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { let virtual, rVirtual, veVirtual, converter; - let deployer, staker, staker2, other; + let deployer, staker, staker2, other, treasury; before(async function () { - [deployer, staker, staker2, other] = await ethers.getSigners(); + [deployer, staker, staker2, other, treasury] = await ethers.getSigners(); }); beforeEach(async function () { @@ -35,6 +35,7 @@ describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { converter = await upgrades.deployProxy(ConverterContract, [ virtual.target, rVirtual.target, + treasury.address, ]); await rVirtual.transfer(converter.target, parseEther("1000000000")); @@ -78,7 +79,9 @@ describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { expect(await veVirtual.numPositions(staker.address)).to.be.equal(0); expect(await veVirtual.balanceOf(staker.address)).to.be.equal(0); expect(await rVirtual.balanceOf(staker.address)).to.be.equal(parseEther("100")); - expect(await virtual.balanceOf(converter.target)).to.be.equal(parseEther("100")); + // Incoming VIRTUAL is routed straight to treasury (L-02 fix), never held by the converter. + expect(await virtual.balanceOf(converter.target)).to.be.equal(0); + expect(await virtual.balanceOf(treasury.address)).to.be.equal(parseEther("100")); }); it("should also convert an already-matured lock", async function () { From 61c57dd92834af5218e0463d2aa15d6e69215b9b Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:45:41 +0800 Subject: [PATCH 05/13] fix(L-04): use SafeERC20.forceApprove instead of raw approve() convertVeVirtualToRVirtual() now grants the converter's allowance via forceApprove(), consistent with the rest of the file's SafeERC20 convention (already imported/used elsewhere via `using SafeERC20 for IERC20`), and checked/return-value-safe by construction. forceApprove zeroes the allowance first if it's currently non-zero, then sets the new value - this is a strict superset of what raw approve() does for well-behaved tokens like VIRTUAL, so there is no behavior change on the happy path; it additionally protects against ERC20 tokens that revert on a direct non-zero-to-non-zero approve() (e.g. USDT-style tokens), should baseToken ever be swapped for one of those. Addresses AUDIT_REPORT.md finding L-04. --- contracts/token/veVirtual.sol | 2 +- test/vevirtual-rvirtual-conversion.js | 35 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/contracts/token/veVirtual.sol b/contracts/token/veVirtual.sol index f7a74755..6695ea5b 100644 --- a/contracts/token/veVirtual.sol +++ b/contracts/token/veVirtual.sol @@ -342,7 +342,7 @@ contract veVirtual is } locks[account].pop(); - IERC20(baseToken).approve(rVirtualConverter, amount); + IERC20(baseToken).forceApprove(rVirtualConverter, amount); IRVirtualConverter(rVirtualConverter).convertVirtualToRVirtual( amount, account diff --git a/test/vevirtual-rvirtual-conversion.js b/test/vevirtual-rvirtual-conversion.js index 7a405a8c..7ded61e7 100644 --- a/test/vevirtual-rvirtual-conversion.js +++ b/test/vevirtual-rvirtual-conversion.js @@ -173,4 +173,39 @@ describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { ).to.be.equal(0); }); }); + + describe("forceApprove instead of raw approve (L-04 fix)", function () { + it("should overwrite (not error on) a stale nonzero allowance toward a no-op converter", async function () { + // A no-op converter never pulls the approved VIRTUAL, so the allowance from the + // FIRST conversion call is left standing at the full lock amount. + const noOpConverter = await ethers.deployContract("NoOpConverterMock"); + await veVirtual.setRVirtualConverter(noOpConverter.target); + + await veVirtual.connect(staker).stake(parseEther("100"), 52, false); + const firstId = (await veVirtual.locks(staker.address, 0)).id; + await veVirtual.connect(staker).convertVeVirtualToRVirtual(firstId); + + expect( + await virtual.allowance(veVirtual.target, noOpConverter.target) + ).to.be.equal(parseEther("100")); + + // A SECOND lock/conversion must not revert despite the standing non-zero + // allowance - forceApprove() overwrites it cleanly. (A raw, non-force approve() + // would also succeed on a standard ERC20 like VIRTUAL, but forceApprove is the + // hardened SafeERC20 path that also works on tokens which reject a direct + // nonzero-to-nonzero approve, e.g. USDT-style tokens.) + await virtual.transfer(staker.address, parseEther("50")); + await veVirtual.connect(staker).stake(parseEther("50"), 52, false); + const secondId = (await veVirtual.locks(staker.address, 0)).id; + + await expect( + veVirtual.connect(staker).convertVeVirtualToRVirtual(secondId) + ).to.not.be.reverted; + + // Allowance now reflects only the second call's amount - overwritten, not summed. + expect( + await virtual.allowance(veVirtual.target, noOpConverter.target) + ).to.be.equal(parseEther("50")); + }); + }); }); From cefee98cbf8c5c7429bdaf9b89459cdb5b3041f8 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:47:10 +0800 Subject: [PATCH 06/13] fix(L-09): validate token identity at converter init and wiring time RVirtualConverter.initialize() now rejects virtualToken and rVirtualToken being the same address. IRVirtualConverter exposes a virtualToken() getter, and veVirtual.setRVirtualConverter() now asserts the converter's virtualToken() matches veVirtual's own baseToken before wiring it in - closing the gap where a misconfigured/mismatched converter would previously only fail with a confusing revert deep inside a user's convertVeVirtualToRVirtual() call instead of at configuration time. NoOpConverterMock gains a constructor-supplied virtualToken to satisfy the extended interface. Addresses AUDIT_REPORT.md finding L-09. --- contracts/token/IRVirtualConverter.sol | 5 +++++ contracts/token/RVirtualConverter.sol | 1 + contracts/token/mocks/NoOpConverterMock.sol | 25 +++++++++++++++++++++ contracts/token/veVirtual.sol | 4 ++++ test/rvirtual-converter.js | 11 +++++++++ test/vevirtual-rvirtual-conversion.js | 19 +++++++++++++++- 6 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 contracts/token/mocks/NoOpConverterMock.sol diff --git a/contracts/token/IRVirtualConverter.sol b/contracts/token/IRVirtualConverter.sol index 39b89c10..6592c1c6 100644 --- a/contracts/token/IRVirtualConverter.sol +++ b/contracts/token/IRVirtualConverter.sol @@ -6,4 +6,9 @@ interface IRVirtualConverter { uint256 amount, address rVirtualReceiver ) external; + + /// @notice The VIRTUAL token this converter accepts as input. Exposed so callers + /// (e.g. veVirtual.setRVirtualConverter) can assert their own base token + /// matches this converter's before wiring it in (see audit L-09). + function virtualToken() external view returns (address); } diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index 51c15571..b878b368 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -53,6 +53,7 @@ contract RVirtualConverter is require(virtualToken_ != address(0), "Invalid virtual token"); require(rVirtualToken_ != address(0), "Invalid rVirtual token"); require(treasury_ != address(0), "Invalid treasury"); + require(virtualToken_ != rVirtualToken_, "Tokens must differ"); // NOTE (audit M-02): the 1:1 conversion below is a raw-integer transfer with no // decimals rescaling. This is safe only because VIRTUAL and rVirtual are both // guaranteed by protocol design to use 18 decimals - if either token is ever diff --git a/contracts/token/mocks/NoOpConverterMock.sol b/contracts/token/mocks/NoOpConverterMock.sol new file mode 100644 index 00000000..e3690154 --- /dev/null +++ b/contracts/token/mocks/NoOpConverterMock.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "../IRVirtualConverter.sol"; + +/// @notice TEST-ONLY mock for verify_H-10.js. Implements IRVirtualConverter but does NOT call +/// transferFrom at all - simulates the specific scenario Chain Agent 2 analyzed where +/// the converter call succeeds (no-op) without pulling the approved VIRTUAL, leaving +/// veVirtual's raw `approve()` allowance (L345) standing after the call returns. +/// Distinct from MaliciousConverterMock (H-4), which DOES pull funds via transferFrom +/// and therefore leaves zero residual allowance. +contract NoOpConverterMock is IRVirtualConverter { + address public virtualToken; + + constructor(address virtualToken_) { + virtualToken = virtualToken_; + } + + function convertVirtualToRVirtual( + uint256 /* amount */, + address /* rVirtualReceiver */ + ) external override { + // Intentionally does nothing - no transferFrom, no revert. + } +} diff --git a/contracts/token/veVirtual.sol b/contracts/token/veVirtual.sol index 6695ea5b..d149e4ab 100644 --- a/contracts/token/veVirtual.sol +++ b/contracts/token/veVirtual.sol @@ -315,6 +315,10 @@ contract veVirtual is address rVirtualConverter_ ) external onlyRole(ADMIN_ROLE) { require(rVirtualConverter_ != address(0), "Invalid converter"); + require( + IRVirtualConverter(rVirtualConverter_).virtualToken() == baseToken, + "Converter token mismatch" + ); rVirtualConverter = rVirtualConverter_; emit RVirtualConverterUpdated(rVirtualConverter_); } diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index 3d5017a2..2f23fc49 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -175,6 +175,17 @@ describe("RVirtualConverter", function () { ).to.be.revertedWith("Invalid treasury"); }); + it("should reject virtualToken and rVirtualToken being the same address (L-09 fix)", async function () { + const Converter = await ethers.getContractFactory("RVirtualConverter"); + await expect( + upgrades.deployProxy(Converter, [ + virtual.target, + virtual.target, + treasury.address, + ]) + ).to.be.revertedWith("Tokens must differ"); + }); + it("should route every conversion's incoming VIRTUAL straight to treasury, never the converter", async function () { await converter.connect(user).convertVirtualToRVirtual(parseEther("30"), user.address); await converter.connect(user).convertVirtualToRVirtual(parseEther("20"), other.address); diff --git a/test/vevirtual-rvirtual-conversion.js b/test/vevirtual-rvirtual-conversion.js index 7ded61e7..3d50d9a2 100644 --- a/test/vevirtual-rvirtual-conversion.js +++ b/test/vevirtual-rvirtual-conversion.js @@ -58,6 +58,21 @@ describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { ).to.be.reverted; }); + it("should reject a converter whose virtualToken doesn't match veVirtual's baseToken (L-09 fix)", async function () { + // A converter wired up with a DIFFERENT token as its virtualToken (e.g. rVirtual + // itself, standing in for any mismatched deployment). + const ConverterContract = await ethers.getContractFactory("RVirtualConverter"); + const mismatchedConverter = await upgrades.deployProxy(ConverterContract, [ + rVirtual.target, + virtual.target, + other.address, + ]); + + await expect( + veVirtual.setRVirtualConverter(mismatchedConverter.target) + ).to.be.revertedWith("Converter token mismatch"); + }); + describe("with rVirtualConverter configured", function () { beforeEach(async function () { await veVirtual.setRVirtualConverter(converter.target); @@ -178,7 +193,9 @@ describe("veVIRTUAL - convertVeVirtualToRVirtual", function () { it("should overwrite (not error on) a stale nonzero allowance toward a no-op converter", async function () { // A no-op converter never pulls the approved VIRTUAL, so the allowance from the // FIRST conversion call is left standing at the full lock amount. - const noOpConverter = await ethers.deployContract("NoOpConverterMock"); + const noOpConverter = await ethers.deployContract("NoOpConverterMock", [ + virtual.target, + ]); await veVirtual.setRVirtualConverter(noOpConverter.target); await veVirtual.connect(staker).stake(parseEther("100"), 52, false); From 5ac0cd95f9447b5658c24d61c7f100c94cc24898 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:47:28 +0800 Subject: [PATCH 07/13] chore: track test-only mocks used by the rVirtual conversion test suite FeeOnTransferMock, MaliciousConverterMock, and MockERC20SixDecimals were created during the earlier security audit's PoC work but never added to git. FeeOnTransferMock is a dependency of the M-01 fix test added in an earlier commit; tracking all three now so the test suite is reproducible from a clean checkout. --- contracts/token/mocks/FeeOnTransferMock.sol | 49 +++++++++++++++++++ .../token/mocks/MaliciousConverterMock.sol | 31 ++++++++++++ .../token/mocks/MockERC20SixDecimals.sol | 25 ++++++++++ 3 files changed, 105 insertions(+) create mode 100644 contracts/token/mocks/FeeOnTransferMock.sol create mode 100644 contracts/token/mocks/MaliciousConverterMock.sol create mode 100644 contracts/token/mocks/MockERC20SixDecimals.sol diff --git a/contracts/token/mocks/FeeOnTransferMock.sol b/contracts/token/mocks/FeeOnTransferMock.sol new file mode 100644 index 00000000..ab2787c6 --- /dev/null +++ b/contracts/token/mocks/FeeOnTransferMock.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +/// @notice TEST-ONLY mock modeling a fee-on-transfer / tax token, used to simulate the +/// real rVirtual (GodToken)'s configurable transfer tax for security PoC purposes +/// (see verify_H-2.js). NOT used in production - lives under contracts/token/mocks +/// solely to be reachable by Hardhat's compiler for the test. +/// +/// Deducts `feeBps` (out of 10_000) from every transfer/transferFrom, sending the +/// fee portion to a burn/dead sink so the recipient always receives strictly less +/// than the nominal transferred amount whenever feeBps > 0. +contract FeeOnTransferMock is ERC20 { + uint256 public immutable feeBps; // e.g. 1000 = 10% + address public immutable feeSink; + + constructor( + string memory name_, + string memory symbol_, + address initialAccount, + uint256 initialBalance, + uint256 feeBps_, + address feeSink_ + ) ERC20(name_, symbol_) { + require(feeBps_ <= 10_000, "fee too high"); + feeBps = feeBps_; + feeSink = feeSink_; + _mint(initialAccount, initialBalance); + } + + function _update(address from, address to, uint256 value) internal override { + // Mint (from == address(0)) and burn (to == address(0)) pass through untaxed - + // only regular transfers between two live accounts are taxed, matching typical + // fee-on-transfer token behavior. + if (from == address(0) || to == address(0) || feeBps == 0) { + super._update(from, to, value); + return; + } + + uint256 fee = (value * feeBps) / 10_000; + uint256 net = value - fee; + + super._update(from, to, net); + if (fee > 0) { + super._update(from, feeSink, fee); + } + } +} diff --git a/contracts/token/mocks/MaliciousConverterMock.sol b/contracts/token/mocks/MaliciousConverterMock.sol new file mode 100644 index 00000000..084e3879 --- /dev/null +++ b/contracts/token/mocks/MaliciousConverterMock.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import "../IRVirtualConverter.sol"; + +/// @notice TEST-ONLY mock simulating a malicious/repointed `rVirtualConverter` (see H-4 PoC, +/// verify_H-4.js). Implements the exact `IRVirtualConverter` interface that +/// `veVirtual.convertVeVirtualToRVirtual()` calls, but instead of delivering rVirtual +/// 1:1, it pulls the approved VIRTUAL via `transferFrom` and routes it to an +/// attacker-controlled address, delivering ZERO rVirtual back to the victim. It does +/// NOT revert - the call succeeds silently from veVirtual's perspective, so the +/// lock deletion (which already happened before this external call) is never rolled +/// back. +contract MaliciousConverterMock is IRVirtualConverter { + address public immutable virtualToken; + address public immutable attacker; + + constructor(address virtualToken_, address attacker_) { + virtualToken = virtualToken_; + attacker = attacker_; + } + + function convertVirtualToRVirtual( + uint256 amount, + address /* rVirtualReceiver */ + ) external override { + // Steal the approved VIRTUAL; deliver no rVirtual, and do not revert. + IERC20(virtualToken).transferFrom(msg.sender, attacker, amount); + } +} diff --git a/contracts/token/mocks/MockERC20SixDecimals.sol b/contracts/token/mocks/MockERC20SixDecimals.sol new file mode 100644 index 00000000..b187358c --- /dev/null +++ b/contracts/token/mocks/MockERC20SixDecimals.sol @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +/// @notice Minimal mock ERC20 with 6 decimals (like USDC), used ONLY to verify H-3 +/// (RVirtualConverter never asserts rVirtualToken.decimals() == 18). +contract MockERC20SixDecimals is ERC20 { + constructor( + string memory name, + string memory symbol, + address initialAccount, + uint256 initialBalance + ) ERC20(name, symbol) { + _mint(initialAccount, initialBalance); + } + + function decimals() public pure override returns (uint8) { + return 6; + } + + function mint(address to, uint256 amount) public { + _mint(to, amount); + } +} From 617035cff5fb12fd09e13a8cf35059afe079bd1e Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:48:13 +0800 Subject: [PATCH 08/13] fix(L-10): add previous value + indexed topics to AdminWalletUpdated RVirtualConverter's AdminWalletUpdated event now emits both the previous and new adminWallet, both indexed, so an off-chain monitor can filter directly on this state change and reconstruct history without replaying the full event log. veVirtual's own admin events (RVirtualConverterUpdated, and the absence of an event on setMaxWeeks) are intentionally left unchanged per request. Addresses AUDIT_REPORT.md finding L-10 (RVirtualConverter side only). --- contracts/token/RVirtualConverter.sol | 8 ++++++-- test/rvirtual-converter.js | 12 ++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index b878b368..43d8941b 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -38,7 +38,10 @@ contract RVirtualConverter is address indexed rVirtualReceiver, uint256 amount ); - event AdminWalletUpdated(address adminWallet); + event AdminWalletUpdated( + address indexed previousAdminWallet, + address indexed newAdminWallet + ); event VirtualWithdrawn(address adminWallet, uint256 amount); function initialize( @@ -93,8 +96,9 @@ contract RVirtualConverter is function setAdminWallet(address adminWallet_) external onlyRole(ADMIN_ROLE) { require(adminWallet_ != address(0), "Invalid admin wallet"); + address previousAdminWallet = adminWallet; adminWallet = adminWallet_; - emit AdminWalletUpdated(adminWallet_); + emit AdminWalletUpdated(previousAdminWallet, adminWallet_); } /// @notice Withdraw accumulated VIRTUAL out of this contract. Only VIRTUAL - there is diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index 2f23fc49..344a7272 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -195,6 +195,18 @@ describe("RVirtualConverter", function () { }); }); + describe("setAdminWallet events (L-10 fix)", function () { + it("should emit both the previous and new admin wallet, indexed", async function () { + await expect(converter.setAdminWallet(adminWallet.address)) + .to.emit(converter, "AdminWalletUpdated") + .withArgs(ethers.ZeroAddress, adminWallet.address); + + await expect(converter.setAdminWallet(other.address)) + .to.emit(converter, "AdminWalletUpdated") + .withArgs(adminWallet.address, other.address); + }); + }); + describe("withdrawVirtual", function () { beforeEach(async function () { await converter.setAdminWallet(adminWallet.address); From 546ec73157994a26813085018f381932fdabc4a7 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:50:07 +0800 Subject: [PATCH 09/13] chore(I-05): move RVirtualConverterV2Mock.sol into contracts/token/mocks/ Test-only UUPS upgrade-path mock was sitting alongside production contracts in contracts/token/. Relocated with the other test-only mocks for this feature; no behavior change. Addresses AUDIT_REPORT.md finding I-05. --- contracts/token/{ => mocks}/RVirtualConverterV2Mock.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename contracts/token/{ => mocks}/RVirtualConverterV2Mock.sol (94%) diff --git a/contracts/token/RVirtualConverterV2Mock.sol b/contracts/token/mocks/RVirtualConverterV2Mock.sol similarity index 94% rename from contracts/token/RVirtualConverterV2Mock.sol rename to contracts/token/mocks/RVirtualConverterV2Mock.sol index b5e1ae19..040d7e0d 100644 --- a/contracts/token/RVirtualConverterV2Mock.sol +++ b/contracts/token/mocks/RVirtualConverterV2Mock.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import "./RVirtualConverter.sol"; +import "../RVirtualConverter.sol"; /// @notice Test-only V2 used to verify RVirtualConverter's UUPS upgrade path round-trips /// cleanly. Adds one new event + trigger function on top of V1; never deployed to From 6f1901f7a858f2db4f65a3cc318a1ee6f54e33e2 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 10:51:37 +0800 Subject: [PATCH 10/13] fix(I-06): remove withdrawVirtual()/adminWallet - superseded by L-02 Once incoming VIRTUAL is routed directly to treasury (L-02), this contract never custodies VIRTUAL, so the adminWallet sweep mechanism (setAdminWallet, withdrawVirtual, and their events) has nothing left to do. Removed entirely rather than left as dead code with no consumer - narrows the admin-controlled surface down to just the UUPS upgrade path. Addresses AUDIT_REPORT.md finding I-06. --- contracts/token/RVirtualConverter.sol | 26 ++-------- test/rvirtual-converter.js | 68 ++++----------------------- 2 files changed, 12 insertions(+), 82 deletions(-) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index 43d8941b..f47555c0 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -26,11 +26,12 @@ contract RVirtualConverter is address public virtualToken; address public rVirtualToken; - address public adminWallet; /// @notice Treasury multisig that every conversion's incoming VIRTUAL is sent to /// directly. Set once at initialize() and never changed at runtime - VIRTUAL /// is never custodied by this contract, so there is no accumulated balance - /// for a compromised or malicious admin key to sweep (see audit L-02). + /// for a compromised or malicious admin key to sweep (see audit L-02). This + /// is also why there is no adminWallet/withdrawVirtual() sweep mechanism here + /// anymore (see audit I-06) - there is nothing left for it to sweep. address public treasury; event ConvertedVirtualToRVirtual( @@ -38,11 +39,6 @@ contract RVirtualConverter is address indexed rVirtualReceiver, uint256 amount ); - event AdminWalletUpdated( - address indexed previousAdminWallet, - address indexed newAdminWallet - ); - event VirtualWithdrawn(address adminWallet, uint256 amount); function initialize( address virtualToken_, @@ -94,22 +90,6 @@ contract RVirtualConverter is emit ConvertedVirtualToRVirtual(_msgSender(), rVirtualReceiver, delivered); } - function setAdminWallet(address adminWallet_) external onlyRole(ADMIN_ROLE) { - require(adminWallet_ != address(0), "Invalid admin wallet"); - address previousAdminWallet = adminWallet; - adminWallet = adminWallet_; - emit AdminWalletUpdated(previousAdminWallet, adminWallet_); - } - - /// @notice Withdraw accumulated VIRTUAL out of this contract. Only VIRTUAL - there is - /// intentionally no withdrawal path for rVirtual or any other token here, so - /// adminWallet is never exposed to rVirtual's transfer tax. - function withdrawVirtual(uint256 amount) external nonReentrant { - require(_msgSender() == adminWallet, "Only admin wallet"); - IERC20(virtualToken).safeTransfer(adminWallet, amount); - emit VirtualWithdrawn(adminWallet, amount); - } - function _authorizeUpgrade( address newImplementation ) internal override onlyRole(ADMIN_ROLE) {} diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index 344a7272..dfdd6031 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -9,10 +9,10 @@ const { parseEther } = ethers; describe("RVirtualConverter", function () { let virtual, rVirtual, converter; - let deployer, user, other, adminWallet, treasury; + let deployer, user, other, treasury; before(async function () { - [deployer, user, other, adminWallet, treasury] = await ethers.getSigners(); + [deployer, user, other, treasury] = await ethers.getSigners(); }); beforeEach(async function () { @@ -195,63 +195,13 @@ describe("RVirtualConverter", function () { }); }); - describe("setAdminWallet events (L-10 fix)", function () { - it("should emit both the previous and new admin wallet, indexed", async function () { - await expect(converter.setAdminWallet(adminWallet.address)) - .to.emit(converter, "AdminWalletUpdated") - .withArgs(ethers.ZeroAddress, adminWallet.address); - - await expect(converter.setAdminWallet(other.address)) - .to.emit(converter, "AdminWalletUpdated") - .withArgs(adminWallet.address, other.address); - }); - }); - - describe("withdrawVirtual", function () { - beforeEach(async function () { - await converter.setAdminWallet(adminWallet.address); - // VIRTUAL no longer accumulates in the converter via conversions (see L-02 fix - // above) - donate directly so withdrawVirtual's own mechanics can still be - // exercised in isolation. - await virtual.transfer(converter.target, parseEther("100")); - }); - - it("should allow only adminWallet to withdraw the accumulated VIRTUAL", async function () { - await expect( - converter.connect(adminWallet).withdrawVirtual(parseEther("100")) - ) - .to.emit(converter, "VirtualWithdrawn") - .withArgs(adminWallet.address, parseEther("100")); - - expect(await virtual.balanceOf(adminWallet.address)).to.be.equal(parseEther("100")); - expect(await virtual.balanceOf(converter.target)).to.be.equal(0); - }); - - it("should allow withdrawing up to the full current balance, no reserve floor", async function () { - const full = await virtual.balanceOf(converter.target); - await expect(converter.connect(adminWallet).withdrawVirtual(full)).to.not.be.reverted; - expect(await virtual.balanceOf(converter.target)).to.be.equal(0); - }); - - it("should reject withdrawal from anyone other than adminWallet", async function () { - await expect( - converter.connect(user).withdrawVirtual(parseEther("100")) - ).to.be.revertedWith("Only admin wallet"); - await expect( - converter.connect(deployer).withdrawVirtual(parseEther("100")) - ).to.be.revertedWith("Only admin wallet"); - }); - - it("should reject non-admin-role setting of adminWallet", async function () { - await expect( - converter.connect(user).setAdminWallet(other.address) - ).to.be.reverted; - }); - }); - - it("should provide no path to withdraw rVirtual or any other token", async function () { - // The contract intentionally only exposes withdrawVirtual() - there is no generic - // rescue/withdraw function for rVirtual or arbitrary tokens. + it("should provide no VIRTUAL/rVirtual sweep mechanism at all (I-06 fix)", async function () { + // withdrawVirtual()/adminWallet/setAdminWallet were removed entirely once L-02's + // treasury routing made them unnecessary - VIRTUAL is never custodied by this + // contract, so there is nothing left for an admin-controlled sweep to reach. + expect(converter.withdrawVirtual).to.be.undefined; + expect(converter.setAdminWallet).to.be.undefined; + expect(converter.adminWallet).to.be.undefined; expect(converter.withdrawRVirtual).to.be.undefined; expect(converter.rescueToken).to.be.undefined; expect(converter.recoverToken).to.be.undefined; From b9a2ea175e597ad5b6fca2f83622e9041b33cb83 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 11:11:33 +0800 Subject: [PATCH 11/13] fix(L-12): disable initializers on the raw implementation contracts Add constructor() { _disableInitializers(); } to both RVirtualConverter and veVirtual, matching the standard OZ upgradeable hardening pattern already used by every other upgradeable contract in this repo (AgentVeToken, Bonding, and 35+ others). Without this, the implementation contract itself (as opposed to any proxy pointing to it) could be initialized directly by an attacker, granting them DEFAULT_ADMIN_ROLE/ADMIN_ROLE over the naked implementation. Addresses AUDIT_REPORT.md finding L-12. --- contracts/token/RVirtualConverter.sol | 5 +++++ contracts/token/veVirtual.sol | 5 +++++ test/rvirtual-converter.js | 11 +++++++++++ test/vevirtual.js | 7 +++++++ 4 files changed, 28 insertions(+) diff --git a/contracts/token/RVirtualConverter.sol b/contracts/token/RVirtualConverter.sol index f47555c0..17d227c2 100644 --- a/contracts/token/RVirtualConverter.sol +++ b/contracts/token/RVirtualConverter.sol @@ -34,6 +34,11 @@ contract RVirtualConverter is /// anymore (see audit I-06) - there is nothing left for it to sweep. address public treasury; + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + event ConvertedVirtualToRVirtual( address indexed caller, address indexed rVirtualReceiver, diff --git a/contracts/token/veVirtual.sol b/contracts/token/veVirtual.sol index d149e4ab..1fae635d 100644 --- a/contracts/token/veVirtual.sol +++ b/contracts/token/veVirtual.sol @@ -35,6 +35,11 @@ contract veVirtual is uint8 public maxWeeks; + /// @custom:oz-upgrades-unsafe-allow constructor + constructor() { + _disableInitializers(); + } + event Stake( address indexed user, uint256 id, diff --git a/test/rvirtual-converter.js b/test/rvirtual-converter.js index dfdd6031..af829253 100644 --- a/test/rvirtual-converter.js +++ b/test/rvirtual-converter.js @@ -207,6 +207,17 @@ describe("RVirtualConverter", function () { expect(converter.recoverToken).to.be.undefined; }); + describe("_disableInitializers on implementation (L-12 fix)", function () { + it("should reject calling initialize() directly on the raw implementation contract", async function () { + const implAddress = await upgrades.erc1967.getImplementationAddress(converter.target); + const implementation = await ethers.getContractAt("RVirtualConverter", implAddress); + + await expect( + implementation.initialize(virtual.target, rVirtual.target, treasury.address) + ).to.be.reverted; + }); + }); + describe("UUPS upgradeability", function () { it("should upgrade to V2, run new code, upgrade back to V1, and end up with identical bytecode", async function () { const implBefore = await upgrades.erc1967.getImplementationAddress(converter.target); diff --git a/test/vevirtual.js b/test/vevirtual.js index da832ee2..0b0510a6 100644 --- a/test/vevirtual.js +++ b/test/vevirtual.js @@ -25,6 +25,13 @@ describe("veVIRTUAL", function () { veVirtual = await upgrades.deployProxy(Contract, [virtual.target, 104]); }); + it("should reject calling initialize() directly on the raw implementation contract (L-12 fix)", async function () { + const implAddress = await upgrades.erc1967.getImplementationAddress(veVirtual.target); + const implementation = await ethers.getContractAt("veVirtual", implAddress); + + await expect(implementation.initialize(virtual.target, 104)).to.be.reverted; + }); + it("should allow staking", async function () { await virtual.transfer(staker.address, parseEther("1000")); From 9139a6881f8fa82c68cf1b78a4a2ebe8351755e2 Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 11:17:07 +0800 Subject: [PATCH 12/13] test(L-12): prove an upgrade adding the constructor preserves proxy state Simulates the real-world scenario: an already-live veVirtual proxy with existing stakes/votes gets upgraded to a new implementation that adds constructor() { _disableInitializers(); }. Forces a genuinely new implementation deployment (redeployImplementation: "always") and asserts every pre-upgrade storage value (lock data, decayed balance, raw voting power) survives untouched, and that the new implementation is independently hardened against direct initialize() calls. This is empirical proof (not just theory) that adding this constructor to a contract that has already been deployed and upgraded multiple times on mainnet is safe: constructors only ever run once, at the new implementation's own deployment transaction, and never execute in the context of - or touch the storage of - the proxy. --- test/vevirtual.js | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/vevirtual.js b/test/vevirtual.js index 0b0510a6..18014e31 100644 --- a/test/vevirtual.js +++ b/test/vevirtual.js @@ -32,6 +32,58 @@ describe("veVIRTUAL", function () { await expect(implementation.initialize(virtual.target, 104)).to.be.reverted; }); + it("should preserve all existing proxy state across an upgrade that adds the L-12 constructor", async function () { + // Simulates upgrading an already-live veVirtual proxy (existing user stakes/votes) + // to a new implementation that adds `constructor() { _disableInitializers(); }`. + // Proves the constructor - which only ever runs once, at the NEW implementation's + // own deployment - never touches the proxy's storage. + await virtual.transfer(staker.address, parseEther("1000")); + await virtual.connect(staker).approve(veVirtual.target, parseEther("1000")); + await veVirtual.connect(staker).delegate(staker.address); + await veVirtual.connect(staker).stake(parseEther("300"), 52, false); + const id = (await veVirtual.locks(staker.address, 0)).id; + + const implBefore = await upgrades.erc1967.getImplementationAddress(veVirtual.target); + const balanceBefore = await veVirtual.balanceOf(staker.address); + const votesBefore = await veVirtual.getVotes(staker.address); + + // Upgrade to a fresh deployment of the SAME (current, constructor-having) contract - + // this is exactly what a real upgrade does: deploy a brand-new implementation and + // repoint the existing proxy at it. The new implementation's constructor runs during + // THIS deployment, not against the proxy. + const VeVirtualContract = await ethers.getContractFactory("veVirtual"); + const upgraded = await upgrades.upgradeProxy(veVirtual.target, VeVirtualContract, { + // Force a genuinely new implementation deployment even though the bytecode is + // identical to the current one - a real upgrade always deploys fresh, and this + // is what proves the constructor runs on a NEW address, not the old one. + redeployImplementation: "always", + }); + const implAfter = await upgrades.erc1967.getImplementationAddress(upgraded.target); + + expect(implAfter).to.not.equal(implBefore); + expect(upgraded.target).to.equal(veVirtual.target); // same proxy address throughout + + // All pre-upgrade state survives untouched. balanceOf is intentionally time-decayed + // (see L-11), so it ticks down by a negligible amount over the 1-2 blocks the + // upgrade transaction itself takes to mine - compare with a tight tolerance rather + // than exact equality. getVotes is the raw, undecayed sum and must match exactly. + expect(await upgraded.numPositions(staker.address)).to.be.equal(1); + expect((await upgraded.locks(staker.address, 0)).id).to.be.equal(id); + expect(await upgraded.balanceOf(staker.address)).to.be.closeTo( + balanceBefore, + parseEther("0.001") + ); + expect(await upgraded.getVotes(staker.address)).to.be.equal(votesBefore); + + // The functionality still works post-upgrade. + await expect(upgraded.connect(staker).stake(parseEther("50"), 52, false)).to.not.be + .reverted; + + // And the NEW implementation is independently hardened too. + const newImplementation = await ethers.getContractAt("veVirtual", implAfter); + await expect(newImplementation.initialize(virtual.target, 104)).to.be.reverted; + }); + it("should allow staking", async function () { await virtual.transfer(staker.address, parseEther("1000")); From 5a859409f715525c08486e70139fe20ff73c149b Mon Sep 17 00:00:00 2001 From: koo-virtuals Date: Thu, 30 Jul 2026 11:29:59 +0800 Subject: [PATCH 13/13] update .gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index eb15dcff..242d89d0 100644 --- a/.gitignore +++ b/.gitignore @@ -22,4 +22,5 @@ fireblocks_secret.key .cursor/ env* -lib/ \ No newline at end of file +lib/ +docs/ \ No newline at end of file