-
Notifications
You must be signed in to change notification settings - Fork 47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: assume trust of protocol fee controller #223
Conversation
| File | % Lines | % Statements | % Branches | % Funcs | |
src/ProtocolFees.sol
Outdated
@@ -11,6 +11,7 @@ import {PoolKey} from "./types/PoolKey.sol"; | |||
import {PoolId} from "./types/PoolId.sol"; | |||
import {IVault} from "./interfaces/IVault.sol"; | |||
import {BipsLibrary} from "./libraries/BipsLibrary.sol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove BipsLibrary and "using BipsLibrary for uint256;" now
src/ProtocolFees.sol
Outdated
address targetProtocolFeeController = address(protocolFeeController); | ||
bytes memory data = abi.encodeCall(IProtocolFeeController.protocolFeeForPool, (key)); | ||
|
||
bool success; | ||
uint256 returnData; | ||
assembly ("memory-safe") { | ||
// only load the first 32 bytes of the return data to prevent gas griefing | ||
success := call(controllerGasLimit, targetProtocolFeeController, 0, add(data, 0x20), mload(data), 0, 32) | ||
success := call(gas(), targetProtocolFeeController, 0, add(data, 0x20), mload(data), 0, 32) | ||
// if success is false this wont actually be returned, instead 0 will be returned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to edit this comments , will revert with "ProtocolFeeCannotBeFetched" , not 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Context
Currently pool initialization requires a buffer of
1% of block.gasLimit
. This means that for BNB chain (with 100m gas), it require user to allocate > 1mil in gas.Problem: This result in integrator asking why do they need so much gas to initialize a pool.
Also:
Description
This PR assume the trust of protocol fee controller. When integrator try to initialize a pool, they will not need to specify the 1 million buffer.
Now
Previously
1% of block.gasLimit