Merge pull request #263 from sprintertech/feat/stashdex-origin-gating

Add basic tx.origin gating to StashDex

Author: mpetrunic

contracts/StashDex.sol

@@ -22,6 +22,7 @@ contract StashDex is AccessControlUpgradeable {
     bytes32 public constant CONFIG_ROLE = "CONFIG_ROLE";
     bytes32 public constant PAUSER_ROLE = "PAUSER_ROLE";
     bytes32 public constant FORWARD_ROLE = "FORWARD_ROLE";
+    bytes32 public constant USER_ROLE = "USER_ROLE";
 
     uint256 private constant BPS = 10_000;
 
@@ -72,6 +73,7 @@ contract StashDex is AccessControlUpgradeable {
     error PoolNotConfigured();
     error EnforcedPause();
     error ExpectedPause();
+    error Unauthorized();
 
     modifier whenNotPaused() {
         require(!_getStorage().paused, EnforcedPause());
@@ -137,6 +139,9 @@ contract StashDex is AccessControlUpgradeable {
         uint256 amountOut,
         address recipient
     ) public whenNotPaused() {
+        // Checking the role on tx.origin instead of _msgSender() because StashDex is called through another contract.
+        /* solhint-disable avoid-tx-origin */
+        require(hasRole(USER_ROLE, tx.origin), Unauthorized());
         StashDexStorage storage $ = _getStorage();
         RouteConfig memory route = $.routes[tokenIn][tokenOut];
         require(route.allowed, RouteNotAllowed());

coverage-baseline.json

@@ -1,6 +1,6 @@
 {
   "lines": "99.71",
   "functions": "99.69",
-  "branches": "93.68",
+  "branches": "93.69",
   "statements": "99.71"
 }

deployments/deploy-ethereum-stage.log

@@ -460,3 +460,23 @@ Threshold met (1/1). Executing on-chain...
 Executed. On-chain TX hash: 0xfffdba1d5234084edcb4ae47945f65fa30ab69bfe6b60e41e1bf582ca0ad797b
 0xfffdba1d5234084edcb4ae47945f65fa30ab69bfe6b60e41e1bf582ca0ad797b
 StashDex upgraded.
+
+Safe 0xA8eeA59b4A17CE2689E57B4dE9e825FD25705414: verified 0xdeC18ca09f163C841Fc3d44237bfD0EBbC8414dE as owner.
+Deployment ID: MVP
+Upgrade ID: STASHDEX_ORIGIN_GATING
+Upgrading StashDex
+Using config for: stage, ETHEREUM
+Deployer        : 0xdeC18ca09f163C841Fc3d44237bfD0EBbC8414dE
+DEPLOYER_ADDRESS: 0xdBD91aD22bE5304e385b7b0A2Cfe91164e416e11
+StashStablecoinDex proxy: 0x125Bf891F832c3A1422E24f4Ca7ef314c0858740
+Oracle: 0x4666013984fD7a624aa8BFADD0E95456870dADEc
+Receiver: 0x697ECA1cae710FA0348e2173900e6C09b180C35b
+New StashDex implementation deployed to 0x4eaf3771c7C272e0Ec838E0c01b24E47Cb0E36bf
+Sending StashDex upgrade transaction.
+Transaction proposed to Safe 0xA8eeA59b4A17CE2689E57B4dE9e825FD25705414. Safe TX hash: 0xbf9b7b577e9f9d4808e62ea67456da7840bcf9303c7c1b71ac730d58b7be30ef
+Threshold met (1/1). Executing on-chain...
+Executed. On-chain TX hash: 0x7eb0f8ac07936f059b99f383b8e6932517e50e5b1eb911720f9eaf5744801fe1
+0x7eb0f8ac07936f059b99f383b8e6932517e50e5b1eb911720f9eaf5744801fe1
+StashDex upgraded.
+USER_ROLE granted to 0xeEa9b38E8C54B52F7387a46A7C81173065fa5A10
+Executed. On-chain TX hash: 0x639ee7cd8d1d5b274ca35df9c9a3b6345cc4c64426a04652f07fa011903e1926

test/StashDex.ts

@@ -4,7 +4,7 @@ import hre from "hardhat";
 import {deploy, getContractAt, setupTests} from "./helpers";
 import {addressToBytes32, ZERO_ADDRESS, DEFAULT_ADMIN_ROLE} from "../scripts/common";
 import {
-  TestUSDC, TestWETH, PaxosOracle, StashDex, TestLiquidityPool, TransparentUpgradeableProxy,
+  TestUSDC, TestWETH, PaxosOracle, StashDex, TestLiquidityPool, TransparentUpgradeableProxy, MockBorrowSwap,
 } from "../typechain-types";
 
 describe("StashDex", function () {
@@ -42,11 +42,13 @@ describe("StashDex", function () {
     const CONFIG_ROLE = await stashDex.CONFIG_ROLE();
     const PAUSER_ROLE = await stashDex.PAUSER_ROLE();
     const FORWARD_ROLE = await stashDex.FORWARD_ROLE();
+    const USER_ROLE = await stashDex.USER_ROLE();
+    await stashDex.connect(admin).grantRole(USER_ROLE, user);
 
     return {
       deployer, admin, configAdmin, pauser, forwarder, user, user2, processor, receiver,
       tokenA, tokenB, tokenC, usdcRef, oracle, pool, stashDex, stashDexImpl,
-      CONFIG_ROLE, PAUSER_ROLE, FORWARD_ROLE, USDC, WETH,
+      CONFIG_ROLE, PAUSER_ROLE, FORWARD_ROLE, USER_ROLE, USDC, WETH,
     };
   };
 
@@ -346,6 +348,60 @@ describe("StashDex", function () {
     });
   });
 
+  describe("USER_ROLE", function () {
+    it("swap reverts Unauthorized when tx.origin lacks USER_ROLE", async function () {
+      const {stashDex, user2, tokenA, tokenB, USDC} = await loadFixture(deployAll);
+      await expect(stashDex.connect(user2).swap(tokenA, tokenB, 10_000n * USDC, 9_997n * USDC, user2))
+        .to.be.revertedWithCustomError(stashDex, "Unauthorized");
+    });
+
+    it("exchange(5 params) reverts Unauthorized when tx.origin lacks USER_ROLE", async function () {
+      const {stashDex, user2, tokenA, tokenB, USDC} = await loadFixture(deployAll);
+      await expect(stashDex.connect(user2)["exchange(uint256,uint256,uint256,uint256,address)"](
+        BigInt(await tokenA.getAddress()), BigInt(await tokenB.getAddress()), 10_000n * USDC, 9_997n * USDC, user2
+      )).to.be.revertedWithCustomError(stashDex, "Unauthorized");
+    });
+
+    it("exchange(4 params) reverts Unauthorized when tx.origin lacks USER_ROLE", async function () {
+      const {stashDex, user2, tokenA, tokenB, USDC} = await loadFixture(deployAll);
+      await expect(stashDex.connect(user2)["exchange(uint256,uint256,uint256,uint256)"](
+        BigInt(await tokenA.getAddress()), BigInt(await tokenB.getAddress()), 10_000n * USDC, 9_997n * USDC
+      )).to.be.revertedWithCustomError(stashDex, "Unauthorized");
+    });
+
+    it("tx.origin is checked: user (USER_ROLE) succeeds via mock, user2 reverts via mock", async function () {
+      const {stashDex, configAdmin, user, user2, tokenA, tokenB, pool, processor, USDC} = await loadFixture(deployAll);
+      const mock = (await deploy("MockBorrowSwap", user)) as MockBorrowSwap;
+
+      await stashDex.connect(configAdmin).setPool(tokenB, pool);
+      await stashDex.connect(configAdmin).setRoute({tokenIn: tokenA, tokenOut: tokenB, feeBps: 3, processor});
+      await tokenA.mint(mock, 10_000n * USDC);
+      await tokenB.mint(pool, 9_997n * USDC);
+
+      // mock (msg.sender) approves stashDex to pull its tokenA during swap
+      const approveData = await tokenA.approve.populateTransaction(stashDex, 10_000n * USDC);
+      await mock.connect(user).callBorrow(tokenA, approveData.data);
+
+      const swapData = await stashDex.swap.populateTransaction(
+        tokenA.target, tokenB.target, 10_000n * USDC, 9_997n * USDC, user.address,
+      );
+
+      // user2 has no USER_ROLE → tx.origin check fails → Unauthorized
+      await expect(mock.connect(user2).callBorrowBubbleRevert(stashDex, swapData.data))
+        .to.be.revertedWithCustomError(stashDex, "Unauthorized");
+
+      // user has USER_ROLE → tx.origin check passes → succeeds
+      const tx = await mock.connect(user).callBorrow(stashDex, swapData.data);
+      await expect(tx).to.emit(stashDex, "Swapped")
+        .withArgs(tokenA.target, tokenB.target, 10_000n * USDC, 9_997n * USDC, user.address);
+
+      expect(await tokenA.balanceOf(mock)).to.equal(0n);
+      expect(await tokenA.balanceOf(processor)).to.equal(10_000n * USDC);
+      expect(await tokenB.balanceOf(user)).to.equal(9_997n * USDC);
+      expect(await tokenB.balanceOf(pool)).to.equal(0n);
+    });
+  });
+
   describe("swap", function () {
     it("reverts RouteNotAllowed when no route is configured", async function () {
       const {stashDex, user, tokenA, tokenB, USDC} = await loadFixture(deployAll);

test/StashDexProcessor.ts

@@ -50,6 +50,7 @@ describe("StashDexProcessor", function () {
     await usdc.mint(pool, debtAmount);
     await tokenA.mint(user, debtAmount);
     await tokenA.connect(user).approve(stashDex, debtAmount);
+    await stashDex.connect(admin).grantRole(await stashDex.USER_ROLE(), user);
     await stashDex.connect(user).swap(tokenA, usdc, debtAmount, debtAmount, user);
 
     // Deploy StashDexProcessor proxy