Token Paymaster - Add Swapping Feature #11
Conversation
| IOracle _nativeAssetToUsdOracle, | ||
| ISwapRouter _uniswapRouter, | ||
| address _wrappedNative, | ||
| address[] memory _tokens, // Array of token addresses supported by the paymaster |
There was a problem hiding this comment.
these are like the bucket of tokens which are supported in independent mode. think of corresponding name or maybe just comment.
There was a problem hiding this comment.
agreed. Needs a better name.
| address _wrappedNative, | ||
| address[] memory _tokens, // Array of token addresses supported by the paymaster | ||
| IOracle[] memory _oracles, // Array of corresponding oracle addresses | ||
| address[] memory _swappableTokens, // Array of tokens that you want swappable by the uniswapper |
There was a problem hiding this comment.
do we really need to pass another array of addresses which is subset of above token addresses?
There was a problem hiding this comment.
hmm to think of it, some tokens from another bucket (External mode) could also be swappable by uniswap.
There was a problem hiding this comment.
also maybe we should create some read method to retrieve token bucket supported by this paymaster for independent mode (and external mode? tricky bc signing service decides that and keeps mapping)
There was a problem hiding this comment.
Addressing the last comment. For independent mode, it would use the tokens that are added by the owner through the public function. For external, the owner could add tokens just like for independent or it would auto add those tokens through the signed data on validation. The latter is impractical imo, since adding a token and corresponding pool is a one time job while the latter would require a change to the data that is signed by the verifyingSigner on every userop plus an additional check in the contract.
There was a problem hiding this comment.
hmm, I think only retrieving independent mode tokens makes sense through the contract. for other we can keep public gated repo
cc Nishant
| address[] memory _tokens, // Array of token addresses supported by the paymaster | ||
| IOracle[] memory _oracles, // Array of corresponding oracle addresses | ||
| address[] memory _swappableTokens, // Array of tokens that you want swappable by the uniswapper | ||
| uint24[] memory _swappableTokenPoolFeeTiers // Array of uniswap pool fee tiers for each swappable token |
There was a problem hiding this comment.
this is required by the swapper?
There was a problem hiding this comment.
yes. need to specify which fee tier pool to swap through. Some tokens have multiple supported tiers causing a difference in liquidity depth, and consequently, potential for price impact.
| } | ||
| } | ||
|
|
||
| function _setTokenPool(address _token, uint24 _poolFeeTier) internal { |
There was a problem hiding this comment.
two methods to add and remove from swappable tokens.
There was a problem hiding this comment.
this is internal
contracts/token/swaps/Uniswapper.sol
Outdated
| tokenToPools[_token] = _poolFeeTier; // set mapping of token to uniswap pool to use for swap | ||
| } | ||
|
|
||
| function _swapTokenToWeth(address _tokenIn, uint256 _amountIn) internal returns (uint256 amountOut) { |
There was a problem hiding this comment.
should write test for this to run on couple of tokens on few mainnets.
| /// It also allows updating price configuration and withdrawing tokens by the contract owner. | ||
| /// The contract uses an Oracle to fetch the latest token prices. | ||
| /// @dev Inherits from BasePaymaster. | ||
| contract TokenPaymaster is BasePaymaster, UniswapHelper, OracleHelper { |
There was a problem hiding this comment.
this reference is from infinitism?
| /// @param requiredPreFund The maximum cost (in native token) the paymaster has to prefund. | ||
| /// @return context The context containing the token amount and user sender address (if applicable). | ||
| /// @return validationResult A uint256 value indicating the result of the validation (always 0 in this implementation). | ||
| function _validatePaymasterUserOp(PackedUserOperation calldata userOp, bytes32, uint256 requiredPreFund) |
There was a problem hiding this comment.
ok, let's discuss here. @ShivaanshK
Q. Does unaccountedGas need to be used at all in calculating precharge?
the maxCost that is being passed into validatePaymasterUserOp is calculated using the limits and the userop can’t use more gas than that
yes, true. it can only deviate by 10% otherwise there is penalty. so requiredPrefund would have paymasterPostOpGasLimit component? (todo: cross verify)
then this is really on how well paymaster service does it's job.
and in _postOp you will get actualGasCost. although diff may be very low in requiredPrefund and actualGasCost there is no harm in keeping unaccountedGasCost [ we should find out this value being set and used in live paymaster and see how off is it / or in our test case ] although we might have to keep it on token config basis but common global can't hurt.
then we shall keep these below lines too
uint256 refundPostopCost = tokenPaymasterConfig.refundPostopCost;
require(refundPostopCost < userOp.unpackPostOpGasLimit(), "TPM: postOpGasLimit too low");
There was a problem hiding this comment.
"My intuition is that even if postOp revert due to exceeding the gas limit, the paymaster still needs to pay the gas used during postOp"
also didn't get this @ShivaanshK
| } | ||
|
|
||
| /** | ||
| * @dev Set a new priceMarkup value. |
|
|
||
| address constant WRAPPED_NATIVE_ADDRESS = address(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2); | ||
|
|
||
| address constant SWAP_ROUTER_ADDRESS = address(0xE592427A0AEce92De3Edee1F18E0157C05861564); |
There was a problem hiding this comment.
which chain addresses are these? does this run on a fork?
|
check lint. |
livingrockrises
changed the title
No description provided.