Merge branch 'master' into fix/revert-swap-from-paymaster

contracts/test/helpers/MockChainlinkAggregator.sol

@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.17;
+
+import "@openzeppelin/contracts/access/Ownable.sol";
+import "../../token/oracles/IOracleAggregator.sol";
+import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
+import "hardhat/console.sol";
+
+/**
+ * @title Mock Oracle Aggregator contract
+ * @notice DO NOT use this in any environment for use
+ */
+contract MockChainlinkOracleAggregator is Ownable, IOracleAggregator {
+    struct TokenInfo {
+        /* Number of decimals represents the precision of the price returned by the feed. For example, 
+     a price of $100.50 might be represented as 100500000000 in the contract, with 9 decimal places 
+     of precision */
+        uint8 decimals;
+        // uint8 tokenDecimals;
+        bool dataSigned;
+        address callAddress;
+        bytes callData;
+    }
+
+    mapping(address => TokenInfo) internal tokensInfo;
+
+    constructor(address _owner) {
+        _transferOwnership(_owner);
+    }
+
+    /**
+     * @dev set price feed information for specific feed
+     * @param callAddress price feed / derived price feed address to call
+     * @param decimals decimals (precision) defined in this price feed
+     * @param callData function selector which will be used to query price data
+     * @param signed if the feed may return result as signed integrer
+     */
+    function setTokenOracle(
+        address token,
+        address callAddress,
+        uint8 decimals,
+        bytes calldata callData,
+        bool signed
+    ) external onlyOwner {
+        require(
+            callAddress != address(0),
+            "ChainlinkOracleAggregator:: call address can not be zero"
+        );
+        require(
+            token != address(0),
+            "ChainlinkOracleAggregator:: token address can not be zero"
+        );
+        tokensInfo[token].callAddress = callAddress;
+        tokensInfo[token].decimals = decimals;
+        tokensInfo[token].callData = callData;
+        tokensInfo[token].dataSigned = signed;
+    }
+
+    /**
+     * @dev query deciamls used by set feed for specific token
+     * @param token ERC20 token address
+     */
+    function getTokenOracleDecimals(
+        address token
+    ) external view returns (uint8 _tokenOracleDecimals) {
+        _tokenOracleDecimals = tokensInfo[token].decimals;
+    }
+
+    /**
+     * @dev query price feed
+     * @param token ERC20 token address
+     */
+    function getTokenPrice(
+        address token
+    ) external view returns (uint256 tokenPrice) {
+        // usually token / native (depends on price feed)
+        tokenPrice = _getTokenPrice(token);
+    }
+
+    /**
+     * @dev exchangeRate : each aggregator implements this method based on how it sources the quote/price
+     * @notice here it is token / native sourced from chainlink so in order to get defined exchangeRate we inverse the feed
+     * @param token ERC20 token address
+     */
+    function getTokenValueOfOneNativeToken(
+        address token
+    ) external view virtual returns (uint256 exchangeRate) {
+        // we'd actually want eth / token
+        uint256 tokenPriceUnadjusted = _getTokenPrice(token);
+        uint8 _tokenOracleDecimals = tokensInfo[token].decimals;
+        exchangeRate =
+            ((10 ** _tokenOracleDecimals) *
+                (10 ** IERC20Metadata(token).decimals())) /
+            tokenPriceUnadjusted;
+    }
+
+    // Making explicit revert or make use of stale price feed which reverts
+    // like done in below function and the test case
+    /*function _getTokenPrice(
+        address token
+    ) internal view returns (uint256 tokenPriceUnadjusted) {
+        bool success = false;
+        require(success, "ChainlinkOracleAggregator:: query failed");
+    }*/
+
+    function _getTokenPrice(
+        address token
+    ) internal view returns (uint256 tokenPriceUnadjusted) {
+        (bool success, bytes memory ret) = tokensInfo[token]
+            .callAddress
+            .staticcall(tokensInfo[token].callData);
+        console.log("price feed reverted here? success = %s ", success);
+        require(success, "ChainlinkOracleAggregator:: query failed");
+        if (tokensInfo[token].dataSigned) {
+            tokenPriceUnadjusted = uint256(abi.decode(ret, (int256)));
+        } else {
+            tokenPriceUnadjusted = abi.decode(ret, (uint256));
+        }
+    }
+}

contracts/test/helpers/MockStalePriceFeed.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.17;
+
+import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
+
+error WrongPrice();
+
+//@review againsnt chainlink reference PriceConverter https://docs.chain.link/docs/get-the-latest-price/
+//@review decimals for individual feeds
+contract MockStalePriceFeed {
+    AggregatorV3Interface internal priceFeed1;
+    AggregatorV3Interface internal priceFeed2;
+
+    constructor() {
+        // todo // do not hard code // polygon values
+        priceFeed1 = AggregatorV3Interface(
+            0xAB594600376Ec9fD91F8e885dADF0CE036862dE0
+        ); // matic usd
+        priceFeed2 = AggregatorV3Interface(
+            0xfE4A8cc5b5B2366C1B58Bea3858e81843581b2F7
+        ); // usdc usd
+    }
+
+    function decimals() public view returns (uint8) {
+        return 18;
+    }
+
+    function getThePrice() public view returns (int) {
+        // Always using decimals 18 for derived price feeds
+        revert WrongPrice();
+    }
+}

contracts/token/BiconomyTokenPaymaster.sol

