Remove deposits maximum total assets limit #332
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to remove the maximum limit for funds held by stBTC contract to accept deposits without any cap.
Since stBTC doesn't have max total assets limit anymore, we don't need to track the limits in the BitcoinDepositor contract.
We no longer need to queue the stake requests as there is no limit for stBTC deposits.
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
We remove test contracts used to test upgradeability as these tests fell like testing OpenZeppelin plugin. Every change to the V1 contract requires us to reflect the changes in the test contracts which is a pain for maintanance.
Reflect changes from V1 contracts in V2 contracts defined for upgrades testing.
We don't need this TODO as minimum deposit amount is managed by `minStake` function.
dimpar
reviewed
Apr 4, 2024
|
|
dimpar
reviewed
Apr 4, 2024
| /// @param depositKey Deposit key identifying the deposit. | ||
| /// @param caller Address that finalized the stake request. | ||
| /// @param queuedAmount Amount of queued tBTC tokens. | ||
| event StakeRequestQueued( |
There was a problem hiding this comment.
This event is also being used in dApp subgraphs e.g. one of the usage is in acre-bitcoin-depositor.ts. There are other places. We should probably let know front-end folks about this change.
Same for other deleted events
There was a problem hiding this comment.
The function has just one property, so we can rename it.
Updated in d38052d |
dimpar
approved these changes
Apr 4, 2024
dimpar
added a commit
that referenced
this pull request
Apr 4, 2024
This PR is a follow-up from #332. Since we removed deposit requests queueing we can combine `finalizeBridging` and `finalizeStake` functions. We include a couple of other small changes in this PR to optimize gas usage. Before: ``` | AcreBitcoinDepositor · finalizeStake · - · - · 208140 · 9 · - │ ·························|···················|·············|·············|··············|···············|·············· | AcreBitcoinDepositor · initializeStake · 145693 · 145717 · 145711 · 8 · - │ ``` After: ``` | AcreBitcoinDepositor · finalizeStake · 176104 · 204304 · 192218 · 14 · - │ ·························|···················|·············|·············|··············|···············|·············· | AcreBitcoinDepositor · initializeStake · 145886 · 145910 · 145903 · 7 · - │ ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We no longer want to enforce a maximum limit for total assets under management. In this PR we remove code related to the maximum total assets limit in stBTC contract.
Additionally, the Bitcoin Depositor contract can be simplified as it won't need to queue deposits anymore due to the maximum assets limit being reached.