-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fixing withdrawal functionality in stBTC #350
Conversation
Upon withdrawal we need to take into account fees that will be collected by the treasury. A user will get the desired assets, but on top we need to pull the funds from Mezo covering not only what a user wants but also pull a bit more to cover the withdrawal fees. Added tests to cover scenarios with allocated funds to Mezo Portal and then withdrawing and redeeming from Acre.
✅ Deploy Preview for acre-dapp-testnet canceled.
|
We need to calculate fees that are associated with withdrawal assets upon withdrawing when calculating how much assets should be withdarawn from the dispatcher contract. Instead of calling a preview and conversion functions we can modify access modifiers of _feeOnRaw and _feeOnTotal to internal and use it in stBTC. That would simplify calculations.
const expectedRedeemedAssets = | ||
(await stbtc.previewRedeem(to1e18(2))) - 2n |
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.
Is there a chance we could hardcode the expected value and add comments clarifying calculations?
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.
I replaced most of the preview*
functions with the helpers feeOnTotal
and feeOnRaw
functions. Note that feeOnTotal
and feeOnRaw
are tested as part of this file. We have a lot to cover and later to maintain. Calculating everything by hand adds more work, especially if we want to change the fees. If for any reason we decide to simulate different fee other than what we have now, all the tests will fail that touch fees and will need to be recalculated again. Internal helper function feeOnTotal
and feeOnRaw
replace the manual calculation which will be done exactly same way and makes the development faster and easier. Besides we have a couple of tests with the manual calculations too, I think we can rely on helper functions (that are tested) and make the development easier.
Depends on #350 Covering `totalAssets()` with tests.
Depends on #350 This configuration is aimed to run `MezoAllocator` tests against `mainnet` fork and check the flow with the real [Mezo Portal contract](https://etherscan.io/address/0xAB13B8eecf5AA2460841d75da5d5D861fD5B8A39#code) deployed on mainnet. This also sets up a ground for other tests that can also be moved and checked against the real contracts on mainnet. The network is called "integration". A local hardhat node should be run to execute these tests. E.g. ``` npx hardhat node --no-deploy --fork https://eth-mainnet.g.alchemy.com/v2/<key> --fork-block-number 19680873 ``` `19680873` the current block number was used as of writing these tests. Once the local node is running, then the tests can be executed by running: ``` pnpm test:integration ```
Upon withdrawal we need to take into account fees that will be collected by the treasury. A user will get the desired assets, but on top of that we need to pull a bit more funds from Mezo covering withdrawal fees.
Added tests to cover scenarios with allocated funds to Mezo Portal and then withdrawing and redeeming from Acre.