@@ -218,16 +218,14 @@ contract BiconomyTokenPaymaster is
         address _token,
         address _oracleAggregator
     ) internal view virtual returns (uint256 exchangeRate) {
-        // get price from chosen oracle aggregator.
-        bytes memory _data = abi.encodeWithSelector(
-            IOracleAggregator.getTokenValueOfOneNativeToken.selector,
-            _token
-        );
-        (bool success, bytes memory returndata) = address(_oracleAggregator)
-            .staticcall(_data);
-        exchangeRate = 0; // this is assigned for fallback
-        if (success) {
-            exchangeRate = abi.decode(returndata, (uint256));
+        try
+            IOracleAggregator(_oracleAggregator).getTokenValueOfOneNativeToken(
+                _token
+            )
+        returns (uint256 exchangeRate) {
+            return exchangeRate;
+        } catch {
+            return 0;
         }
     }
 
@@ -373,6 +371,11 @@ contract BiconomyTokenPaymaster is
             bytes calldata signature
         )
     {
+        // paymasterAndData.length should be at least SIGNATURE_OFFSET + 65 (checked separate)
+        require(
+            paymasterAndData.length >= SIGNATURE_OFFSET,
+            "BTPM: Invalid length for paymasterAndData"
+        );
         priceSource = ExchangeRateSource(
             uint8(
                 bytes1(paymasterAndData[VALID_PND_OFFSET - 1:VALID_PND_OFFSET])

contracts/token/oracles/ChainlinkOracleAggregator.sol

@@ -42,11 +42,11 @@ contract ChainlinkOracleAggregator is Ownable, IOracleAggregator {
     ) external onlyOwner {
         require(
             callAddress != address(0),
-            "OracleAggregator:: call address can not be zero"
+            "ChainlinkOracleAggregator:: call address can not be zero"
         );
         require(
             token != address(0),
-            "OracleAggregator:: token address can not be zero"
+            "ChainlinkOracleAggregator:: token address can not be zero"
         );
         tokensInfo[token].callAddress = callAddress;
         tokensInfo[token].decimals = decimals;
@@ -98,6 +98,7 @@ contract ChainlinkOracleAggregator is Ownable, IOracleAggregator {
         (bool success, bytes memory ret) = tokensInfo[token]
             .callAddress
             .staticcall(tokensInfo[token].callData);
+        require(success, "ChainlinkOracleAggregator:: query failed");
         if (tokensInfo[token].dataSigned) {
             tokenPriceUnadjusted = uint256(abi.decode(ret, (int256)));
         } else {

test/token-paymaster/biconomy-token-paymaster-specs.ts

@@ -393,6 +393,64 @@ describe("Biconomy Token Paymaster", function () {
       .withArgs(0, "AA33 reverted: BTPM: invalid signature length in paymasterAndData");
     });
 
+    it("should revert (from EntryPoint) on invalid paymaster and data length", async ()  => {
+
+      const userSCW: any = BiconomyAccountImplementation__factory.connect(walletAddress, deployer)
+
+      const userOp = await fillAndSign(
+        {
+          sender: walletAddress,
+          verificationGasLimit: 200000,
+          // initCode: hexConcat([walletFactory.address, deploymentData]),
+          // nonce: 0,
+          callData: encodeERC20Approval(
+            userSCW,
+            token,
+            paymasterAddress,
+            ethers.constants.MaxUint256
+          ),
+          paymasterAndData: '0x1234',
+        },
+        walletOwner,
+        entryPoint,
+        "nonce"
+      );
+
+      await expect(entryPoint.callStatic.simulateValidation(userOp))
+      .to.be.revertedWith("AA93 invalid paymasterAndData")
+    });
+
+    it("should revert (from Paymaster) on invalid paymaster and data length", async ()  => {
+
+      const userSCW: any = BiconomyAccountImplementation__factory.connect(walletAddress, deployer)
+
+      const userOp = await fillAndSign(
+        {
+          sender: walletAddress,
+          verificationGasLimit: 200000,
+          // initCode: hexConcat([walletFactory.address, deploymentData]),
+          // nonce: 0,
+          callData: encodeERC20Approval(
+            userSCW,
+            token,
+            paymasterAddress,
+            ethers.constants.MaxUint256
+          ),
+          paymasterAndData: ethers.utils.hexConcat([
+            paymasterAddress,
+            '0x1234',
+          ]),
+        },
+        walletOwner,
+        entryPoint,
+        "nonce"
+      );
+
+      await expect(entryPoint.callStatic.simulateValidation(userOp))
+      .to.be.revertedWithCustomError(entryPoint, "FailedOp")
+      .withArgs(0, "AA33 reverted: BTPM: Invalid length for paymasterAndData");
+    });
+
     it("should revert on invalid signature", async ()  => {
 
       const userSCW: any = BiconomyAccountImplementation__factory.connect(walletAddress, deployer)

test/token-paymaster/btpm-coverage-specs.ts

@@ -48,12 +48,13 @@ export async function deployEntryPoint(
 
 export const encodePaymasterData = (
   feeToken = ethers.constants.AddressZero,
+  oracleAggregator = ethers.constants.AddressZero,
   exchangeRate: BigNumberish = ethers.constants.Zero,
   fee: BigNumberish = ethers.constants.Zero
 ) => {
   return ethers.utils.defaultAbiCoder.encode(
-    ["uint48", "uint48", "address", "uint256", "uint256"],
-    [MOCK_VALID_UNTIL, MOCK_VALID_AFTER, feeToken, exchangeRate, fee]
+    ["uint48", "uint48", "address", "address", "uint256", "uint256"],
+    [MOCK_VALID_UNTIL, MOCK_VALID_AFTER, feeToken, oracleAggregator, exchangeRate, fee]
   );
 };