SMR-1677 Flow Rate #41
Conversation
flow rate ported, intergration tests for rootBridge switched over to rootBridgeFlowRate. flowRate controls not yet in effect
…m-bridge-contracts into SMR-1677-FlowRate
📃CI ReportCompiling 146 files with 0.8.19
For a full HTML report run: |
src/root/RootERC20Bridge.sol
Outdated
| * @param newRootWETHToken Address of ERC20 WETH on the root chain. | ||
| * @param newChildChain Name of child chain. | ||
| * @param newImxCumulativeDepositLimit The cumulative IMX deposit limit. | ||
| * @dev Can only be called once. |
There was a problem hiding this comment.
Can be called multiple times since its not gated by initializer
There was a problem hiding this comment.
what do we want to do about this? should we check if the roles or other variables set on the initialiser are set and if they are revert? so it can only be called once? Or is the fact its internal and only be accessed through the bridge or flowrate initializers enough?
There was a problem hiding this comment.
It being internal is enough
There was a problem hiding this comment.
ill just remove the dev comment then :)
| if (address(rootToken) == address(0)) { | ||
| revert ZeroAddress(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why aren't rootIMXToken and NATIVE_ETH stored in the mapping, so you can check them all at once rather than doing an if/else?
There was a problem hiding this comment.
I agree. I recall raising this a while back, in a different PR... the discussions around that are here for context
There was a problem hiding this comment.
this will be split out to another ticket as to not hold up this PR
Co-authored-by: Ermyas Abebe <[email protected]>
Co-authored-by: Ermyas Abebe <[email protected]>
Co-authored-by: Ermyas Abebe <[email protected]>
Co-authored-by: Zoraiz Mahmood <[email protected]>
Co-authored-by: Zoraiz Mahmood <[email protected]>
Co-authored-by: Zoraiz Mahmood <[email protected]>
Co-authored-by: Zoraiz Mahmood <[email protected]>
Co-authored-by: Zoraiz Mahmood <[email protected]>
…m-bridge-contracts into SMR-1677-FlowRate
proletesseract
requested review from
ermyas,
wcgcyx,
tsnewnami and
rzmahmood
The flow rate implementation should be code complete.
Callouts
All unit tests for the flow rate have been ported over except the tests around pausing the bridge since that is not merged into main yet.
The withdraw integration tests have been updated to use the RootERC20BridgeFlowRate.
No additional integration tests have been added at this stage.
Todo
Review Focus
The main two files to review are;
src/root/RootERC20Bridge.sol
src/root/flowrate/RootERC20BridgeFlowRate.sol
The interface for the flow rate has been split out to here;
src/interfaces/root/flowrate/IRootERC20BridgeFlowRate.sol
The detection and queue files should be unchanged ;
src/root/flowrate/FlowRateDetection.sol
src/root/flowrate/FlowRateWithdrawalQueue.sol
The Unit tests have all been ported except the ones related to pausing. I did remove some stuff to do with asserting the charlie remainder which i couldnt figure out what it was doing and it was failing the tests.
test/unit/root/flowrate/RootERC20BridgeFlowRate.t.sol
The withdraw integration tests have been updated to use the flowRate bridge;
test/integration/root/withdrawals.t.sol/RootERC20BridgeWithdraw.t.sol