Executor and LiquidityPool.sol should use EIP-1559 transaction fee calculation mechanism and not the legacy mechanism

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-biconomy/blob/main/contracts/hyphen/LiquidityPool.sol#L263-L340

Vulnerability details

Impact

LiquidityPool.sol uses legacy transaction fee calculation mechanism. At this moment there aren't yet any specific dates on how log legacy transactions will be supported however accordingly to Coinbase blog post EIP-1559 transactions have huge benefits: https://blog.coinbase.com/the-technical-benefits-of-eip-1559-c41bb85f5924
EIP-1559 transactions on average:

• saved about 9% on effective gas prices
• improved broadcast-to-confirmation time by 11 seconds, or 0.7 blocks

Proof of Concept

Documentation "https://ethereum.org/en/developers/docs/gas/" chapters describes how to calculate gas for legacy and EIP-1559 transactions:

• https://ethereum.org/en/developers/docs/gas/#pre-london
• https://ethereum.org/en/developers/docs/gas/#post-london
  It is necessary to use transaction fee model after the London Upgrade.

Tools Used

Recommended Mitigation Steps

Use EIP-1559 / post London Upgrade transaction fee model:

• LiquidityPool.sendFundsToUser() needs a new input argument transactionFeeTip in order to adjust calculation is there is a tip
• use current block’s base fee global variable: block.basefee https://docs.soliditylang.org/en/latest/units-and-global-variables.html#block-and-transaction-properties

[ankurdubey521]

For networks that support EIP1559, the gas calculations do take EIP1559 into account.

In the contract, you can see that the gasPrice deducted is calculated as
uint256 gasFee = totalGasUsed * tokenGasPrice;
tokenGasPrice is a parameter that is calculated in our backend and passed to the function as a part of the contract call.
In case of EIP1559, tokenGasPrice is calculated as
maxFeePerGas * priceOfNativeTokenInUSD / tokenPriceInUSD

But you're right that we should utilize tx.gasPrice instead of maxFeePerGas off-chain, as this would be much more accurate since it's not guaranteed that maxFeePerGas would be utilized completely.

This is great catch, and should decrease overall user gas costs.

[ankurdubey521]

https://ethereum.stackexchange.com/questions/122090/what-does-tx-gasprice-represent-after-eip-1559
just leaving this here for internal future reference

[pauliax]

Good suggestion but I think this does not pose any serious issues so should belong to the QA report of the warden: #126