From 37ee56afd4728b556b6f2eccb5865e778a6d7774 Mon Sep 17 00:00:00 2001 From: Ankur Dubey Date: Fri, 17 Nov 2023 12:20:21 +0530 Subject: [PATCH] gas opt: token paymaster and oracle aggregator --- .gas-snapshot | 29 ++--- contracts/token/BiconomyTokenPaymaster.sol | 101 ++++++++++++++---- .../oracles/ChainlinkOracleAggregator.sol | 44 +++++--- 3 files changed, 124 insertions(+), 50 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index c62f117..dd4b98c 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,14 +1,15 @@ -TokenPaymasterTest:test2TokenPaymasterFailInvalidPaymasteDataLength() (gas: 278584) -TokenPaymasterTest:testCall() (gas: 313171) -TokenPaymasterTest:testCheckStates() (gas: 18600) -TokenPaymasterTest:testDeploy() (gas: 1604349) -TokenPaymasterTest:testOwnershipTransfer() (gas: 23443) -TokenPaymasterTest:testParsePaymasterData() (gas: 621) -TokenPaymasterTest:testTokenPaymasterFailHighPriceMarkup() (gas: 300775) -TokenPaymasterTest:testTokenPaymasterFailInvalidPMSignature() (gas: 283184) -TokenPaymasterTest:testTokenPaymasterFailInvalidPMSignatureLength() (gas: 283496) -TokenPaymasterTest:testTokenPaymasterFailInvalidPaymasteDataLength() (gas: 221406) -TokenPaymasterTest:testTokenPaymasterFailWrongPMSignature() (gas: 295014) -TokenPaymasterTest:testTokenPaymasterRefund() (gas: 399655) -TokenPaymasterTest:testWithdrawERC20(uint256) (runs: 256, μ: 61802, ~: 64060) -TokenPaymasterTest:testWithdrawERC20FailNotOwner(uint256) (runs: 256, μ: 48871, ~: 51000) \ No newline at end of file +TokenPaymasterTest:test2TokenPaymasterFailInvalidPaymasteDataLength() (gas: 292916) +TokenPaymasterTest:testCall() (gas: 327004) +TokenPaymasterTest:testCheckStates() (gas: 18629) +TokenPaymasterTest:testDecode() (gas: 11952) +TokenPaymasterTest:testDeploy() (gas: 1642864) +TokenPaymasterTest:testOwnershipTransfer() (gas: 23457) +TokenPaymasterTest:testParsePaymasterData() (gas: 658) +TokenPaymasterTest:testTokenPaymasterFailHighPriceMarkup() (gas: 315354) +TokenPaymasterTest:testTokenPaymasterFailInvalidPMSignature() (gas: 297563) +TokenPaymasterTest:testTokenPaymasterFailInvalidPMSignatureLength() (gas: 297878) +TokenPaymasterTest:testTokenPaymasterFailInvalidPaymasteDataLength() (gas: 230920) +TokenPaymasterTest:testTokenPaymasterFailWrongPMSignature() (gas: 309240) +TokenPaymasterTest:testTokenPaymasterRefund() (gas: 413708) +TokenPaymasterTest:testWithdrawERC20(uint256) (runs: 256, μ: 61908, ~: 64071) +TokenPaymasterTest:testWithdrawERC20FailNotOwner(uint256) (runs: 256, μ: 48851, ~: 50980) \ No newline at end of file diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index 85e6098..a4d76b7 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -15,6 +15,7 @@ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import "../utils/SafeTransferLib.sol"; import {TokenPaymasterErrors} from "./TokenPaymasterErrors.sol"; import "@openzeppelin/contracts/utils/Address.sol"; +import "hardhat/console.sol"; // Biconomy Token Paymaster /** @@ -232,11 +233,13 @@ contract BiconomyTokenPaymaster is address _token, address _oracleAggregator ) internal view virtual returns (uint256) { + uint256 gas = gasleft(); try IOracleAggregator(_oracleAggregator).getTokenValueOfOneNativeToken( _token ) returns (uint256 exchangeRate) { + console.log("gas used for exchangePrice: %s", gas - gasleft()); return exchangeRate; } catch { return 0; @@ -349,8 +352,8 @@ contract BiconomyTokenPaymaster is abi.encode( userOp.getSender(), userOp.nonce, - keccak256(userOp.initCode), - keccak256(userOp.callData), + userOp.initCode, + userOp.callData, userOp.callGasLimit, userOp.verificationGasLimit, userOp.preVerificationGas, @@ -451,6 +454,7 @@ contract BiconomyTokenPaymaster is ); // review: in this method try to resolve stack too deep (though via-ir is good enough) + uint256 gas = gasleft(); ( ExchangeRateSource priceSource, uint48 validUntil, @@ -461,6 +465,7 @@ contract BiconomyTokenPaymaster is uint32 priceMarkup, bytes calldata signature ) = parsePaymasterAndData(userOp.paymasterAndData); + console.log("gas used for parsePmd: %s", gas - gasleft()); // we only "require" it here so that the revert reason on invalid signature will be of "VerifyingPaymaster", and not "ECDSA" require( @@ -468,6 +473,7 @@ contract BiconomyTokenPaymaster is "BTPM: invalid signature length in paymasterAndData" ); + gas = gasleft(); bytes32 _hash = getHash( userOp, priceSource, @@ -478,6 +484,7 @@ contract BiconomyTokenPaymaster is exchangeRate, priceMarkup ).toEthSignedMessageHash(); + console.log("gas used for getHash: %s", gas - gasleft()); context = ""; @@ -511,6 +518,7 @@ contract BiconomyTokenPaymaster is "BTPM: account does not have enough token balance" ); + gas = gasleft(); context = abi.encode( account, feeToken, @@ -520,6 +528,15 @@ contract BiconomyTokenPaymaster is priceMarkup, userOpHash ); + console.log("gas used for context encode: %s", gas - gasleft()); + + // console.log("account: %s", account); + // console.log("feeToken: %s", address(feeToken)); + // console.log("oracleAggregator: %s", oracleAggregator); + // console.log("priceSource: %s", uint8(priceSource)); + // console.log("exchangeRate: %s", exchangeRate); + // console.log("priceMarkup: %s", priceMarkup); + // console.log("userOpHash: %s", uint256(userOpHash)); return ( context, @@ -538,26 +555,66 @@ contract BiconomyTokenPaymaster is bytes calldata context, uint256 actualGasCost ) internal virtual override { - ( - address account, - IERC20 feeToken, - address oracleAggregator, - ExchangeRateSource priceSource, - uint256 exchangeRate, - uint32 priceMarkup, - bytes32 userOpHash - ) = abi.decode( - context, - ( - address, - IERC20, - address, - ExchangeRateSource, - uint256, - uint32, - bytes32 - ) - ); + uint256 gas = gasleft(); + // ( + // address account, + // IERC20 feeToken, + // address oracleAggregator, + // ExchangeRateSource priceSource, + // uint256 exchangeRate, + // uint32 priceMarkup, + // bytes32 userOpHash + // ) = abi.decode( + // context, + // ( + // address, + // IERC20, + // address, + // ExchangeRateSource, + // uint256, + // uint32, + // bytes32 + // ) + // ); + address account; + IERC20 feeToken; + address oracleAggregator; + ExchangeRateSource priceSource; + uint256 exchangeRate; + uint32 priceMarkup; + bytes32 userOpHash; + assembly ("memory-safe") { + let offset := context.offset + + account := calldataload(context.offset) + offset := add(offset, 0x20) + + feeToken := calldataload(offset) + offset := add(offset, 0x20) + + oracleAggregator := calldataload(offset) + offset := add(offset, 0x20) + + priceSource := calldataload(offset) + offset := add(offset, 0x20) + + exchangeRate := calldataload(offset) + offset := add(offset, 0x20) + + priceMarkup := calldataload(offset) + offset := add(offset, 0x20) + + userOpHash := calldataload(offset) + } + console.log("gas used for context decode: %s", gas - gasleft()); + + // console.log("account: %s", account); + // console.log("feeToken: %s", address(feeToken)); + // console.log("oracleAggregator: %s", oracleAggregator); + // console.log("priceSource: %s", uint8(priceSource)); + // console.log("exchangeRate: %s", exchangeRate); + // console.log("priceMarkup: %s", priceMarkup); + // console.log("userOpHash: %s", uint256(userOpHash)); uint256 effectiveExchangeRate = exchangeRate; diff --git a/contracts/token/oracles/ChainlinkOracleAggregator.sol b/contracts/token/oracles/ChainlinkOracleAggregator.sol index bffccdf..387b1ee 100644 --- a/contracts/token/oracles/ChainlinkOracleAggregator.sol +++ b/contracts/token/oracles/ChainlinkOracleAggregator.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.20; import "@openzeppelin/contracts/access/Ownable.sol"; import "./IOracleAggregator.sol"; import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; +import {DerivedPriceFeed} from "./DerivedPriceFeed.sol"; +import "hardhat/console.sol"; /** * @title Primary Oracle Aggregator contract used to maintain price feeds for chainlink supported tokens. @@ -72,7 +74,7 @@ contract ChainlinkOracleAggregator is Ownable, IOracleAggregator { address token ) external view returns (uint256 tokenPrice) { // usually token / native (depends on price feed) - tokenPrice = _getTokenPrice(token); + (tokenPrice, ) = _getTokenPriceAndDecimals(token); } /** @@ -84,26 +86,40 @@ contract ChainlinkOracleAggregator is Ownable, IOracleAggregator { address token ) external view virtual returns (uint256 exchangeRate) { // we'd actually want eth / token - uint256 tokenPriceUnadjusted = _getTokenPrice(token); - uint8 _tokenOracleDecimals = tokensInfo[token].decimals; + ( + uint256 tokenPriceUnadjusted, + uint8 tokenOracleDecimals + ) = _getTokenPriceAndDecimals(token); exchangeRate = - ((10 ** _tokenOracleDecimals) * - (10 ** IERC20Metadata(token).decimals())) / + 10 ** (tokenOracleDecimals + IERC20Metadata(token).decimals()) / tokenPriceUnadjusted; } - function _getTokenPrice( + function _getTokenPriceAndDecimals( address token - ) internal view returns (uint256 tokenPriceUnadjusted) { + ) + internal + view + returns (uint256 tokenPriceUnadjusted, uint8 tokenOracleDecimals) + { // Note // If the callData is for latestAnswer, it could be for latestRoundData and then validateRound and extract price then - (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))); + TokenInfo storage tokenInfo = tokensInfo[token]; + tokenOracleDecimals = tokenInfo.decimals; + // (bool success, bytes memory ret) = tokenInfo.callAddress.staticcall( + // tokenInfo.callData + // ); + // require(success, "ChainlinkOracleAggregator:: query failed"); + // if (tokenInfo.dataSigned) { + // tokenPriceUnadjusted = uint256(abi.decode(ret, (int256))); + // } else { + // tokenPriceUnadjusted = abi.decode(ret, (uint256)); + // } + if (tokenInfo.dataSigned) { + tokenPriceUnadjusted = uint256( + DerivedPriceFeed(tokenInfo.callAddress).getThePrice() + ); } else { - tokenPriceUnadjusted = abi.decode(ret, (uint256)); + // ?? } } }