From ab671eceb23391b50b9f42010cc187401d0889b5 Mon Sep 17 00:00:00 2001 From: livingrockrises <90545960+livingrockrises@users.noreply.github.com> Date: Mon, 4 Dec 2023 18:24:16 +0400 Subject: [PATCH] code refactor + potential new event with reduced params --- contracts/token/BiconomyTokenPaymaster.sol | 53 ++++++------------- contracts/token/oracles/OracleAggregator.sol | 45 +++++++--------- .../oracle-aggregator-specs.ts | 4 +- .../oracle-aggregator-specs.ts | 4 +- 4 files changed, 41 insertions(+), 65 deletions(-) diff --git a/contracts/token/BiconomyTokenPaymaster.sol b/contracts/token/BiconomyTokenPaymaster.sol index e364d7d..3b9143e 100644 --- a/contracts/token/BiconomyTokenPaymaster.sol +++ b/contracts/token/BiconomyTokenPaymaster.sol @@ -110,7 +110,7 @@ contract BiconomyTokenPaymaster is * Designed to enable tracking how much fees were charged from the sender and in which ERC20 token * More information can be emitted like exchangeRate used, what was the source of exchangeRate etc*/ // priceMarkup = Multiplier value to calculate markup, 1e6 means 1x multiplier = No markup - event TokenPaymasterOperation( + /*event TokenPaymasterOperation( address indexed sender, address indexed token, uint256 indexed totalCharge, @@ -118,6 +118,14 @@ contract BiconomyTokenPaymaster is bytes32 userOpHash, uint256 exchangeRate, ExchangeRateSource priceSource + );*/ + + // Review can add exchangeRate + event UserOperationSponsored( + address indexed sender, + address indexed token, + uint256 indexed totalCharge, + uint256 actualGasCost ); /** @@ -487,17 +495,9 @@ contract BiconomyTokenPaymaster is feeToken, priceSource, exchangeRate, - priceMarkup, - userOpHash + 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("priceSource: %s", uint8(priceSource)); - // console.log("exchangeRate: %s", exchangeRate); - // console.log("priceMarkup: %s", priceMarkup); - // console.log("userOpHash: %s", uint256(userOpHash)); return ( context, @@ -516,32 +516,12 @@ contract BiconomyTokenPaymaster is bytes calldata context, uint256 actualGasCost ) internal virtual override { - // uint256 gas = gasleft(); - // ( - // address account, - // IERC20 feeToken, - // ExchangeRateSource priceSource, - // uint256 exchangeRate, - // uint32 priceMarkup, - // bytes32 userOpHash - // ) = abi.decode( - // context, - // ( - // address, - // IERC20, - // address, - // ExchangeRateSource, - // uint256, - // uint32, - // bytes32 - // ) - // ); address account; IERC20 feeToken; ExchangeRateSource priceSource; uint256 exchangeRate; uint32 priceMarkup; - bytes32 userOpHash; + // bytes32 userOpHash; assembly ("memory-safe") { let offset := context.offset @@ -558,9 +538,9 @@ contract BiconomyTokenPaymaster is offset := add(offset, 0x20) priceMarkup := calldataload(offset) - offset := add(offset, 0x20) + // offset := add(offset, 0x20) - userOpHash := calldataload(offset) + // userOpHash := calldataload(offset) } uint256 effectiveExchangeRate = exchangeRate; @@ -590,7 +570,7 @@ contract BiconomyTokenPaymaster is feeReceiver, charge ); - emit TokenPaymasterOperation( + /*emit TokenPaymasterOperation( account, address(feeToken), charge, @@ -598,7 +578,8 @@ contract BiconomyTokenPaymaster is userOpHash, effectiveExchangeRate, priceSource - ); + );*/ + emit UserOperationSponsored(account, address(feeToken), charge, actualGasCost); } else { // In case transferFrom failed in first handlePostOp call, attempt to charge the tokens again bytes memory _data = abi.encodeWithSelector( diff --git a/contracts/token/oracles/OracleAggregator.sol b/contracts/token/oracles/OracleAggregator.sol index 77f1e4b..a545363 100644 --- a/contracts/token/oracles/OracleAggregator.sol +++ b/contracts/token/oracles/OracleAggregator.sol @@ -3,12 +3,15 @@ pragma solidity ^0.8.20; import "@openzeppelin/contracts/access/Ownable.sol"; import "./IOracleAggregator.sol"; -import "./IPriceOracle.sol"; import "./FeedInterface.sol"; abstract contract OracleAggregator is Ownable, IOracleAggregator { error MismatchInBaseAndQuoteDecimals(); + error InvalidPriceFromRound(); + error LatestRoundIncomplete(); + error PriceFeedStale(); + error OracleAddressCannotBeZero(); struct TokenInfo { uint8 tokenDecimals; @@ -30,18 +33,15 @@ abstract contract OracleAggregator is Ownable, IOracleAggregator { address nativeOracle, bool isDerivedFeed ) external onlyOwner { - require( - tokenOracle != address(0), - "feed address can not be zero" - ); - require( - nativeOracle != address(0), - "feed address can not be zero" - ); + if(tokenOracle == address(0)) revert OracleAddressCannotBeZero(); + if(nativeOracle == address(0)) revert OracleAddressCannotBeZero(); require( token != address(0), "token address can not be zero" ); + uint8 decimals1 = FeedInterface(nativeOracle).decimals(); + uint8 decimals2 = FeedInterface(tokenOracle).decimals(); + if (decimals1 != decimals2) revert MismatchInBaseAndQuoteDecimals(); tokensInfo[token].tokenOracle = tokenOracle; tokensInfo[token].nativeOracle = nativeOracle; tokensInfo[token].tokenDecimals = tokenDecimals; @@ -78,19 +78,13 @@ abstract contract OracleAggregator is Ownable, IOracleAggregator { tokenDecimals = tokenInfo.tokenDecimals; if (tokenInfo.isDerivedFeed) { - //uint8 decimals1 = FeedInterface(tokenInfo.nativeOracle).decimals(); - //uint8 decimals2 = FeedInterface(tokenInfo.tokenOracle).decimals(); - //if (decimals1 != decimals2) revert MismatchInBaseAndQuoteDecimals(); - - uint256 price1 = fetchPrice(IPriceOracle(tokenInfo.nativeOracle)); - - uint256 price2 = fetchPrice(IPriceOracle(tokenInfo.tokenOracle)); - + uint256 price1 = fetchPrice(FeedInterface(tokenInfo.nativeOracle)); + uint256 price2 = fetchPrice(FeedInterface(tokenInfo.tokenOracle)); tokenPrice = (price2 * (10 ** 18)) / price1; tokenOracleDecimals = 18; } else { tokenPrice = - fetchPrice(IPriceOracle(tokenInfo.tokenOracle)); + fetchPrice(FeedInterface(tokenInfo.tokenOracle)); tokenOracleDecimals = FeedInterface(tokenInfo.tokenOracle).decimals(); } } @@ -99,7 +93,7 @@ abstract contract OracleAggregator is Ownable, IOracleAggregator { /// @dev This function is used to get the latest price from the tokenOracle or nativeOracle. /// @param _oracle The Oracle contract to fetch the price from. /// @return price The latest price fetched from the Oracle. - function fetchPrice(IPriceOracle _oracle) internal view returns (uint256 price) { + function fetchPrice(FeedInterface _oracle) internal view returns (uint256 price) { ( uint80 roundId, int256 answer, @@ -107,13 +101,14 @@ abstract contract OracleAggregator is Ownable, IOracleAggregator { uint256 updatedAt, uint80 answeredInRound ) = _oracle.latestRoundData(); - require(answer > 0, "BTPM: Chainlink price <= 0"); + + // validateRound + if (answer <= 0) revert InvalidPriceFromRound(); // 2 days old price is considered stale since the price is updated every 24 hours - require( - updatedAt >= block.timestamp - 60 * 60 * 24 * 2, - "BTPM: Incomplete round" - ); - require(answeredInRound >= roundId, "BTPM: Stale price"); + if (updatedAt < block.timestamp - 60 * 60 * 24 * 2) + revert PriceFeedStale(); + if (answeredInRound < roundId) revert PriceFeedStale(); + price = uint256(answer); } } \ No newline at end of file diff --git a/test/bundler-integration/token-paymaster/oracle-aggregator-specs.ts b/test/bundler-integration/token-paymaster/oracle-aggregator-specs.ts index 1514abc..d4d8a46 100644 --- a/test/bundler-integration/token-paymaster/oracle-aggregator-specs.ts +++ b/test/bundler-integration/token-paymaster/oracle-aggregator-specs.ts @@ -312,12 +312,12 @@ describe("Biconomy Token Paymaster (With Bundler)", function () { ); const eventLogs = BiconomyTokenPaymaster.interface.decodeEventLog( - "TokenPaymasterOperation", + "UserOperationSponsored", receipt.logs[3].data ); // Confirming that it's using backup (external) exchange rate in case oracle aggregator / price feed is stale / anything goes wrong - expect(eventLogs.exchangeRate).to.be.equal(rate1); + // expect(eventLogs.exchangeRate).to.be.equal(rate1); await expect( entryPoint.handleOps([userOp], await offchainSigner.getAddress()) diff --git a/test/token-paymaster/oracle-aggregator-specs.ts b/test/token-paymaster/oracle-aggregator-specs.ts index 8ef9425..def8d28 100644 --- a/test/token-paymaster/oracle-aggregator-specs.ts +++ b/test/token-paymaster/oracle-aggregator-specs.ts @@ -311,12 +311,12 @@ describe("Biconomy Token Paymaster", function () { ); const eventLogs = BiconomyTokenPaymaster.interface.decodeEventLog( - "TokenPaymasterOperation", + "UserOperationSponsored", receipt.logs[3].data ); // Confirming that it's using backup (external) exchange rate in case oracle aggregator / price feed is stale / anything goes wrong - expect(eventLogs.exchangeRate).to.be.equal(rate1); + // expect(eventLogs.exchangeRate).to.be.equal(rate1); await expect( entryPoint.handleOps([userOp], await offchainSigner.getAddress())