Implementation of BitcoinRedeemer contract #309
Closed
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
The contract is used to integrate stBTC unstaking with bridging to Bitcoin via tBTC Bridge. The `requestRedemption` function expects the owner of stBTC shares to approve stBTC token shares to the BticoinRedeemer contract. Then when `requestRedemption` function is called it redeems stBTC and calls TBTC token contract to redeem tBTC to Bitcoin.
The extension allows approvals to be made via signatures as per EIP-2612, which opens possibilities for gasless token transfers.
Here we add redeemToBitcoinWithPermit function that uses EIP-712 signatures to increase allowance from the owner to the BitcoinRedeemer contract that will request redemption of stBTC to tBTC and tBTC to Bitcoin via tBTC Bridge.
❌ Deploy Preview for acre-dapp-testnet failed.
|
The event is emitted after stbtc.redeem call, but we control the stBTC contract, so there is no risk.
Merged
The directory is needed by the deployemnt scripts as it contains
`validations.json` used by open zeppelin hardhat-upgrades plugin.
Without it jobs were failing:
```
1) AcreBitcoinDepositor
"before all" hook in "AcreBitcoinDepositor":
ERROR processing /home/runner/work/acre/acre/core/deploy/04_deploy_bitcoin_redeemer.ts:
Error: Validations cache not found. Recompile with `hardhat compile --force`
at readValidations (/home/runner/work/acre/acre/node_modules/.pnpm/@OpenZeppelin[email protected]_@[email protected]_@nomicfoundation+h_7qmii226sdyciusj57th5lkecy/node_modules/@openzeppelin/hardhat-upgrades/src/utils/validations.ts:57:13)
at async getDeployData (/home/runner/work/acre/acre/node_modules/.pnpm/@OpenZeppelin[email protected]_@[email protected]_@nomicfoundation+h_7qmii226sdyciusj57th5lkecy/node_modules/@openzeppelin/hardhat-upgrades/src/utils/deploy-impl.ts:45:23)
at async deployProxyImpl (/home/runner/work/acre/acre/node_modules/.pnpm/@OpenZeppelin[email protected]
```
We use the wrapper in tests instead the snapshots, so we want to be consistent.
For consistency with BitcoinRedeemerUpdated and DispatcherUpdated we added old treasury address in the event.
dimpar
reviewed
Mar 12, 2024
| /// @dev Hash of the type for redeeming stBTC to Bitcoin with permit. | ||
| bytes32 private constant REDEEM_TO_BITCOIN_TYPEHASH = | ||
| keccak256( | ||
| "RedeemToBitcoin(address owner,uint256 shares,bytes bitcoinOutputScript,uint256 nonce,uint256 deadline)" |
There was a problem hiding this comment.
shouldn't bitcoinOutputScript be bytes32?
There was a problem hiding this comment.
It's bytes as per tBTC Bridge requirement: https://github.com/keep-network/tbtc-v2/blob/89616d552b9c36036e0725740443d73dfad5b682/solidity/contracts/bridge/Redemption.sol#L322-L330
There was a problem hiding this comment.
I see, bytes32 redeemerOutputScriptHash what brought my attention. I think bytes can dynamically adjust so yea, should be good
dimpar
reviewed
Mar 12, 2024
|
|
||
| _approve(owner, address(bitcoinRedeemer), shares); | ||
|
|
||
| return |
There was a problem hiding this comment.
Result of bitcoinRedeemer.withdrawAssetsAndUnmint. Which is nothing, as this function doesn't return anything.
There was a problem hiding this comment.
So why do we need return here? When reading the code I find it confusing.
dimpar
reviewed
Mar 12, 2024
| uint256 shares, | ||
| bytes calldata tbtcRedemptionData | ||
| ) public { | ||
| uint256 tbtcAmount = stbtc.redeem(shares, address(this), owner); |
There was a problem hiding this comment.
Just to double check.. I take that this call would fail if the requestRedemption is called directly skipping redeemToBitcoinWithPermit because of the approval?
There was a problem hiding this comment.
Great question! I need to revisit the implementation, as I found out that there is a bug for the direct call path.
dimpar
reviewed
Mar 12, 2024
core/contracts/BitcoinRedeemer.sol
Outdated
| /// @title Bitcoin Redeemer | ||
| /// @notice This contract facilitates redemption of stBTC tokens to Bitcoin through | ||
| /// tBTC redemption process. | ||
| contract BitcoinRedeemer is Initializable { |
There was a problem hiding this comment.
Maybe AcreBitcoinRedeemer just like AcreBitcoinDepositor?
dimpar
reviewed
Mar 12, 2024
| /// tBTC redemption process. | ||
| contract BitcoinRedeemer is Initializable { | ||
| /// Interface for tBTC token contract. | ||
| ITBTCToken public tbtcToken; |
There was a problem hiding this comment.
Since we already import "@keep-network/tbtc-v2" should we maybe import TBTC type instead?
dimpar
reviewed
Mar 12, 2024
This function name should better describe what is happening inside.
This path allows the user to sign exact amount of tBTC tokens to withdraw. It complements RedeemToBitcoin path and is aligned with withdraw and redeem functions defined by ERC4626.
This code is copied from OpenZeppelin Upgradable Contracts library: https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/3cf491630086558f50504d88e76bb4e736c738ab/contracts/token/ERC20/extensions/ERC20PermitUpgradeable.sol With the following changes: - replaced relative import paths with `@openzeppelin/contracts-upgradeable`,
This reverts commit 0d03792.
This reverts commit f719271.
Instead of requiring the user to approve and request redemption in two transactions we use approveAndCall pattern to cover it within one transaction.
|
Closing in favor of #335 |
dimpar
added a commit
that referenced
this pull request
Apr 9, 2024
Closes: #159 In this PR we integrate the withdrawal of tBTC from stBTC vault with tBTC Bridge redemption, so the user can withdraw directly to Bitcoin in one transaction. For the Bitcoin-only experience, it is expected that the owner of the stake will be Safe with Bitcoin message signatures verification capability as per https://github.com/thesis/orangekit/tree/main/solidity. This implementation is different from the initial implementation in #309 as we removed ERC20 permits to handle withdrawals, as dApp users will produce Bitcoin signatures for the Safe, not the EIP-712 Ethereum signatures.
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.
Closes: #159
Closes: #183
Closes: #161
In this PR we implement unstaking process of stBTC shares to Bitcoin via tBTC Bridge in gasless way.
The gasless experience is achieved by EIP-712 signatures over typed data.
The parameters the staker has to sign in
RedeemToBitcoinstructure are:owner- address of the staker,shares- number of stBTC shares to redeem,bitcoinOutputScript- output script for Bitcoin transaction, where the user want's the funds to be directed to,nonce- nonce for the signature,deadline- deadline for the signature.Once the signature is available anyone (in our case this will be a relayer bot) can call
stBTC.redeemToBitcoinWithPermitfunction to initiate redemption.The function validates the signature and allows
BitcoinRedeemercontract to use stBTC staker's shares to redeem stBTC to tBTC and bridge tBTC to Bitcoin via tBTC Bridge.TODO: